[9.x] Patch regex rule parsing due to Rule::forEach()

[stevebauman]

Description

This PR fixes #40924.

This also allows the ability to supply a regex validation rule inside of a single string-based rule, instead of requiring it to be an array.

Before:

This would previously fail in Laravel:

$data = ['items' => [['type' => 'admin']]]

// Fails. preg_match throws malformed regex exception.
$rules = ['items.*' => 'regex:/^(super|admin)$/i'];

// Passes.
$rules = ['items.*' => ['regex:/^(super|admin)$/i']];

Validator::make($data, $rules)->passes();

After:

$data = ['items' => [['type' => 'admin']]]

// Passes.
$rules = ['items.*' => 'regex:/^(super|admin)$/i'];

// Passes.
$rules = ['items.*' => ['regex:/^(super|admin)$/i']];

Validator::make($data, $rules)->passes();

Notes

I've added various test cases to ensure parsing is done properly. However, maybe we should introduce an exception when preg_match() throws an exception due to a malformed regex which would indicate to the developer they must wrap the regex rule in an array when using multiple rules?

Let me know your thoughts. If you like the idea, I will update my PR to include this exception.

[stevebauman]

I've used the spread operator to completely omit the third ($limit) parameter if the rule is not regex.

If the rule is not a regex:

1. false will be returned from ruleIsRegex()
2. array_filter will then clear false out of the array, resulting in an empty array
3. Spread operator will do nothing, as there are no elements of the array

I've added this, because explode() will take a null, false, or 0 parameter and set the $limit to 1.

We want $limit to be 1 when the rule is a regular expression, or $limit to be completely omitted when the rule is not.

[taylorotwell]

Can we adjust this code to actually explicitly pass an integer to explode?

[stevebauman]

Yes absolutely, one moment.

[stevebauman]

I've simplified it to:

return static::ruleIsRegex($name) ? [$rule] : explode('|', $rule);

Since the explode('|', $rule, 1) will simply wrap the rule in an array. Let me know if you want me to change this. If I wanted to explicitly pass in an integer value, I'd have to use PHP_INT_MAX for the default value (which looked a bit off -- not sure of your opinion on this):

return explode('|', $rule, static::ruleIsRegex($name) ? 1 : PHP_INT_MAX);

[stevebauman]

Hmm, a random test is failing. If we execute another run, we should be green 👍

[taylorotwell]

I'm guessing this still doesn't work?

'attribute' => 'required|string|regex:/^(super|admin)$/i|another_rule|final_rule`,

[stevebauman]

Yes that's correct -- the rule will fail to parse properly. I've added a test case so we have some visibility on this:

https://github.com/stevebauman/framework/blob/ae6b101abbf4665396a860e52537c2ea7c3fd6c8/tests/Validation/ValidationRuleParserTest.php#L127-L138

[taylorotwell]

@stevebauman can you explain how your original PR actually introduced this bug? I'm not sure I quite understand how and what broke.

[stevebauman]

@taylorotwell

I introduced this bug by using Arr::flatten() on this line:

framework/src/Illuminate/Validation/ValidationRuleParser.php

Line 145 in 4a474ce

foreach (Arr::flatten((array) $rules) as $rule) {

The structure of $rules given to explodeExplicitRule() was changed:

// Previously:
^ array:1 [
  0 => array:2 [
    0 => "in:foo"
    1 => "regex:/^(foo|bar)$/i"
  ]
]

// With Arr::flatten()
^ array:2 [
  0 => "in:foo"
  1 => "regex:/^(foo|bar)$/i"
]

The arrays were previously multi-dimensional and are now single dimensional.

This means, the first is_string() condition is now being hit on explodeExplicitRule(), instead of the last prepareRule array map, since this method will be called for each rule in the array:

framework/src/Illuminate/Validation/ValidationRuleParser.php

Lines 85 to 97 in 4a474ce

	protected function explodeExplicitRule($rule, $attribute)
	{
	if (is_string($rule)) {
	return explode('|', $rule);
	} elseif (is_object($rule)) {
	return Arr::wrap($this->prepareRule($rule, $attribute));
	}
	return array_map(
	[$this, 'prepareRule'],
	$rule,
	array_fill(array_key_first($rule), count($rule), $attribute)
	);

This causes the explode to inadvertently target the regex rules, when it did not previously.

For Rule::forEach() to work, I had to flatten the attributes $rules so that I could check each rule if it's an instance of NestedRules. Without flatten(), we would have to cycle over the rules again to determine if they contain any Rule::forEach() instances, and would not be able to parse nested Rule::forEach()'s without recursion.