From 18b8626712d18c3815c36353f3d5e865f0450fab Mon Sep 17 00:00:00 2001 From: Niel Buys Date: Tue, 6 Oct 2026 21:18:27 +0200 Subject: [PATCH 1/4] Fix #6337: blank array-notation field repopulates with the default _reduce_array() turned a blank '' into NULL for array-notation fields (address[city]). set_value() treats NULL postdata as "not posted" and returned the default, so a field the user cleared came back filled in. Plain fields have kept '' since the #3816 fix; this line was missed then. The NULL conversion was added for #113 (2012), when blank-field rule skipping only checked for NULL. _execute() has skipped both NULL and '' since 2013, so #113 stays fixed without it; a test now guards that. Behaviour change, for array-notation fields submitted blank only - all now match plain fields: - set_value() returns '' instead of the default - callbacks receive '' instead of NULL - matches[] between two blank fields passes instead of failing Co-Authored-By: Claude Opus 5.5 --- system/libraries/Form_validation.php | 3 +- .../libraries/Form_validation_test.php | 57 +++++++++++++++++++ 2 files changed, 58 insertions(+), 2 deletions(-) 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; + } } From c03733f6e2e70f4166bf01c01f52c54ec7263156 Mon Sep 17 00:00:00 2001 From: Niel Buys Date: Tue, 6 Oct 2026 21:19:02 +0200 Subject: [PATCH 2/4] Archive issue #6337 review notes Status updated to applied in 18b862671, option (a). Records the NCompPOSv2 and ixaya/manager downstream scans and the upgrade note text for the release. Co-Authored-By: Claude Opus 5.5 --- ...sue-6337-blank-array-field-repopulation.md | 56 ++++++++++++++++++- 1 file changed, 55 insertions(+), 1 deletion(-) rename docs/todo/{ => archive}/issue-6337-blank-array-field-repopulation.md (75%) 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..5023056ac2 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-07 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,43 @@ 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, and +[changelog.rst](user_guide_src/source/changelog.rst) has not been maintained +since 3.1.13, so this text must be added to the GitHub release by hand: + +> **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. From 2b2f4871968fa035dc3cca3cc52df16f3411b1b6 Mon Sep 17 00:00:00 2001 From: Niel Buys Date: Tue, 6 Oct 2026 21:30:15 +0200 Subject: [PATCH 3/4] Changelog: add #6337 to 3.1.24, with an upgrade note Co-Authored-By: Claude Opus 5.5 --- user_guide_src/source/changelog.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/user_guide_src/source/changelog.rst b/user_guide_src/source/changelog.rst index 1a57287bb4..055db4dec7 100644 --- a/user_guide_src/source/changelog.rst +++ b/user_guide_src/source/changelog.rst @@ -27,6 +27,9 @@ Bug fixes for 3.1.24 - Fixed a bug (pocketarc #51) - :doc:`Text Helper ` function :php:func:`highlight_code()` left a stray ``?>`` at the end of its output on PHP 8.3+, visible on every query in the :doc:`Profiler `. - Fixed a bug (pocketarc #47) - :doc:`File Helper ` function :php:func:`get_dir_file_info()` raised a PHP 8.1 "automatic conversion of false to array" deprecation for entries ``get_file_info()`` cannot read, such as broken symlinks. Such entries are now skipped. +- 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.23 ============== From 98f91e09cf42220db2e21397f64cff4d3105f3b0 Mon Sep 17 00:00:00 2001 From: Niel Buys Date: Tue, 6 Oct 2026 22:18:28 +0200 Subject: [PATCH 4/4] Move #6337 changelog entry to 3.1.25; correct date Held out of 3.1.24, so the entry moves to the new 3.1.25 section. The review doc's status date is corrected to 2026-10-06 and its upgrade-note section now points at the changelog. Co-Authored-By: Claude Opus 5.5 --- .../archive/issue-6337-blank-array-field-repopulation.md | 9 +++++---- user_guide_src/source/changelog.rst | 7 ++++--- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/docs/todo/archive/issue-6337-blank-array-field-repopulation.md b/docs/todo/archive/issue-6337-blank-array-field-repopulation.md index 5023056ac2..0be52c0ef5 100644 --- a/docs/todo/archive/issue-6337-blank-array-field-repopulation.md +++ b/docs/todo/archive/issue-6337-blank-array-field-repopulation.md @@ -3,7 +3,7 @@ **Upstream:** https://github.com/bcit-ci/CodeIgniter/issues/6337 **Target:** this repository (CodeIgniter 3.1-stable derivative) **Reviewed:** 2026-10-02 -**Status:** Applied 2026-10-07 in `18b862671`, option (a) (section 8). Upgrade note for +**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 @@ -213,9 +213,10 @@ only warning they get. ## 10. Upgrade note for the release -Release notes here are generated from PR titles, and -[changelog.rst](user_guide_src/source/changelog.rst) has not been maintained -since 3.1.13, so this text must be added to the GitHub release by hand: +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 diff --git a/user_guide_src/source/changelog.rst b/user_guide_src/source/changelog.rst index 1943295ce8..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 ============== @@ -36,9 +40,6 @@ Bug fixes for 3.1.24 - Fixed a bug (pocketarc #51) - :doc:`Text Helper ` function :php:func:`highlight_code()` left a stray ``?>`` at the end of its output on PHP 8.3+, visible on every query in the :doc:`Profiler `. - Fixed a bug (pocketarc #47) - :doc:`File Helper ` function :php:func:`get_dir_file_info()` raised a PHP 8.1 "automatic conversion of false to array" deprecation for entries ``get_file_info()`` cannot read, such as broken symlinks. Such entries are now skipped. -- 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.23 ==============