Framework: Can't use boolean validation on query string parameters with JSON requests

[atrauzzi]

• Laravel Version: 5.8
• PHP Version: 7.2
• Database Driver & Version: N/A?

Description:

Inbound requests with content type of application/json that also have query string parameters might be experiencing some inconsistency when using FormRequest rules for validation.

Steps To Reproduce:

Inside of a subclass of FormRequest:

    // ...
    public function rules()
    {
        return [
            'scoped' => 'boolean',
        ];
    }
    // ...

Then, issue a request similar to:

Which has the response body:

{
    "message": "The given data was invalid.",
    "errors": {
        "scoped": [
            "The scoped field must be true or false."
        ]
    }
}

[staudenmeir]

Has this worked before?

[atrauzzi]

Not sure at this point as I've only just encountered it now.

Really, if the validator is saying "The scoped field must be true or false" and my scoped value is "false", my assumption is that validation should be passing, not failing. 😉

[setkyar]

I try it on Laravel 5.7. *. Same behavior, it's the correct behavior? Since you are using the query string. It should solve from your application?

[atrauzzi]

@setkyar - Apologies, I'm not sure what you mean.

I'm just thinking, true,false,0,1 are probably allowed values, but it seems like the content type might be getting in the way when Laravel tries to parse the value for validation?

That might not make sense either as while "false" is not a valid JSON string, it is parse-able to false. but that's moot as the content-type only pertains to the request body. Not the query string.

[laurencei]

This is correct and intended behavior. I actually created the PR boolean validation rule a number of years ago, and this was very specific: "true" as a string is not a boolean.

If the validation allowed it through, then you'll get logic errors in your database if you are storing it as a boolean. i.e. $record->boolean_column = "false" - will record 1 in your DB acting as true, which is not correct.

This is not valid: true,false,0,1,"true","false"
This is valid: true,false,0,1

Your only option is to cast the "true" to true before validating, or create a custom validation that you want to allow "true" as a string through, but understand there are going to be side effects down the line in your code, especially when you use "false" strings (which can be incorrectly cast as true)

[laurencei]

@atrauzzi - there is a solution in this ideas thread for some middleware that you can use, that will solve the problem: https://github.com/laravel/ideas/issues/514#issuecomment-299038674

[atrauzzi]

@laurencei - I understand how types work. As such, I also know that no value in the query string is ever going to be a type other than string. All non-string values in query strings are coerced or interpreted.

1, 0, "0" and "1" are not boolean either.

The expectation of true and not "true" is not really relevant to this discussion as unless you're imposing a requirement that the validator only apply to the request body+content-type pair, I am by definition using it correctly.

I think there's further discussion warranted here as I think a white-list of values would be notably convenient.

[devcircus]

IMO we don't change anything here. This has always been the expected behavior. Great use-case for a simple middleware.

This is also exactly how Symfony validates true/false.

[laurencei]

As such, I also know that no value in the query string is ever going to be a type other than string. All non-string values in query strings are coerced or interpreted.
1, 0, "0" and "1" are not boolean either.

Actually - this is incorrect.

Consider this:

    dd(
        1 == true,      // prints true (correct)
        '1' == true,     // prints true (correct)
        0 == false,      // prints true (correct)
        '0' == false,     // prints true (correct)
        true == true,   // prints true (correct)
        false == false,  // prints true (correct)
        'true' == true,  // prints true (correct)
        'false' == false  // prints false (WRONG)
    );

PHP does accept 0,1 as boolean, and it casts "0" and "1" to boolean when asked to do so.

It wont cast "false" to false - and that is why "false" does not pass a boolean validation.

The main reason for a boolean validation is that you can then use that value further down the line on your DB etc. If we allowed "false" to get through, it would lead to logic errors in other parts of data, and that is why we cant.

But those developers that understand that risk, and know their values, you can override the validation to allow it if needed, that is completely fine.

[atrauzzi]

I know all of this already and still don't think any of it lends to your argument. What is the harm in expanding the validator to interpret "true" and "false" into opposing boolean values?

The value would still have to pass a whitelist, so the concern of most strings bring truthy is moot.

[laurencei]

What is the harm in expanding the validator to interpret "true" and "false" into opposing boolean values?

The main reason for a boolean validation is that you can then use that value further down the line on your DB etc. If we allowed "false" to get through, it would lead to logic errors in other parts of data, and that is why we cant.

Anyway - there's no further discussion here. It's been discussed at length on the https://github.com/laravel/ideas/issues/514

There's also 4-5 closed PRs on this exact issue, which Taylor has rejected all of them for the reasons above. Here's 3 alone:

https://github.com/laravel/framework/pull/19034
https://github.com/laravel/framework/pull/20181
https://github.com/laravel/framework/pull/22243