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.4] ConvertEmptyStringsToNull middleware is causing problems in validation #17663

Closed
ankurk91 opened this issue Jan 30, 2017 · 9 comments
Closed

Comments

@ankurk91
Copy link
Contributor

ankurk91 commented Jan 30, 2017

  • Laravel Version: 5.4.6
  • PHP Version: 7.0.8
  • Database Driver & Version: MySql 5.6.30

Description:

I upgrade my project from 5.3 to 5.4, everything worked fine.
Now i am facing issues when an input field is optional and submitted as empty string.

Steps To Reproduce:

Lets have a validation request class with rules like -

'amount' => 'numeric|min:0|max:9999',
<input type="number" name="amount" value="">

This field is optional, when we submit the form without typing anything into text field it should pass the validation. But it not.
If we comment out ConvertEmptyStringsToNull middleware it passes the validation.
This is not related to numeric validation. It applies to all type of validations for eg alpha

FYI: I have those two new middlewares in Kernal.php .

protected $middleware = [
        \Illuminate\Foundation\Http\Middleware\CheckForMaintenanceMode::class,
        \Illuminate\Foundation\Http\Middleware\ValidatePostSize::class,
        \App\Http\Middleware\TrimStrings::class,
        \Illuminate\Foundation\Http\Middleware\ConvertEmptyStringsToNull::class,
    ];
@laurencei
Copy link
Contributor

What is the validation error you are getting?

@ankurk91
Copy link
Contributor Author

@lioannou
I am getting the desired validation error message for this field. Here is the message.

The amount must be a number.

@joseaneto
Copy link

Same problem here using "required_with":

        'password' => 'max:200',
        'repeat_password' => 'required_with:password|max:200|same:password',

With password field empty I have a error in repeat_password:

The repeat password and password must match.

Removing the middleware ConvertEmptyStringsToNull works as expected.

@fernandobandeira
Copy link
Contributor

What happens when you use "nullable"?

@ankurk91
Copy link
Contributor Author

ankurk91 commented Jan 30, 2017

@fernandobandeira

Adding nullable to rule passes the validation. This can be a workaround.

'amount' => 'nullable|numeric|min:0|max:9999',

PS - Such changes should be documented somewhere.
PS2 - Taylor added a note about optional fields on docs see

@fernandobandeira
Copy link
Contributor

This is a duplicate of #17558 btw...
This is the intended behavior...

ping @themsaid It seems there's an increasing number of issues like this one, should we add a warning on the validation docs? This middleware is enabled by default...

@themsaid
Copy link
Member

Here's a blog post describing the issue http://themsaid.com/laravel-convert-empty-strings-to-null-20170130/

I'm not sure if it should be in the docs though, maybe the upgrade guide :)

Please feel free to PR the docs to add a hint about that if you want.

@IJack
Copy link

IJack commented Feb 3, 2017

@themsaid I add to my validation nullable, but nw I get many issues with saving the data. Does ConvertEmptyStringsToNull convert my value to "null" also for storing the data, or just for the validation session?

@themsaid
Copy link
Member

themsaid commented Feb 3, 2017

It converts empty request parameters to null, so yeah it'll hit the database as null.

The middleware is not mandatory, you can comment it and get back to the 5.3 behaviour easily.

@ankurk91 ankurk91 changed the title [5.4] ConvertEmptyStringsToNull middleware was causing problems in validation [5.4] ConvertEmptyStringsToNull middleware is causing problems in validation Feb 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants