Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[5.2] Numeric fails to validate against empty value #11452

Closed
arthurkirkosa opened this issue Dec 22, 2015 · 25 comments
Closed

[5.2] Numeric fails to validate against empty value #11452

arthurkirkosa opened this issue Dec 22, 2015 · 25 comments
Labels

Comments

@arthurkirkosa
Copy link
Contributor

https://github.com/laravel/framework/blob/5.2/src/Illuminate/Validation/Validator.php#L152

Having numeric (and others) in the $implicitRules variable implies that the particular field MUST have a value, and fails for empty values.

// InvoiceRequest
public function rules()
    {
        return [
            ...
            'amount' => 'required|numeric',
            'vat_amount' => 'numeric',
            ...
        ];
    }

// Checking the HTML output

<li>The vat amount must be a number.</li>

<input class="form-control" placeholder="VAT Amount" required="" name="vat_amount" type="text" value="">
@GrahamCampbell
Copy link
Member

Is this an issue on 5.1 too?

@GrahamCampbell GrahamCampbell changed the title Numeric fails to validate against empty value [5.2] Numeric fails to validate against empty value Dec 22, 2015
@arthurkirkosa
Copy link
Contributor Author

No. I didn't have this issue until the upgrade to 5.2. Traced the issue to that line

@GrahamCampbell
Copy link
Member

Thanks.

@GrahamCampbell
Copy link
Member

Ping @taylorotwell. Was this intentional. I know there were some changes to validation in L5.2.

@taylorotwell
Copy link
Member

Yeah I think it is intentional. If you send a value, it has to be numeric. So I would suggest sending 0 or something instead of an empty string.

@jordandobrev
Copy link

Brakes my project too.

validate( ['nullable_number' => null], ['nullable_number' => 'numeric'] )

fails

@taylorotwell
Copy link
Member

Yes, that is intentional. If you provide the field, it must be numeric, as
your validation rules state.

On Tue, Dec 22, 2015 at 8:32 AM, Jordan Dobrev notifications@github.com
wrote:

Brakes my project too.

validate(
['nullable_number' => null],
['nullable_number' => 'numeric']
)

fails


Reply to this email directly or view it on GitHub
#11452 (comment)
.

@jordandobrev
Copy link

Aren't we supposed to use sometimes for that?

How do we handle the validation of nullable numbers?

@taylorotwell
Copy link
Member

​You could send a 0.​

On Tue, Dec 22, 2015 at 8:48 AM, Jordan Dobrev notifications@github.com
wrote:

Aren't we supposed to use sometimes for that?

How do we handle the validation of nullable numbers?


Reply to this email directly or view it on GitHub
#11452 (comment)
.

@jordandobrev
Copy link

This won't work if the validation is part of your eloquent model.

I am using Jeffrey's package like validation:
https://github.com/JeffreyWay/Laravel-Model-Validation/blob/master/src/Way/Database/Model.php

$v = $this->validator->make($this->attributes, static::$rules, static::$messages);

Attributes is going to contain nullable values when you attempt to perform an update.

Should we filter the attributes and skip null values then?

@taylorotwell
Copy link
Member

You could. You could also extend Validator using Validator::extend to add a
nullable_numeric rule or something like that. Would be easy.

On Tue, Dec 22, 2015 at 9:01 AM, Jordan Dobrev notifications@github.com
wrote:

This won't work if the validation is part of your eloquent model.

I am using Jeffrey's package like validation:

https://github.com/JeffreyWay/Laravel-Model-Validation/blob/master/src/Way/Database/Model.php

$v = $this->validator->make($this->attributes, static::$rules,
static::$messages);

Attributes is going to contain nullable values when you attempt to perform
an update.

Should we filter the attributes and skip null values then?


Reply to this email directly or view it on GitHub
#11452 (comment)
.

@jordandobrev
Copy link

Great! Thx

@mtctonyhkhk2010
Copy link

But in the 5.2 docs, http://laravel.com/docs/5.2/validation#custom-validation-rules.
In the part about Implicit Extensions, it said:

"By default, when an attribute being validated is not present or contains an empty value as defined by the required rule, normal validation rules, including custom extensions, are not run. For example, the integer rule will not be run against a null value:"

Does the docs need to be updated? Or not?

@taylorotwell
Copy link
Member

In this case the docs would need to be updated. Basically, in Laravel 5.2,
if you say something has to be numeric, it actually has to be numeric. Or,
if you say something has to be a string, it has to be a string. You can't
specify something must be numeric and then try to pass null or an empty
string through the validator.

On Tue, Dec 22, 2015 at 9:37 AM, mtctonyhkhk2010 notifications@github.com
wrote:

But in the 5.2 docs,
http://laravel.com/docs/5.2/validation#custom-validation-rules.
In the part about Implicit Extensions, it said:

"By default, when an attribute being validated is not present or contains
an empty value as defined by the required rule, normal validation rules,
including custom extensions, are not run. For example, the integer rule
will not be run against a null value:"

Does the docs need to be updated? Or not?


Reply to this email directly or view it on GitHub
#11452 (comment)
.

@arthurkirkosa
Copy link
Contributor Author

This means that the required rule is not necessary any more for 'Array', 'Boolean', 'Integer', 'Numeric', 'String' (which sum up all the possible types you can pass from a form)

@taylorotwell
Copy link
Member

I'll just revert to 5.1 behavior even though I think it's kind of weird.

On Tue, Dec 22, 2015 at 10:20 AM, Arthur Kirkosa notifications@github.com
wrote:

This means that the required rule is not necessary any more for 'Array',
'Boolean', 'Integer', 'Numeric', 'String' (which sum up all the possible
types you can pass from a form)


Reply to this email directly or view it on GitHub
#11452 (comment)
.

@arthurkirkosa
Copy link
Contributor Author

It's not necessarily weird. If you have a integer nullable field in you db you'd like a null value if the field is left empty.

Or a datetime nullable ... for an "expiration_date" field... you'd expect it to allow empty values.

@jordandobrev
Copy link

What if an additional 'nullable' rule is added to the validations. Wouldn't that make more sense?

validate(
    ['nullable_field' => null],
    ['nullable_field' => 'numeric|nullable']]
);

@panique
Copy link

panique commented Apr 6, 2016

+1, can reproduce on L5.2, this feels broken, empty string is not numeric, also numeric|min:1 validates an empty string to true ... hmmm

@GrahamCampbell GrahamCampbell reopened this Apr 6, 2016
@themsaid
Copy link
Member

@GrahamCampbell passing an empty string as a value will cause the validator to neglect the key for validation if the rules doesn't indicate the field is required and sometimes isn't used, it's apparently a bug but fixing it might be considered breaking. What's the best way to deal with such situation? Keep the same behaviour for 5.2 and fix in 5.3? Or just fix in 5.2 since it's considered a bug?

@themsaid
Copy link
Member

themsaid commented Sep 6, 2016

This was reverted in 5.2, and in 5.3 a new nullable validation rule was added and the validator became more strict when it comes to primitives.

@themsaid themsaid closed this as completed Sep 6, 2016
@jbajou
Copy link

jbajou commented Dec 23, 2016

I really don't think this nullable rule makes a lot of sense (or I don't understand it as much as you guys).
I mean, we have already the required rule that validates if you actually pass some data. Having the nullable makes it able to not be required, like the opposite of the required rule BUT only for primitives.
From my point of view, if it is not required then it is nullable, and this has nothing to do with the type of the data. I think previous (< L5.1) was more consistant with the wording. Am I missing something here ?

@denis-chmel
Copy link

denis-chmel commented Feb 7, 2017

Guys, this is so sick.
Now it is not possible (is it?) to validate some primitive formats with "require_if".
E.g. in the following (wild life) use-case, laravel 5.4:

       'voucher_price' => "required_if:voucher_enabled,1|integer",

Now no matter if my "voucher_enabled" checkbox is checked or not - the "voucher_price" is failing validation, because some empty price comes in post. Which is not illegal in this case.

And if I change the rule to:

       'voucher_price' => "required_if:voucher_enabled,1|integer|nullable",

Then it won't fail validation even when it should: i.e. when price is empty, while required.
And when I try this:

       'voucher_price' => "bail|required_if:voucher_enabled,1|integer",

Then if my voucher_enabled is 0, voucher_price is empty (not illegal) and validation fails on "integer" rule, because null is not integer.

Do you expect us to write our own "integer_if", "min_if" "numeric_unless", "email_if" custom validations?

@ITwrx
Copy link

ITwrx commented Jan 21, 2018

@jbajou I agree. To me, nullable should just be the default when a field is not required. If i send null, I don't expect laravel to be trying to validate that it matches the rule I created for when a "normal value"(anything except null) is actually sent. I understand that laravel is just trying to be strict/explicit about what is being validated, but maybe doing the reverse would make more sense for the majority of devs. IOW, a "not_nullable" rule.

@chuoke
Copy link
Contributor

chuoke commented May 19, 2018

How to validate field is email or empty in default rules?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests