Validation: empty string passes the `array` rule

[danilopolani]

• Laravel Version: 9.5.1
• PHP Version: 8.1.3
• Database Driver & Version: Not relevant

Description:

Using an empty string as value for the array rule passes but it should not.

Note: this is an intentional duplicate of #18948, but since that issue has never been solved I decided to open a new one. In that issue there was a discussion about ConvertEmptyStringsToNull but that's incorrect, since we could use the validation outside of HTTP requests.

Steps To Reproduce:

\Validator::make(['field' => ''], ['field' => 'present|array'])->fails(); // false, WRONG

\Validator::make(['field' => null], ['field' => 'present|array'])->fails(); // true, correct
\Validator::make(['field' => 0], ['field' => 'present|array'])->fails(); // true, correct
\Validator::make(['field' => false], ['field' => 'present|array'])->fails(); // true, correct

[driesvints]

This is expected since this is considered an empty value. You're not requiring the value and thus the validation passes.

[danilopolani]

@driesvints yes, but it's required to be an array, or am I getting it wrong? I thought this was the correct way to say "it must be an array, but it can also be an empty array".

Or how should the rules become to achieve that?

[driesvints]

There doesn't seems to be a way atm to require an empty array.

[ElteHupkes]

@driesvints I may have to open a new issue for this, but I'd like to point out that this issue isn't limited to empty arrays, but to essentially every validation rule when passing in an empty string. Let's take the following test:

$validator = Validator::make(
        data: [
            'field' => '  ',
            'field2' => 1,
        ],
        rules: [
            'field' => ['present', 'integer', 'min:10', 'same:field2']
        ]
    );
    $this->assertTrue($validator->fails());

This test fails, that is, validation passes.

I could've added any number of other arbitrary rules to it, but I think the point is clear. Any empty or whitespace string will cause any non-implicit rule to fail because it isn't considered validatable.

It is my impression that the validator basically assumes you have the TrimStrings and ConvertEmptyStringsToNull middleware enabled, which works around these issues (at least for incoming requests). I had these turned off (whitespace strings are not an invalid scenario for me) and that opened up this can of worms.

I don't know if "any whitespace string is acceptable for any non-required field" is expected behavior, but for someone (me) new to Laravel it was very much unexpected.

[driesvints]

@ElMatella the example you provided seems correct to me. You're passing an empty string and not requiring the value. So the validation passes. If I add "required" to the rules it will fail. So this is expected. We'd welcome PR's to clarify this in the docs!

[ElteHupkes]

@driesvints I'll make sure to do that once I have a slightly better idea what I'm talking about and where such a piece of information belongs. Clearly my thoughts coming in are a bit different than how it's actually designed, so I'm a bit hesitant about telling people what's up now 😉.

What sent me down the rabbit hole is that I was trying to create a rule that passes:

• ['foreign_id' => 123] (field is an integer)
• ['foreign_id' => null] (field is null)
• [] (field not specified)

required and filled are out the window, because they ignore nullable. All non-implicit rules aren't evaluated for whitespace strings. Without turning the middleware back on, I could only solve this with a custom rule, which seemed a bit odd to me.

[liquid207]

@ElteHupkes
I quite agree with you.
Now Validator can work normally on the basis that the data has been processed by middleware ConvertEmptyStringsToNull. But, not all Laravel application have ConvertEmptyStringsToNull enabled; Validator doesn't just process the data in the request that processed by ConvertEmptyStringsToNull.

If the special treatment of empty string is removed, this issue will disappear. @driesvints Do you think this modification is ok?

[driesvints]

No I don't sorry. This has been in the framework since forever and modifying it could break tons of apps. I'm sorry about this but we're not changing this.

[liquid207]

@driesvints

Got it, thanks.

[macropay-solutions]

We just had a production incident about this BUG. Empty string is not validated.

'currency' => 'sometimes|size:3|in:EUR,USD',

    protected function isValidatable($rule, $attribute, $value)
    {
        if (in_array($rule, $this->excludeRules)) {
            return true;
        }

        return $this->presentOrRuleIsImplicit($rule, $attribute, $value) && // this returns false

    protected function presentOrRuleIsImplicit($rule, $attribute, $value)
    {
        if (is_string($value) && trim($value) === '') {
            return $this->isImplicit($rule); // returns from here false 
        }

        return $this->validatePresent($attribute, $value) ||
               $this->isImplicit($rule);
    }

The field should be validated only when present so this is clearly a bug.

Asking for the user to specify that it must be required via 'sometimes|required|size:3|in:EUR,USD' is nonsence since the size and the in rules should be applied.

We are thinking of removing $this->presentOrRuleIsImplicit($rule, $attribute, $value) call to fix this by overwriting the validator because we have it overwritten for the latest security bug anyway.

Update
or just remove this code

        if (is_string($value) && trim($value) === '') {
            return $this->isImplicit($rule);
        }