diff --git a/system/libraries/Form_validation.php b/system/libraries/Form_validation.php index e4a5189579a..f5b07a2058d 100644 --- a/system/libraries/Form_validation.php +++ b/system/libraries/Form_validation.php @@ -493,6 +493,63 @@ public function run($group = '') // -------------------------------------------------------------------- + /** + * Prepare rules + * + * Re-orders the provided rules in order of importance, so that + * they can easily be executed later without weird checks ... + * + * "Callbacks" are given the highest priority (always called), + * followed by 'required' (called if callbacks didn't fail), + * and then every next rule depends on the previous one passing. + * + * @param array $rules + * @return array + */ + protected function _prepare_rules($rules) + { + $new_rules = array(); + $callbacks = array(); + + foreach ($rules as &$rule) + { + // Let 'required' always be the first (non-callback) rule + if ($rule === 'required') + { + array_unshift($new_rules, 'required'); + } + // 'isset' is a kind of a weird alias for 'required' ... + elseif ($rule === 'isset' && (empty($new_rules) OR $new_rules[0] !== 'required')) + { + array_unshift($new_rules, 'isset'); + } + // The old/classic 'callback_'-prefixed rules + elseif (is_string($rule) && strncmp('callback_', $rule, 9) === 0) + { + $callbacks[] = $rule; + } + // Proper callables + elseif (is_callable($rule)) + { + $callbacks[] = $rule; + } + // "Named" callables; i.e. array('name' => $callable) + elseif (is_array($rule) && isset($rule[0], $rule[1]) && is_callable($rule[1])) + { + $callbacks[] = $rule; + } + // Everything else goes at the end of the queue + else + { + $new_rules[] = $rule; + } + } + + return array_merge($callbacks, $new_rules); + } + + // -------------------------------------------------------------------- + /** * Traverse a multidimensional $_POST array index until the data is found * @@ -580,70 +637,7 @@ protected function _execute($row, $rules, $postdata = NULL, $cycles = 0) return; } - // If the field is blank, but NOT required, no further tests are necessary - $callback = FALSE; - if ( ! in_array('required', $rules) && ($postdata === NULL OR $postdata === '')) - { - // Before we bail out, does the rule contain a callback? - foreach ($rules as &$rule) - { - if (is_string($rule)) - { - if (strncmp($rule, 'callback_', 9) === 0) - { - $callback = TRUE; - $rules = array(1 => $rule); - break; - } - } - elseif (is_callable($rule)) - { - $callback = TRUE; - $rules = array(1 => $rule); - break; - } - elseif (is_array($rule) && isset($rule[0], $rule[1]) && is_callable($rule[1])) - { - $callback = TRUE; - $rules = array(array($rule[0], $rule[1])); - break; - } - } - - if ( ! $callback) - { - return; - } - } - - // Isset Test. Typically this rule will only apply to checkboxes. - if (($postdata === NULL OR $postdata === '') && ! $callback) - { - if (in_array('isset', $rules, TRUE) OR in_array('required', $rules)) - { - // Set the message type - $type = in_array('required', $rules) ? 'required' : 'isset'; - - $line = $this->_get_error_message($type, $row['field']); - - // Build the error message - $message = $this->_build_error_msg($line, $this->_translate_fieldname($row['label'])); - - // Save the error message - $this->_field_data[$row['field']]['error'] = $message; - - if ( ! isset($this->_error_array[$row['field']])) - { - $this->_error_array[$row['field']] = $message; - } - } - - return; - } - - // -------------------------------------------------------------------- - - // Cycle through each rule and run it + $rules = $this->_prepare_rules($rules); foreach ($rules as $rule) { $_in_array = FALSE; @@ -740,12 +734,6 @@ protected function _execute($row, $rules, $postdata = NULL, $cycles = 0) { $this->_field_data[$row['field']]['postdata'] = is_bool($result) ? $postdata : $result; } - - // If the field isn't required and we just processed a callback we'll move on... - if ( ! in_array('required', $rules, TRUE) && $result !== FALSE) - { - continue; - } } elseif ( ! method_exists($this, $rule)) { @@ -1055,7 +1043,9 @@ public function set_checkbox($field = '', $value = '', $default = FALSE) */ public function required($str) { - return is_array($str) ? (bool) count($str) : (trim($str) !== ''); + return is_array($str) + ? (empty($str) === FALSE) + : (trim($str) !== ''); } // -------------------------------------------------------------------- diff --git a/tests/codeigniter/libraries/Form_validation_test.php b/tests/codeigniter/libraries/Form_validation_test.php index 3537eb3552a..d11d616ad74 100644 --- a/tests/codeigniter/libraries/Form_validation_test.php +++ b/tests/codeigniter/libraries/Form_validation_test.php @@ -55,9 +55,9 @@ public function test_rule_matches() ); $values_base = array('foo' => 'sample'); - $this->assertTrue($this->run_rules($rules, array_merge($values_base, array('bar' => '')))); $this->assertTrue($this->run_rules($rules, array_merge($values_base, array('bar' => 'sample')))); + $this->assertFalse($this->run_rules($rules, array_merge($values_base, array('bar' => '')))); $this->assertFalse($this->run_rules($rules, array_merge($values_base, array('bar' => 'Sample')))); $this->assertFalse($this->run_rules($rules, array_merge($values_base, array('bar' => ' sample')))); } diff --git a/user_guide_src/source/changelog.rst b/user_guide_src/source/changelog.rst index 567eb10a6ff..eb36b0d1e82 100644 --- a/user_guide_src/source/changelog.rst +++ b/user_guide_src/source/changelog.rst @@ -28,6 +28,7 @@ Bug fixes for 3.0.7 - Fixed a bug (#4563) - :doc:`Input Library ` method ``request_headers()`` ignores ``$xss_clean`` parameter value after first call. - Fixed a bug (#4605) - :doc:`Config Library ` method ``site_url()`` stripped trailing slashes from relative URIs passed to it. - Fixed a bug (#4613) - :doc:`Email Library ` failed to send multiple emails via SMTP due to "already authenticated" errors when keep-alive is enabled. +- Fixed a bug (#4633) - :doc:`Form Validation Library ` ignored multiple "callback" rules for empty, non-required fields. Version 3.0.6 =============