From 2f1b69f46c4380995ee7332ffe38cde9ff513670 Mon Sep 17 00:00:00 2001 From: Niel Buys Date: Sun, 26 Jul 2026 09:29:51 +0200 Subject: [PATCH 1/2] Separate the callback target from the CI reference in Form_validation $CI was doing two unrelated jobs: it is the handle for lang, uri, router and input, and it was also the only place a callback_ rule could be looked up. Code wanting callbacks on anything other than the controller - a model, a service - had to overwrite $CI outright, which handed away the services too and made the callback target depend on whoever assigned last. Because the library is shared for the whole request and the assignment typically happens in a constructor, loading two such objects left the first one's callbacks resolving to "unable to find callback validation rule". The rule never ran, and the field failed with Unable to access an error message corresponding to your field name X. which reads like a missing language line rather than a missing callback. Adds a separate _callback_object with set_callback_object() to set it and callback_object() to read it, consulted only where callbacks are resolved. It defaults to NULL, meaning the CodeIgniter instance, so every existing caller behaves exactly as before and nothing needs to change. Co-Authored-By: Claude Opus 5 --- system/libraries/Form_validation.php | 63 +++++++++++++- .../libraries/Form_validation_test.php | 85 +++++++++++++++++++ 2 files changed, 146 insertions(+), 2 deletions(-) diff --git a/system/libraries/Form_validation.php b/system/libraries/Form_validation.php index eeb83b99f0c..f8b848b4977 100644 --- a/system/libraries/Form_validation.php +++ b/system/libraries/Form_validation.php @@ -56,6 +56,23 @@ class CI_Form_validation { */ protected $CI; + /** + * Object that callback_ rules are resolved against. + * + * NULL means the CodeIgniter instance, which is the historic behaviour and + * is correct when callbacks live on the controller. + * + * This exists because $CI was doing two unrelated jobs: it is the handle + * for core services (lang, uri, router, input) AND it was the only place a + * callback could be looked up. Code that wanted callbacks somewhere other + * than the controller had to overwrite $CI outright, which quietly broke + * the services and made the callback target depend on whoever assigned + * last. Setting this instead leaves $CI alone. + * + * @var object|null + */ + protected $_callback_object = NULL; + /** * Validation data for the current form submission * @@ -152,6 +169,46 @@ public function __construct($rules = array()) // -------------------------------------------------------------------- + /** + * Set Callback Object + * + * Tells the validator where to look for callback_ rules. Without this the + * only way to run callbacks on a model, a service or any other collaborator + * is to overwrite $CI, which also hands it the job of supplying lang, uri, + * router and input. + * + * The library is shared for the whole request, so set this immediately + * before run() rather than once in a constructor - otherwise whoever + * constructed last owns every callback lookup that follows. + * + * Pass NULL to go back to the CodeIgniter instance. + * + * @param object|null $object + * @return CI_Form_validation + */ + public function set_callback_object($object = NULL) + { + $this->_callback_object = is_object($object) ? $object : NULL; + return $this; + } + + // -------------------------------------------------------------------- + + /** + * Callback Object + * + * Where callback_ rules are resolved, defaulting to the CodeIgniter + * instance so existing controllers are unaffected. + * + * @return object + */ + public function callback_object() + { + return isset($this->_callback_object) ? $this->_callback_object : $this->CI; + } + + // -------------------------------------------------------------------- + /** * Set Rules * @@ -713,7 +770,9 @@ protected function _execute($row, $rules, $postdata = NULL, $cycles = 0) { if ($callback) { - if ( ! method_exists($this->CI, $rule)) + $object = $this->callback_object(); + + if ( ! method_exists($object, $rule)) { log_message('debug', 'Unable to find callback validation rule: '.$rule); $result = FALSE; @@ -721,7 +780,7 @@ protected function _execute($row, $rules, $postdata = NULL, $cycles = 0) else { // Run the function and grab the result - $result = $this->CI->$rule($postdata, $param); + $result = $object->$rule($postdata, $param); } } else diff --git a/tests/codeigniter/libraries/Form_validation_test.php b/tests/codeigniter/libraries/Form_validation_test.php index 7d941a93c09..6f502c5ed8f 100644 --- a/tests/codeigniter/libraries/Form_validation_test.php +++ b/tests/codeigniter/libraries/Form_validation_test.php @@ -604,6 +604,79 @@ public function test_encode_php_tags() $this->assertEquals('?>', $this->form_validation->encode_php_tags('?>')); } + // -------------------------------------------------------------------- + // set_callback_object() + // -------------------------------------------------------------------- + + public function test_callback_object_defaults_to_the_ci_instance() + { + // The historic behaviour: callbacks live on the controller + $this->assertSame( + $this->ci_instance(), + $this->form_validation->callback_object() + ); + } + + public function test_set_callback_object_redirects_callback_lookup() + { + $this->form_validation->set_callback_object(new Form_validation_test_callbacks()); + + $this->assertTrue($this->run_rules( + array(array('field' => 'foo', 'label' => 'Foo', 'rules' => 'callback_is_the_word_ok')), + array('foo' => 'ok') + )); + + $this->assertFalse($this->run_rules( + array(array('field' => 'foo', 'label' => 'Foo', 'rules' => 'callback_is_the_word_ok')), + array('foo' => 'nope') + )); + } + + public function test_set_callback_object_leaves_the_ci_reference_alone() + { + // The point of the whole change. Overwriting $CI to move callbacks + // also took lang, uri, router and input with it. + $before = $this->ci_instance(); + + $this->form_validation->set_callback_object(new Form_validation_test_callbacks()); + + $this->assertSame($before, $this->ci_instance()); + $this->assertNotSame($this->ci_instance(), $this->form_validation->callback_object()); + } + + public function test_set_callback_object_null_restores_the_ci_instance() + { + $this->form_validation->set_callback_object(new Form_validation_test_callbacks()); + $this->form_validation->set_callback_object(NULL); + + $this->assertSame($this->ci_instance(), $this->form_validation->callback_object()); + } + + public function test_set_callback_object_ignores_a_non_object() + { + $this->form_validation->set_callback_object('not an object'); + + $this->assertSame($this->ci_instance(), $this->form_validation->callback_object()); + } + + public function test_set_callback_object_is_chainable() + { + $this->assertSame( + $this->form_validation, + $this->form_validation->set_callback_object(new Form_validation_test_callbacks()) + ); + } + + public function test_a_missing_callback_still_fails_rather_than_erroring() + { + $this->form_validation->set_callback_object(new Form_validation_test_callbacks()); + + $this->assertFalse($this->run_rules( + array(array('field' => 'foo', 'label' => 'Foo', 'rules' => 'callback_no_such_method')), + array('foo' => 'ok') + )); + } + /** * Run rules * @@ -627,3 +700,15 @@ public function run_rules($rules, $values) return $valid; } } + +/** + * A collaborator that is not the controller, which is the whole point of + * set_callback_object(). + */ +class Form_validation_test_callbacks { + + public function is_the_word_ok($str) + { + return $str === 'ok'; + } +} From 1d89d2fd0f523a134765776814e46f460649a94c Mon Sep 17 00:00:00 2001 From: Niel Buys Date: Sun, 26 Jul 2026 09:39:15 +0200 Subject: [PATCH 2/2] Check out with LF so the helper tests pass on Windows Fifteen tests fail in a Windows clone with core.autocrlf=true and pass everywhere else: thirteen in Form_helper_test, one in Html_helper_test, one in Calendar_test. They are not PHP version failures. Each compares helper output against a literal multi-line string written in the test file. The helpers build their output from "\n" escapes, so they always emit LF; the expected value picks up whatever line ending the file on disk happens to have. A CRLF checkout therefore compares a CRLF expectation against LF output. .gitattributes only carried export-ignore rules, so nothing told git how to treat line endings and core.autocrlf won by default. Repository content is unchanged - git add --renormalize touched no file but this one, confirming the blobs were already LF and only the checkout differed. Nothing downstream is affected. Co-Authored-By: Claude Opus 5 --- .gitattributes | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/.gitattributes b/.gitattributes index b4f0b6cbb44..93ab7e6c7c5 100644 --- a/.gitattributes +++ b/.gitattributes @@ -1,3 +1,36 @@ +# Line endings +# +# Checked out with LF everywhere, whatever core.autocrlf says locally. +# +# Without this, a Windows clone with core.autocrlf=true gets CRLF source, and +# fifteen tests fail that pass everywhere else. They compare helper output +# against a literal multi-line string written in the test file: the helpers +# build their output from "\n" escapes, so they always emit LF, while the +# expected value picks up whatever the file on disk uses. CRLF checkout, CRLF +# expectation, LF actual, failure - in Form_helper_test, Html_helper_test and +# Calendar_test. +# +# This only affects what lands in the working tree. Repository content is +# unchanged, and PHP does not care either way, so nothing downstream breaks. +* text=auto eol=lf + +# Windows-only files keep their native endings +*.bat text eol=crlf + +# Binaries git should not touch at all +*.png binary +*.jpg binary +*.jpeg binary +*.gif binary +*.ico binary +*.woff binary +*.woff2 binary +*.ttf binary +*.eot binary +*.pdf binary +*.zip binary +*.gz binary + # This file tells which files and directories should be ignored and # NOT downloaded when using composer to pull down a project with # the --prefer-dist option selected. Used to remove development