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

Added support for "true" and "false" text values to the Boolean validation rule for compatibility with FormData #43955

Closed

Conversation

andrey-helldar
Copy link
Contributor

@andrey-helldar andrey-helldar commented Aug 31, 2022

Very often, data is sent from the web interface along with the file.

Unfortunately, you can only send a file through JS using FormData, and it has its own characteristics and limitations. For example, it can only transfer strings:

formData.append('name', "John");   // "John"
formData.append('age', 72);        // "72"
formData.append('is_admin', true); // "true"
formData.append('avatar', (binary));

At this point, we are faced with another problem - the validation of a boolean value:

public function rules(): array
{
    return [
        // ...
        'is_admin' => ['required', 'bool']
    ];
}

Because "true" and "false" are strings of 4 and 5 characters respectively, not booleans.

For this reason, in the above request, the validator will return the message:

The :attribute field must be true or false.

Of course, you can use the filter_var function when checking, but then another problem arises - it defines null values as false:

filter_var(true, FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE);    // true
filter_var(false, FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE);   // false
filter_var('true', FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE);  // true
filter_var('false', FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE); // false
filter_var('yes', FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE);   // true
filter_var('no', FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE);    // false
filter_var(null, FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE);    // false
filter_var($obj, FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE);    // null
filter_var([], FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE);      // null

Therefore, it is better not to use it, because. often it is necessary to catch exactly null value.

That is why I propose to extend the existing variable definition method by adding string values for true and false to it.

Since defining string values changes behavior, I consider these changes to be backwards incompatible.

At the moment, you have to insert a crutch in the form of a custom rule. In my opinion, this is wrong, given the presence of the rule in the "box".

class BooleanRule implements Rule
{
    public function passes($attribute, $value): bool
    {
        $acceptable = [true, false, 'true', 'false', 0, 1, '0', '1'];

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

    public function message()
    {
        return trans('validation.boolean');
    }
}

class CustomRequest
{
    public function rules(): array
    {
        // ...
        'is_admin' => ['required', new BooleanRule()]
    }
}

@andrey-helldar andrey-helldar changed the title Added support for "true" and "false" text options for compatibility with FormData Added support for "true" and "false" text values for compatibility with FormData Aug 31, 2022
@andrey-helldar andrey-helldar force-pushed the patch/2022-08-31/23-00 branch 2 times, most recently from 0e2d331 to 7c812ad Compare August 31, 2022 20:55
@igorgaming
Copy link

Please see previous attempts, for example: #40041

@andrey-helldar andrey-helldar changed the title Added support for "true" and "false" text values for compatibility with FormData Added support for "true" and "false" text values to the Boolean validation rule for compatibility with FormData Aug 31, 2022
@igorgaming
Copy link

igorgaming commented Aug 31, 2022

@andrey-helldar This example just illustrates possible breaking changes.
If the validation rule guarantees that there is a bool, then the developer believes that an expression like if ($val) will always work correctly. If this PR is accepted, this entry will no longer work for "false" and "true". Validation MUST ensure that the value is bool.
You can also pay attention to another example with a database that will affect many more applications and I think it will only complicate the life of developers (the bool rule will no longer mean anything for sure).
You can use 1 of 0 instead of true or false in FormData.

P.s: and no, its comparison for boolean value, not an empty string.
If you try this:

$var = '0';
if ($var) {
    echo 'true';
} else {
    echo 'false';
}

than you see the 'false'. The string is not empty

@andrey-helldar
Copy link
Contributor Author

andrey-helldar commented Aug 31, 2022

@igorgaming, the developer must determine what goes into the database.

From the frontend side, the string equivalent comes to the API. Yes, this is a shortcoming of the JS FormData technology.

And if everything you say had a weight, the filter_var function would also not return values like 'true', 'false', 'yes', 'no', 'on', 'off', '' and null as boolean.

See: https://www.php.net/manual/en/filter.filters.validate.php

@igorgaming
Copy link

igorgaming commented Aug 31, 2022

@andrey-helldar So, are you suggesting that the developer should do an additional check on bool before the data puts into the database?
Then the meaning of the bool validation rule will be completely lost.
filter_var is used for other things, you should understand that this change can break too many application, but frontend can easily put 0 or 1 instead of true or false.

Also see: https://www.php.net/manual/en/language.types.boolean.php#language.types.boolean.casting

@andrey-helldar
Copy link
Contributor Author

andrey-helldar commented Aug 31, 2022

@igorgaming,

public function __invoke(UpdateRequest $request)
{
    $is_admin = $request->boolean('is_admin'); // boolean value `true` or `false`

    // ...
}
/**
 * Retrieve input as a boolean value.
 *
 * Returns true when value is "1", "true", "on", and "yes". Otherwise, returns false.
 *
 * @param  string|null  $key
 * @param  bool  $default
 * @return bool
 */
public function boolean($key = null, $default = false)
{
    return filter_var($this->input($key, $default), FILTER_VALIDATE_BOOLEAN);
}

See: https://github.com/laravel/framework/blob/9.x/src/Illuminate/Http/Concerns/InteractsWithInput.php#L320-L323

@taylorotwell
Copy link
Member

taylorotwell commented Sep 1, 2022

I suggest just creating a new Rule::boolean method that creates a Boolean rule class which can accept additional options for values that should be allowed:

Rule::boolean(['true', 'false'])

Feel free to send a PR for that if you wish.

@igorgaming
Copy link

igorgaming commented Sep 1, 2022

@andrey-helldar ofc in this case its ok, because filter_var returns true or false. In Laravel, bool validation rule just check the value, doesnt change it.
Taylor said that you can do, this is the best option for all of us.

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

Successfully merging this pull request may close these issues.

3 participants