[6.x] Type hinted arguments for Illuminate\Validation\Rules\RequiredIf

[0xcrypto]

Fixes a security issue as explained in https://www.huntr.dev/bounties/3-laravel/framework/

If the argument to Illuminate\Validation\Rules\RequiredIf is not boolean or callable, it will throw InvalidArgumentException.

Added tests to check

1. that the provided input is either a callable or a boolean (as it was hinted in docblock)
2. that the provided callable is not serializable.

[0xcrypto]

Thank you @taylorotwell, can you please validate the report and patch on huntr.dev as well (https://www.huntr.dev/bounties/3-laravel/framework/). It helps me earn a living for the contribution I do to open source projects.

[nuernbergerA]

This test would lead to false/positives.

expectException will succeed if one of the 3 rules hit
it would be better to test with a data provider or separate tests.

[0xcrypto]

oh my bad, will fix it.

[nuernbergerA]

not sure what the security issues was, I assume it's related to __toString

I consider this as a breaking change because before it was possible to have something like this

Validator::make($request->all(), [
    'role_id' => Rule::requiredIf(optional($request->user())->is_admin),
]);

This will fail now if it's null

[0xcrypto]

not sure what the security issues was, I assume it's related to __toString

I consider this as a breaking change because before it was possible to have something like this

Validator::make($request->all(), [
    'role_id' => Rule::requiredIf(optional($request->user())->is_admin),
]);

This will fail now if it's null

I agree this is a breaking change if people are using null to pass as false. But as the docblock says, the function only accepts a callable or boolean. I am unsure if allowing null would be better here or following the arguments description in the docblock. Maybe cast the null to boolean before passing? Or rather do it inside the function?

[ankurk91]

Maybe cast the null to boolean before passing? Or rather do it inside the function?

Yes, i think it is ok to cast null to bool

if(!is_callable($condition)  || ! (bool)$condition){

}

[0xcrypto]

if(!is_callable($condition) || ! (bool)$condition){

or maybe just check for argument not being a string as any literal other than string would make is_callable return false anyway.

if (! is_string($condition)) {
    $this->condition = $condition;
}