Framework: Validator greater/less then can't compare ints vs floats

[JochemKlingeler]

System

• Laravel Version: v5.6.23
• PHP Version: 7.2.4

Description:

When using the gt/lt/gte/lte validation rules, it can't compare integers against floats as these rules check if its comparing values of the same type.

I would expect that if both values are 'numeric' it shouldn't matter if one of them is a float and another one is an integer.

The culprit seems to be the requireSameType($first, $second) function in ValidatesAttributes.php which calls gettype() for both values.

Steps To Reproduce:

Execute the following code:

$validator = \Illuminate\Support\Facades\Validator::make(
    [
        'minimum' => 1.25,
        'maximum' => 1,
    ],
    [
        'minimum' => 'numeric',
        'maximum' => 'numeric|gte:minimum',
    ]
);
if ($validator->fails()) {
    echo 'Failure!' . PHP_EOL;
} else {
    echo 'Success!' . PHP_EOL;
}
return;

Expected output:

> Failure!

Actual output:

In ValidatesAttributes.php line 1655:

  [InvalidArgumentException]
  The values under comparison must be of the same type  

Exception trace:
 Illuminate\Validation\Validator->requireSameType() at /var/www/hub/vendor/laravel/framework/src/Illuminate/Validation/Concerns/ValidatesAttributes.php:899
 Illuminate\Validation\Validator->validateGte() at /var/www/hub/vendor/laravel/framework/src/Illuminate/Validation/Validator.php:365
 Illuminate\Validation\Validator->validateAttribute() at /var/www/hub/vendor/laravel/framework/src/Illuminate/Validation/Validator.php:268
 Illuminate\Validation\Validator->passes() at /var/www/hub/app/Console/Commands/TestCommand.php:98
 App\Console\Commands\TestCommand->handle() at n/a:n/a
 call_user_func_array() at /var/www/hub/vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php:29
 Illuminate\Container\BoundMethod::Illuminate\Container\{closure}() at /var/www/hub/vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php:87
 Illuminate\Container\BoundMethod::callBoundMethod() at /var/www/hub/vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php:31
 Illuminate\Container\BoundMethod::call() at /var/www/hub/vendor/laravel/framework/src/Illuminate/Container/Container.php:564
 Illuminate\Container\Container->call() at /var/www/hub/vendor/laravel/framework/src/Illuminate/Console/Command.php:183

[vlakoff]

Makes sense. That might be fixed with:

if (!is_numeric($value) || !is_numeric($comparedToValue)) {
    $this->requireSameType($value, $comparedToValue);
}

return $this->getSize($attribute, $value) > $this->getSize($attribute, $comparedToValue);

Or maybe better:

if (is_numeric($value) && is_numeric($comparedToValue)) {
    return $value > $comparedToValue;
}

$this->requireSameType($value, $comparedToValue);

return $this->getSize($attribute, $value) > $this->getSize($attribute, $comparedToValue);

[jurios]

Hi,

Sorry for my English, I’m not an English speaker.

I wanted to report a tricky behavior that it could be really related with this issue. That’s why I’ve reported here. (I’m sorry if that’s not the procedure)

I have two inputs which are validated by a comparison rule (lte, gte… whatever). When I submit the form with one of the inputs empty, the comparison validation has the same error as @JochemKlingeler . Because, I could imagine, the empty input has a null value when is validated and can’t be compared. So a 500 is sent to the client.

If I add a required validation on both inputs in order to validate if the inputs are empty and I submit the form ( again, with an empty input), the validation has the same error because the comparison validation has been applied anyway (all rules are applied).

As a general fact, if we use a comparison validation (like lte, gte… and so on) and we submit an empty input (which will be compared) then we’ll have a 500 error on the server.

In my honest opinion, and as a suggestion, I think comparison validations should compare both inputs only when their values can be compared. If one of them, or both, are incomparable values (as null and numeric, for example) then it should return a validation fail directly (avoiding the comparison logic and the 500 error) or, event better, I would give the responsibility to the developer to define which should be the validation result when that happens.

It could be using a 3th argument in the validation rule like this:

“input” => “numeric|gte:other_input, true” // This will return OK whether input or other_input has incomparable values
“input” => “numeric|gte:other_input, false” // This will return FAIL whether input or other_input has incomparable values

Or creating a new rules like gte_or_fail or gte_or_ok, for example.

What do you think about that?

I hope you understand me, I’ve tried my best to explain myself as good as I’ve able to.

[vlakoff]

@jurios, if I guessed correctly, even if you put a required rule at the beginning of both rulesets, there is an error, because in this example:

'foo' => 'required|gt:bar'
'bar' => 'required'

"foo gt bar" runs before "bar required", so it fetches the value of bar attribute and tries to compare against it, before the presence of bar is enforced.

[jurios]

@vlakoff yes, it will throw this error in that scenario. If you change the position:

'bar' => 'required',
'foo' => 'required|gt:bar'

it will throw the The values under comparison must be of the same type error too. I've realized that it throws an error when bar(which is the input in the gt rule) has no value. So, in our example, when 'bar' is null it throws an exception.

Looks like the comparison validation rules are being fired regardless of the status of the others rules. So it doesn't matter if you use required or not becuase gt will be fired and then it will throw an exception.

[driesvints]

Heya, thanks for submitting this. This seems like a feature request or an improvement. It's best to post these at the laravel/ideas repository to get support for your idea. After that you may send a PR to the framework. Please only use this issue tracker to report bugs and issues with the framework.