Fix #6337: blank array-notation field repopulates with the default

docs/todo/issue-6337-blank-array-field-repopulation.md → docs/todo/archive/issue-6337-blank-array-field-repopulation.md

@@ -3,7 +3,8 @@
 **Upstream:** https://github.com/bcit-ci/CodeIgniter/issues/6337
 **Target:** this repository (CodeIgniter 3.1-stable derivative)
 **Reviewed:** 2026-10-02
-**Status:** Not actioned.
+**Status:** Applied 2026-10-06 in `18b862671`, option (a) (section 8). Upgrade note for
+the release in section 10.
 **Verdict:** Genuine bug, present in this repository on a different line. The
 one-line fix proposed upstream is correct. It removes a leftover from the #113 fix
 that should have gone when #3816 was fixed, and makes array-notation fields
@@ -34,6 +35,10 @@ symptom for plain fields, fixed only for plain fields). The reporter mentions
 
 ## 2. Where the defect is in this repository
 
+> Line numbers in sections 2–7 refer to `Form_validation.php` as reviewed, before
+> `18b862671`. That commit removed two lines (626–627), so every reference after
+> line 627 is now two lines lower in the file.
+
 Upstream cites `Form_validation.php:573`/`:577` at `3658d73`. Here the same code
 sits at [Form_validation.php:619-628](system/libraries/Form_validation.php#L619-L628).
 The line numbers differ because of local changes higher in the file (for example
@@ -159,6 +164,15 @@ This change can be fully tested without a database. Suggested additions to
   fix);
 - a callback on a blank `name[key]` field receives `''`.
 
+**Added in `18b862671`**, together with a fourth test for `matches` between two
+blank array fields. Without the fix, three of the four fail. The #113 test
+passes either way, as it should: it guards against a regression.
+
+Note for anyone writing a `matches` test: `set_rules()` ignores a field whose
+rule string is empty, so the target of `matches[...]` must carry at least one
+rule (the test uses `trim`). Otherwise `matches` has no `postdata` to compare
+against and fails, for plain and array fields alike.
+
 ## 8. Effort and scope options
 
 | Option | Effort | Risk |
@@ -177,3 +191,44 @@ This change can be fully tested without a database. Suggested additions to
    upstream and pocketarc can take the same fix.
 
 Nothing is deferred.
+
+**Decided: (a).** Applied in `18b862671`. Steps 2 and 3 are outside this
+repository: step 2 is the upgrade note in section 10, and step 3 is still open.
+
+### Downstream scans before the decision
+
+Two dependants were searched for the patterns in section 5 (validation rules on
+`name[key]` fields, callbacks on them, `matches` between them, and `set_value()`
+on them):
+
+| Dependant | Result |
+|-----------|--------|
+| NCompPOSv2 | Not affected. All rules are on plain fields, the two `matches` rules are on plain fields, `set_value()` is never called (forms repopulate through the application's own `form_value()`), and its `custom[ID]` fields are validated outside `CI_Form_validation`. |
+| [ixaya/manager](https://packagist.org/packages/ixaya/manager) 2.4.0 (`6c342e0`) | Not affected. The package makes no `set_rules()`, `set_value()` or `matches` calls and does not extend `CI_Form_validation`. Its two sample applications under `extras/` validate only plain `email` and `password` fields. |
+
+Applications built on ixaya/manager could not be scanned. ixaya/manager
+requires `nielbuys/framework ^3.1`, which accepts any 3.x release, so a minor
+version bump does not hold the change back from them; the upgrade note is the
+only warning they get.
+
+## 10. Upgrade note for the release
+
+Release notes here are generated from PR titles, so this text must be added to
+the GitHub release by hand. The same note is in
+[changelog.rst](user_guide_src/source/changelog.rst) under 3.1.25. This change
+was held out of 3.1.24 so users had a release cycle to comment on PR #21.
+
+> **Behaviour change — blank array-notation fields (#6337).** A field named with
+> array notation (for example `address[city]`) that is submitted blank now keeps
+> its blank value, the same as a plain field. Previously it was treated as "not
+> submitted", so `set_value()` showed the default again.
+>
+> Check before upgrading, for rules on `name[key]` fields only:
+>
+> - **Callbacks** now receive `''` instead of `NULL` for a blank value. A
+>   callback that uses `=== NULL` or `is_null()` to mean "left blank" must also
+>   accept `''`.
+> - **`matches`** between two blank array fields now passes. Add `required`
+>   where a blank pair must be rejected.
+>
+> Plain fields are unaffected.

system/libraries/Form_validation.php

@@ -623,8 +623,7 @@ protected function _reduce_array($array, $keys, $i = 0)
 			return isset($array[$keys[$i]]) ? $this->_reduce_array($array[$keys[$i]], $keys, ($i+1)) : NULL;
 		}
 
-		// NULL must be returned for empty fields
-		return ($array === '') ? NULL : $array;
+		return $array;
 	}
 
 	// --------------------------------------------------------------------

tests/codeigniter/libraries/Form_validation_test.php

@@ -462,6 +462,55 @@ public function test_set_value()
 		$_POST = array();
 	}
 
+	public function test_set_value_blank_array_field()
+	{
+		// #6337: a blank array-notation field must repopulate as blank,
+		// not with the default - the same as a plain field does
+		$this->form_validation->set_rules('city', 'City', 'required');
+		$this->form_validation->set_rules('address[city]', 'City', 'required');
+
+		$_POST = array('city' => '', 'address' => array('city' => ''));
+		$this->assertFalse($this->form_validation->run());
+		$this->assertSame('', $this->form_validation->set_value('city', 'default'));
+		$this->assertSame('', $this->form_validation->set_value('address[city]', 'default'));
+		$this->assertSame('', set_value('address[city]', 'default'));
+
+		$_POST = array();
+	}
+
+	public function test_blank_optional_array_field_skips_rules()
+	{
+		// #113: a blank, non-required array field must not fail valid_email
+		$rules = array(array('field' => 'user[email]', 'label' => 'Email', 'rules' => 'valid_email'));
+
+		$this->assertTrue($this->run_rules($rules, array('user' => array('email' => ''))));
+		$this->assertFalse($this->run_rules($rules, array('user' => array('email' => 'invalid'))));
+	}
+
+	public function test_blank_array_field_callback_receives_empty_string()
+	{
+		$callbacks = new Form_validation_test_callbacks();
+		$this->form_validation->set_callback_object($callbacks);
+
+		$this->assertTrue($this->run_rules(
+			array(array('field' => 'address[city]', 'label' => 'City', 'rules' => 'callback_remember')),
+			array('address' => array('city' => ''))
+		));
+		$this->assertSame('', $callbacks->remembered);
+	}
+
+	public function test_rule_matches_blank_array_fields()
+	{
+		// Two blank fields match, as they do for plain fields
+		$rules = array(
+			array('field' => 'pw[a]', 'label' => 'Password', 'rules' => 'matches[pw[b]]'),
+			array('field' => 'pw[b]', 'label' => 'Confirm', 'rules' => 'trim')
+		);
+
+		$this->assertTrue($this->run_rules($rules, array('pw' => array('a' => '', 'b' => ''))));
+		$this->assertFalse($this->run_rules($rules, array('pw' => array('a' => '', 'b' => 'x'))));
+	}
+
 	public function test_set_select()
 	{
 		// Test 1: No options selected
@@ -707,8 +756,16 @@ public function run_rules($rules, $values)
  */
 class Form_validation_test_callbacks {
 
+	public $remembered = FALSE;
+
 	public function is_the_word_ok($str)
 	{
 		return $str === 'ok';
 	}
+
+	public function remember($str)
+	{
+		$this->remembered = $str;
+		return TRUE;
+	}
 }

user_guide_src/source/changelog.rst

@@ -15,6 +15,10 @@ Release Date: Not Released
 Bug fixes for 3.1.25
 ====================
 
+-  Fixed a bug (#6337) - :doc:`Form Validation Library <libraries/form_validation>` treated a blank array-notation field (e.g. ``address[city]``) as not submitted, so ``set_value()`` repopulated it with the default. Array-notation fields now behave like plain fields.
+
+   .. note:: This changes behaviour for blank array-notation fields only. Callbacks on them now receive ``''`` instead of ``NULL``, and ``matches`` between two blank array fields now passes. Review callbacks that use ``=== NULL`` or ``is_null()`` to detect a blank value, and add ``required`` where a blank pair must be rejected.
+
 Version 3.1.24
 ==============