Framework: [5.3] Validation: rules with "required" continue to evaluate

[mfn]

When a validation rule contains required, it will not stop evaluating further rules. This is really a problem/break with previous versions when using custom validation which, until now, could always expect that the value they're passed fulfills the required rule (i.e. it's not something "nully", "empty", etc.).

Example with 5.2:

~/src/laravel/laravel-5.2 $ ./artisan tinker
Psy Shell v0.7.2 (PHP 7.0.9 — cli) by Justin Hileman
>>> \Validator::extend('custom', function($attr, $val) { var_dump($val); });
=> null
>>> \Validator::make(['foo' => null], ['foo' => 'required|custom'])->messages()->all();
=> [
     "The foo field is required.",
   ]

or

>>> \Validator::make(['link' => null], ['link' => 'required|url'])->messages()->all();
=> [
     "The link field is required.",
   ]

Example with 5.3.4:

~/src/laravel/laravel-5.3 $ ./artisan tinker
Psy Shell v0.7.2 (PHP 7.0.9 — cli) by Justin Hileman
>>> \Validator::extend('custom', function($attr, $val) { var_dump($val); });
=> null
>>> \Validator::make(['foo' => null], ['foo' => 'required|custom'])->messages()->all();
NULL
PHP error:  Array to string conversion in /src/laravel/laravel-5.3/vendor/laravel/framework/src/Illuminate/Support/MessageBag.php on line 245

or

>>> \Validator::make(['link' => null], ['link' => 'required|url'])->messages()->all();    => [
     "The link field is required.",
     "The link format is invalid.",
   ]

The NULL form the var_dump($val) should never have happened and the url validator should not have been called.

Why are further rules called, when the value did not match the required criteria?

[themsaid]

In 5.2 if an attribute was null, empty string, or empty array it would pass all other validation rules, for example this would pass:

Validator::make(['link' => ''], ['link' => 'url'])->messages()->all()

That's acceptable in case the data is coming from HTML, in which an empty field would result an empty string value, but since validation is also used to validate arrays an API request might have a value as an empty string, empty array, or null that shouldn't pass such check.

For this we added a nullable rule, which will pass validation in case the value is null, otherwise the attribute will be validated (unlike 5.2).

To stop validation when required fails you need to use the bail rule:

'bail|required|url'

This will prevent running any further checks after the required check fails which will prevent the situation you are referring to.

[mfn]

@themsaid thank you very much for clearing that up!

In fact I'm using validations for a API-only project and yes, the nullable is very welcome as it was always puzzling why a string validation rule would accept a null.

I see that bail did exist before, so this is great.

Basically, required in Laravel < 5.3 seems to do an implicit bail which now needs to be done explicitly.

Can you confirm this for me? I'm happy to make a PR for the Upgrade Notes because I couldn't find any reference for this.

[themsaid]

Actually in 5.2 even if you didn't use required the attribute would pass validation if it was empty/null, so using bail|required in 5.3 will have same effect as using required only in 5.2

But be careful that bail will always run the first rule, so bail|required|url is not like bail|url|required

[mfn]

Thanks and understood. So basically bail is now required to have the 5.2 behavior for required => breaking change.

[GrahamCampbell]

Sure, though Laravel doesn't follow semver. We always have breaking changes in minor releases.

[mfn]

Fine by me, I just wanted to state it explicitly and have it confirmed so it's good to go for the upgrade guide which doesn't mention it.

[GrahamCampbell]

👍

[taylorotwell]

It really to me seems like if the required rule fails there is no point in calling any other rules. Seems weird that we do.

[mfn]

I share this sentiment.

I can't come up with a use-case where I would want required to continue evaluation further rules. Maybe @GrahamCampbell or @themsaid can come up with an example?

[themsaid]

Working on changing that.

[mfn]

That's pretty awesome, thanks everyone here and especially for @themsaid for fixing and (and ofc @taylorotwell for ... ya know, the framework and all 😄 )