diff --git a/docs/todo/issue-6337-blank-array-field-repopulation.md b/docs/todo/archive/issue-6337-blank-array-field-repopulation.md similarity index 75% rename from docs/todo/issue-6337-blank-array-field-repopulation.md rename to docs/todo/archive/issue-6337-blank-array-field-repopulation.md index 8853227295..0be52c0ef5 100644 --- a/docs/todo/issue-6337-blank-array-field-repopulation.md +++ b/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. diff --git a/system/libraries/Form_validation.php b/system/libraries/Form_validation.php index f8b848b497..4ac4b3db56 100644 --- a/system/libraries/Form_validation.php +++ b/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; } // -------------------------------------------------------------------- diff --git a/tests/codeigniter/libraries/Form_validation_test.php b/tests/codeigniter/libraries/Form_validation_test.php index 6f502c5ed8..e6e67fde09 100644 --- a/tests/codeigniter/libraries/Form_validation_test.php +++ b/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; + } } diff --git a/user_guide_src/source/changelog.rst b/user_guide_src/source/changelog.rst index 27ef0511a5..9c13cf52cb 100644 --- a/user_guide_src/source/changelog.rst +++ b/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 ` 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 ==============