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

Array keys validation issue #48854

Closed
andreich1980 opened this issue Oct 30, 2023 · 6 comments
Closed

Array keys validation issue #48854

andreich1980 opened this issue Oct 30, 2023 · 6 comments

Comments

@andreich1980
Copy link
Contributor

Laravel Version

10.29.0

PHP Version

8.1.13

Database Driver & Version

No response

Description

Wrong validation message when validating array keys.

I use example from the docs https://laravel.com/docs/10.x/validation#rule-array

Steps To Reproduce

use Illuminate\Support\Facades\Validator;
 
$input = [
    'user' => [
        'name' => 'Taylor Otwell',
        'username' => 'taylorotwell',
        'admin' => true,
    ],
];
 
Validator::make($input, [
    'user' => 'array:name,username',
])->validate();

This snippet gives me validation message: The user must be an array.
But user IS an array.

According to the docs,

each key in the input array must be present within the list of values provided to the rule. In the following example, the admin key in the input array is invalid since it is not contained in the list of values provided to the array rule.

So I expected validation message about invalid/unexpected key admin instead.

Also, validated() method returns all the array keys, including admin, even though it's not within the expected array keys in the rule:

use Illuminate\Support\Facades\Validator;

$input = [
    'user' => [
        'name' => 'Taylor Otwell',
        'username' => 'taylorotwell',
        'admin' => true,
    ],
];

Validator::make($input, [
    'user' => 'array:name,username',
])->validated();

Output:

[
    "user" => [
      "name" => "Taylor Otwell",
      "username" => "taylorotwell",
      "admin" => true, // <-- why is it here?
    ],
  ]
@github-actions
Copy link

Thank you for reporting this issue!

As Laravel is an open source project, we rely on the community to help us diagnose and fix issues as it is not possible to research and fix every issue reported to us via GitHub.

If possible, please make a pull request fixing the issue you have described, along with corresponding tests. All pull requests are promptly reviewed by the Laravel team.

Thank you!

@renmichaela
Copy link

I found the logic for validating arrays in Illuminate\Validation\Concerns\ValidatesAttributes:

    public function validateArray($attribute, $value, $parameters = [])
    {
        if (! is_array($value)) {
            return false;
        }

        if (empty($parameters)) {
            return true;
        }

        return empty(array_diff_key($value, array_fill_keys($parameters, '')));
    }

The way it's written, it expects the input array to contain exactly the same keys as what is in the rule, no more and no less. So even though the input contains the keys specified by the rule, array_diff_key is still returning the extra admin key causing this method to return false.

@Guxinpei
Copy link

Guxinpei commented Nov 1, 2023

Hello, @jakesuellentrop

Thank you for providing detailed insights. I'd like to further elaborate on your observation regarding the behavior of the array_diff_key function. To illustrate this, I conducted a simple test:

$emptyKeyDiff = empty(array_diff_key(['foo'=>'','bar'=>'','baz'=>''], ['foo'=>'','bar'=>''])); // Result is false
$otherEmptyKeyDiff = empty(array_diff_key(['foo'=>'','bar'=>''],['foo'=>'','bar'=>'','baz'=>''])); // Result is true

From the above code, it's evident that when the input array contains extra keys, array_diff_key will return those extra keys, leading the validation to fail.

This indeed raises a potential concern where the message "The user must be an array." might not be adequately descriptive. A more appropriate message could be: "The input array contains keys not specified in the rule." or another more descriptive prompt.Just a thought!

I hope this provides a clearer understanding of the issue at hand, and it would be great to see a more descriptive error message in future iterations. Thank you again for your feedback!

@renmichaela
Copy link

renmichaela commented Nov 1, 2023

array_diff_key(array $array, array ...$arrays): array

Returns an array containing all the entries from array whose keys are absent from all of the other arrays.

So if the arguments were switched around when calling array_diff_key, then it would return an array of all the keys from $parameters that were not present in $value.

@Guxinpei
Copy link

Guxinpei commented Nov 1, 2023

Hi , @jakesuellentrop

You're absolutely right based on the logic you've pointed out; that's how Laravel currently seems to function. However, after referring to the Laravel docs, I'm pondering if perhaps the behavior should slightly differ.

That being said, it's just a personal thought, and I might be missing some nuances. I'd love to hear any insights or suggestions you might have on this. It's great collaborating and contributing to the Laravel community together.

Best regards,

@driesvints
Copy link
Member

Thanks all. It seems this is the expected behaviour. If you want a different behaviour you could write your own custom validation rule.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants