Array validation error message is off-by-one

[dmg-aimmerman]

Laravel Version

10 (Any version which supports array validation)

PHP Version

8.1 (Any)

Database Driver & Version

MySQL 8.0 - Issue is database agnostic

Description

When displaying array validation (e.g. emails.*) error messages to the user, the error message is incorrect & misleading due to the start index of 0. If the second email is invalid, the error message indicates the first email is invalid, so-on.

For example, given the following comma-seperated emails (to be exploded before validation), "[email protected], invalid_email, [email protected]", will produce the error message "The emails.1 must be a valid email address."; where the expected result to the end-user should be "The emails.2 must be a valid email address." - or better yet, a human-readable output "The 2nd email must be a valid email address".

Steps To Reproduce

User/QA Steps

• Populate an email array with 3 values, where the second value is invalid (e.g. [email protected], invalid_email, [email protected])
• Submit
• Observe error message incorrectly indicates that the first email address is invalid

Developer Steps

$emails = explode(',', '[email protected], invalid_email, [email protected]');

$result = Validator::make(
    [ 'email' => $emails ],
    ['email.*' => 'email']
);

echo $result->errors()->first();

[calebdw]

https://laravel.com/docs/10.x/validation#error-message-indexes-and-positions

[dmg-aimmerman]

Thank you @calebdw; but this bug report is referring to the default behavior, not custom error messages

[calebdw]

@dmg-aimmerman, there's no bug, the validation uses :index so emails.1 is the second position. If you want to change this then you need to use a custom error message with :position.

[dmg-aimmerman]

@calebdw, The bug I'm referring to is exactly what you just mentioned, "the validation uses :index". These error messages are presented to the end-user in plain English without indicating that counting starts at 0. I know computers may start counting at 0; but no end-user does. So, the message will always be incorrect and misleading to the intended consumer (the end-user).

[calebdw]

Note that instead of writing custom validation messages, you can just change the attribute:

Validator::make(
    // ...
    ['emails.*' => 'Email #:position'],
);

[dmg-aimmerman]

@calebdw, Thanks, what are your thoughts on fixing the default in Laravel? What's case(s), if any, does showing the user a number starting at 0 make more sense?

I only ask because I've seen this issue come up several times with different developers/projects/companies (usually reported as a defect during QA or UAT), but I can't think of any scenario where the desired result would be to see an ordinal number starting at 0 (as current behavior)

Thank you!

[calebdw]

I'd probably be fine with a default of 'email.*' => 'email #:position', not sure how Laravel feels about it

I also don't like having to update every array validation to make it user friendly. I can submit a PR if no one beats me to it

[driesvints]

Not sure I see a bug here sorry.

[dmg-aimmerman]

@driesvints This is a textbook off-by-one-error so I'm not sure what you're missing?

This problem could arise when a programmer makes mistakes such as ..., or fails to take into account that a sequence starts at zero rather than one

https://en.wikipedia.org/wiki/Off-by-one_error

[driesvints]

@dmg-aimmerman if you could send in a PR with a failing test for us to look at, then we can work from there.

[dmg-aimmerman]

@driesvints Thank you, please see PR 47771

Also, perhaps it will help to see the original issue as reported by our quality assurance team:

Please note we have had other reports with the same root-cause.

Here is output from the failing test:

There was 1 failure:

1. Illuminate\Tests\Validation\ValidationValidatorTest::testArrayValidationMessagesAreAccurate
   Failed asserting that two strings are identical.
   --- Expected
   +++ Actual
   @@ @@
   -'The emails.2 field must be a valid email address.'
   +'The emails.1 field must be a valid email address.'

[driesvints]

I'll reopen this but it might be a while before I get to it.

[driesvints]

I reviewed this again but I don't think we ever intended it to work like this. The wildcard is meant to be used in combination with keyed items, not index based arrays. We don't even have a code example in the docs. So right now, we're not going to take action here, sorry.