fuel / validation

FuelPHP Framework - Data Validation library
70 stars 14 forks source link

Validator class method validateField() $dataPresent handling ignores boolean values causing issues with rules based on null interpretation of field values #47

Open mlatzko opened 8 years ago

mlatzko commented 8 years ago

Code causing the issue

// If there is data, and the data is not empty and not numeric. This allows for strings such as '0' to be passed
// as valid values.
$dataPresent = isset($data[$field]) && ! (empty($data[$field]) && ! is_numeric($data[$field]));

See: https://github.com/fuelphp/validation/blob/php7/src/Validator.php#L253

Description

The condition to populate $dataPresent will be set to false value because an "empty(false)" is true. This leads to further issue because the value of the field becomes transformed from "false" to "null". This than leads to issues with rules based on "null" interpretation like "Fuel\Validation\Rule\Required". The problem only occurs while a value of a field is Boolean "false".

Example

$ruleProvider = new \Fuel\Validation\RuleProvider\FromArray;
$validator    = new \Fuel\Validation\Validator;

$rules          = array('myBooleanVariable' => array('required'));
$dataToValidate = array('myBooleanVariable' => false);

$ruleProvider->setData($rules)->populateValidator($validator);

$result = $validator->run($dataToValidate);

$result->getErrors(); // array(1) {'myBooleanVariable' => string(49) "The field is required and has not been specified."}

Possible solution (adding check for Boolean values)

// If there is data, and the data is not empty and not numeric nor boolean. This allows for strings such as '0' to be passed
// as valid values.
$dataPresent = isset($data[$field]) && ! (empty($data[$field]) && ! is_numeric($data[$field]) && ! is_bool($data[$field]));
emlynwest commented 8 years ago

A situation like this should already be covered by the unit tests. You can check against the php7 branch which has the most up-to-date development version of the code?

mlatzko commented 8 years ago

I already take the pre-release 2.0.0. which seems to be based on the branch php7 isnt it?

see: https://github.com/fuelphp/validation/blob/php7/src/Validator.php#L253

emlynwest commented 8 years ago

It's been a while since I have had a chance to do any proper work so I could not remember what state the 2.0 pre-release was in. the php7 branch is indeed based off that.

emlynwest commented 8 years ago

This should be solved by #53