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

validateBoolean is not working with verbose "true" parameter via browser #18563

Closed
webmake opened this issue Mar 29, 2017 · 12 comments
Closed

validateBoolean is not working with verbose "true" parameter via browser #18563

webmake opened this issue Mar 29, 2017 · 12 comments

Comments

@webmake
Copy link
Contributor

webmake commented Mar 29, 2017

  • Laravel Version: 5.4.16
  • PHP Version: 7.1

Description:

Boolean validation seems failing, because value is "true" not true, with message "The parameter field must be true or false."

    protected function validateBoolean($attribute, $value)
    {
        $acceptable = [true, false, 0, 1, '0', '1'];

        return in_array($value, $acceptable, true);
    }

Steps To Reproduce:

$.post('http://appurl.dev/post', { parameter: true})

    public function rules(): array
    {
        return [
            'parameter' => 'bool',
        ];
    }

Maybe I missed something, for example casting or other middleware, or is it bug? Testing application with phpunit tests this problem won't occur, only via browser requests

@laurencei
Copy link
Contributor

laurencei commented Mar 29, 2017

I modified the validateBoolean function a while ago to include the additional types.

If you dump the response on the the server side - are you sure you are getting a true or a "true".

If you are getting a string representation of "true" - then that will not pass - which is intentional.

@laurencei
Copy link
Contributor

"Testing application with phpunit tests this problem won't occur, only via browser requests"

Yeah - this probably means your not getting a true boolean in the request - it is being converted to a string somewhere.

Probably best to have your javascript send 0 or 1.

Any further issues feel free to discuss on the forums on in LaraSlack and I'll try and help out.

@webmake
Copy link
Contributor Author

webmake commented Mar 29, 2017

I am using xdebug, and see, that there is string, not casted value. yes, it works with 1,0, but I hope that true/false also works :/

@webmake
Copy link
Contributor Author

webmake commented Mar 29, 2017

It is not converted to string at all. All values passed through get/post (except php unit tests) are in string representation. So I don't know what reason to keep true, false, if there is no way to pass booleans, either it should be strings, either middleware should cast, IMHO

@devcircus
Copy link
Contributor

your ajax is going to send a string unless you send 0 or 1. I have my form request base class setup to cast
"true" and "false" to bool and the validation works as expected.

@laurencei
Copy link
Contributor

laurencei commented Mar 29, 2017

It is not converted to string at all.

I meant you are sending a true but PHP interprets "true"

So I don't know what reason to keep true, false, if there is no way to pass booleans,

The booleans could come from elsewhere (such as manually cast). There is no need to remove them, as it doesnt negatively affect anything.

either should be strings

The problem is if you allow "true" and "false" - then the string variables are past through, and if you are saving it at the database level you will get:

$user->accepted = "false".

Which in the database will be saved as 1 (i.e. true) - because a non-empty string will evaluate to true I think.

either middleware should cast, IMHO

You can write you own route middleware to do the cast, or do as @devcircus says and cast it yourself on the form request.

We cant add a global middleware to always convert "true" to true - because you might want/expect a string version of true.

@webmake
Copy link
Contributor Author

webmake commented Mar 29, 2017

I meant, that all values from the form are in the same representation, without any casting from string to boolean, and then from boolean to string again. Actually only string value is passed to validateBoolean method as "false"/"true".
BTW, if you don't believe, you can check yourself, how forms and get parameters works:
localhost/?param=true&param2=false, and in index.php put var_dump($_GET), same works with $_POST.
output:

index.php:2:
array (size=2)
  'param' => string 'true' (length=4)
  'param2' => string 'false' (length=5)

I understand negative aspects if there would be $acceptable = [true, false, 0, 1, '0', '1', 'true', 'false']; as you mentioned literally values such as "true"/"false" saving to database would be truncated, when not expected. BUT: in my opinion it should be casted as made with issue #18125 with objects, casting them once.

And yes, I have my own castings as @devcircus mentioned, but validation doesn't call them, because rules method is called before the controller (they are called via controller), so it has no effect when validation fails. Any suggestions how setup values before rules if this is not still a bug? Without middleware would be perfect.

@webmake
Copy link
Contributor Author

webmake commented Mar 29, 2017

btw: #18141 there was changed ValidationRuleParser, so I believe that casting to boolean should work also in that way

@laurencei
Copy link
Contributor

laurencei commented Mar 29, 2017

@webmake. You could do something like this: #17010

The issue around validation and casting has been discussed many times such as #14467, #15920 and #17010

I suggest looking at the comments from the Laravel team there, and perhaps discuss this further - there is a Laravel/Internals discussion about it here: laravel/ideas#42

@themsaid
Copy link
Member

Closing this issue then, please refer to @laurencei's comment.

@webmake
Copy link
Contributor Author

webmake commented Mar 30, 2017

I think this would be best solution for booleans, http://stackoverflow.com/a/15075609 it covers most of cases

filter_var(    'true', FILTER_VALIDATE_BOOLEAN); // true
filter_var(         1, FILTER_VALIDATE_BOOLEAN); // true
filter_var(       '1', FILTER_VALIDATE_BOOLEAN); // true
filter_var(      'on', FILTER_VALIDATE_BOOLEAN); // true
filter_var(     'yes', FILTER_VALIDATE_BOOLEAN); // true

filter_var(   'false', FILTER_VALIDATE_BOOLEAN); // false
filter_var(         0, FILTER_VALIDATE_BOOLEAN); // false
filter_var(       '0', FILTER_VALIDATE_BOOLEAN); // false
filter_var(     'off', FILTER_VALIDATE_BOOLEAN); // false
filter_var(      'no', FILTER_VALIDATE_BOOLEAN); // false
filter_var('asdfasdf', FILTER_VALIDATE_BOOLEAN); // false
filter_var(        '', FILTER_VALIDATE_BOOLEAN); // false
filter_var(      null, FILTER_VALIDATE_BOOLEAN); // false

Currently I solved this issue like:

    public function prepareForValidation()
    {
        $input = $this->all();
        $input['parameter'] = filter_var(array_get($input, 'parameter'), FILTER_VALIDATE_BOOLEAN);
        $this->replace($input);
    }

@webmake
Copy link
Contributor Author

webmake commented Mar 30, 2017

Well final solution was to send as json content type, without prepareForValidation method, and boolean rule works as expected.

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

4 participants