laravel / framework

The Laravel Framework.
https://laravel.com
MIT License
32.4k stars 10.98k forks source link

Integer validation rule not applying for empty string #30858

Closed anam-hossain closed 4 years ago

anam-hossain commented 4 years ago

Description:

Integer validation rule not applied for empty string

Steps To Reproduce:

Query:

http://localhost/search?page=

$this->validate($request, [
        'page' => 'integer|min:1',
 ]);
devcircus commented 4 years ago

Empty strings are converted to null, which is allowed since you're not using the "required" rule.

EDIT: actually the ConvertEmptyStringsToNull middleware should cause the empty string to be converted to null, therefore causing it to fail for 'integer'.

anam-hossain commented 4 years ago

According to me, by default, the validation should check the integer if the input field is present unless nullable rule is applied.

devcircus commented 4 years ago

Hmm gonna try to reproduce.

gocanto commented 4 years ago

This is odd since this method returns false for empty strings. :/

https://github.com/laravel/framework/blob/6.x/src/Illuminate/Validation/Concerns/ValidatesAttributes.php#L1093

output

Psy Shell v0.9.9 (PHP 7.3.12 — cli) by Justin Hileman
>>> (int) ''
=> 0
>>> filter_var('', FILTER_VALIDATE_INT)
=> false
>>>
>>> filter_var(' ', FILTER_VALIDATE_INT)
=> false
>>>
devcircus commented 4 years ago

Yeah I can't reproduce, the following rule fails with "The number must be an integer", when passing empty string:

['number' => 'integer|min:1']

maybe someone else can take a look. Have you tried one of the Laravel support forums?

devcircus commented 4 years ago

In your kernel.php, do you have the ConvertEmptyStringsToNull middleware in the protected $middleware property? If I comment out this middleware, I get the exact behavior you describe.

robclancy commented 4 years ago

Yes the ConvertEmptyStringsToNull middleware will be why.

anam-hossain commented 4 years ago

@devcircus I believe Lumen does not have kernel.php

https://github.com/laravel/laravel/blob/9bc23ee468e1fb3e5b4efccdc35f1fcee5a8b6bc/app/Http/Kernel.php

Lumen's app.php does not include it as well

https://github.com/laravel/lumen/blob/master/bootstrap/app.php

driesvints commented 4 years ago

Integer validation rule not applied for empty string

Heya. This is actually explained for the integer rule here in a note on the integer rule docs: https://laravel.com/docs/6.x/validation#rule-integer

This validation rule does not verify that the input is of the "integer" variable type, only that the input is a string or numeric value that contains an integer.

So you'll need to add the numeric validation rule if you want to make sure that the value is numeric and not an empty string. An empty string for 'integer|min:1' still passes because you're not adding required to the ruleset. min:1 only denounces that when a value is present that it should have a minimum length of 1. In this case I believe you want: required|integer.

Does that help?

anam-hossain commented 4 years ago

Hi, Thank you for the detail response. The difference between integer|min:1 and required|integer is significant as the API request will fail if the input value is not available.

I can see Laravel deal with this issue by using ConvertEmptyStringsToNull middleware. Is there any plan for Lumen framework to use a similar kind of middleware?

devcircus commented 4 years ago

Ah completely missed that you were using lumen.

gocanto commented 4 years ago

I am pretty positive you can port this middleware to lumen

driesvints commented 4 years ago

@anam-hossain you'll need to use integer|nullable then in this case as the ConvertEmptyStringsToNull isn't available in Lumen. We don't have any plans to add this to Lumen for the time being, sorry.