Fix #4633

bcit-ci · May 17, 2016 · 0fae625 · 0fae625
1 parent 0a840c6
commit 0fae625
Show file tree

Hide file tree

Showing 3 changed files with 63 additions and 72 deletions.
diff --git 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
@@ -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
@@ -28,6 +28,7 @@ Bug fixes for 3.0.7
 -  Fixed a bug (#4563) - :doc:`Input Library <libraries/input>` method ``request_headers()`` ignores ``$xss_clean`` parameter value after first call.
 -  Fixed a bug (#4605) - :doc:`Config Library <libraries/config>` method ``site_url()`` stripped trailing slashes from relative URIs passed to it.
 -  Fixed a bug (#4613) - :doc:`Email Library <libraries/config>` 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 <libraries/form_validation>` ignored multiple "callback" rules for empty, non-required fields.
 
 Version 3.0.6
 =============