[8.x] Fixes array to string conversion exception when using custom validation messages for size rules

[mateusjunges]

This PR fixes a Array to string conversion exception when using custom validation messages for size rules.

When using laravel validation, you can specify custom validation messages for attributes using the convention "attribute.rule" to name the lines:

'custom' => [
     'attribute-name' => [
        'rule-name' => 'custom-message',
    ],
],

The problem is: when using size rules, such as Between, if you define your custom message as something like this (i used the email field for this test:

 'custom' => [
   'email' => [
        'between' => [
            'numeric' => 'The :attribute must be between :min and :max.',
            'file' => 'The :attribute must be between :min and :max kilobytes.',
            'string' => 'Some custom message here',
            'array' => 'The :attribute must have between :min and :max items.',
        ]
    ]
],

it will cause an exception:

This occurs because the actual custom message for size rules is returned before checking if the rule being validated is a size rule or not. So, the message passed to the Illuminate\Support\MessageBag transform method is this one:

array:1 [▼
  0 => array:4 [▼
    "numeric" => "The email must be between 20 and 24."
    "file" => "The email must be between 20 and 24 kilobytes."
    "string" => "Some custom message here."
    "array" => "The email must have between 20 and 24 items."
  ]
]

This PR fixes this error by adding a check for size rules before returning the custom validation message to the MessageBag class, and, if it actually is a size rule, get the message for the correct attribute type:

    if ($customMessage !== $customKey) {
+     if (in_array($rule, $this->sizeRules)) {         
+         return $this->getCustomSizeMessage($attribute, $rule);   
+     }
    return $customMessage;
    }

+ protected function getCustomSizeMessage($attribute, $rule)
+ {
+    $lowerRule = Str::snake($rule);
+    $type = $this->getAttributeType($attribute);
+    $key = "validation.custom.{$attribute}.{$lowerRule}.{$type}";
+    return $this->translator->get($key);
+ }

With this fix, the validation returns the correct message, which, in this case is Some custom message here.

[driesvints]

Hmm, I don 't get the array conversion but the entire array for `between is returned instead which indeed seems wrong. I'd expect just one of them to be returned.

    'custom' => [
        'email' => [
            'between' => [
                'numeric' => 'The :attribute must be between :min and :max.',
                'file' => 'The :attribute must be between :min and :max kilobytes.',
                'string' => 'Some custom message here',
                'array' => 'The :attribute must have between :min and :max items.',
            ],
        ],
    ],

Route::get('/test', function () {
    return Validator::make(['email' => 50], ['email' => 'between:10,30'])->errors();
})->name('test');

[mateusjunges]

The array to string conversion comes from the line 269 on Illuminate\Support\MessageBag.php, when laravel replace the message in the string:

framework/src/Illuminate/Support/MessageBag.php

Lines 262 to 271 in a106b93

	protected function transform($messages, $format, $messageKey)
	{
	return collect((array) $messages)
	->map(function ($message) use ($format, $messageKey) {
	// We will simply spin through the given messages and transform each one
	// replacing the :message place holder with the real message allowing
	// the messages to be easily formatted to each developer's desires.
	return str_replace([':message', ':key'], [$message, $messageKey], $format);
	})->all();
	}

[driesvints]

@mateusjatenee why don't I get that error?

[mateusjunges]

I think you are not getting the error because, with

return Validator::make(['email' => 50], ['email' => 'between:10,30'])->errors();

it does not enter the code where the exception is thrown (the line 269 of Illuminate\Support\MessageBag.php file:

framework/src/Illuminate/Support/MessageBag.php

Line 269 in d786e74

return str_replace([':message', ':key'], [$message, $messageKey], $format);

But, if you have your views and try to register a user, for example, with this custom validation message for the email field,
you will get the exception. To be clear, i followed this steps:

• Create a new laravel project
• Installed laravel/jetstream
• Add this custom validation for email:

'custom' => [
    'email' => [
        'between' => [
            'numeric' => 'Test message for numeric',
            'file' => 'Test message for file',
            'string' => 'Test message for string',
            'array' => 'Test message for array',
        ]
    ]
],

• Try to register a new user in my application

[mateusjunges]

@driesvints I think a better solution for this would be, instead of adding a new getCustomSizeMessage method, use the correct $customKey with the getCustomMessageFromTranslator method, switching this lines:

$customMessage = $this->getCustomMessageFromTranslator(
    $customKey = "validation.custom.{$attribute}.{$lowerRule}"
);

by this ones:

$customKey = in_array($rule, $this->sizeRules)
    ? "validation.custom.{$attribute}.{$lowerRule}.{$this->getAttributeType($attribute)}"
    : "validation.custom.{$attribute}.{$lowerRule}";

$customMessage = $this->getCustomMessageFromTranslator($customKey);

I personally like this solution more because i don't have to add new methods to the class. Will change the code in a minute.

[taylorotwell]

Possible to add tests?

[mateusjunges]

@taylorotwell I'll look into it.

[mateusjunges]

@taylorotwell done.

[mateusjunges]

@taylorotwell check if you agree with me here: i think it could be a potential breaking change. Consider this code as an api validating requests and doing some stuff:

Route::get('/test', function (Request $request) {
    $validator = Validator::make(['email' => request('email')], ['email' => 'between:20,40']);

    if ($validator->fails()) {
        return response()->json($validator->errors());
    } else {
        // some logic here
    }
});

since this exception is thrown only when you try to use the validation message in blade views, it returns a wrong validation message to the user, containing all numeric, file, string and array messages, not only the message for the specif attribute type:

{
   "email":[
      {
         "numeric":"Test message for numeric",
         "file":"Test message for file",
         "string":"Test message for string",
         "array":"Test message for array"
      }
   ]
}

if the user uses this response in his code, he will get the correct message with something like this:

$response['email'][0]['string']

This PR will change the return of this validation to this:

{
    "email": [
        "Test message for string"
    ]
}

Now, the user can't get the validation message as before.

What do you think about it?

[taylorotwell]

I honestly don't see a problem at all. Just define your custom rules like so:

There would typically be no need to define one for each type for the same attribute name. An email is never a file or a numeric. It's basically always string. If you have something that is ambiguous just give the field another name in your frontend.

[mateusjunges]

@taylorotwell Sorry but i do think it is a problem. I agree with you that an email is always a string and never a file or a numeric, and maybe i used the wrong attribute name as an example for this PR.

Better examples: if your field is called video, in the frontend, it could be a file, which is the actual video file, or a string, the URL for a video on youtube, for example. A field called image could be a string containing the URL for a particular image on imgur or an image file.

Giving the field another name in the frontend solves this problem, but i think that, since the framework allows the developer to use the same rule name for each field type with it's default messages, the custom messages should also be available that way, and renaming the field is just a workaround for solving the bug.

Any chance that you reconsider this PR?