From 1d155099bd39184706f49665abab688588da38fc Mon Sep 17 00:00:00 2001 From: James Brooks Date: Thu, 10 Feb 2022 12:47:53 +0000 Subject: [PATCH 01/12] Add failing test for `forEach` validation using regex --- tests/Validation/ValidationForEachTest.php | 26 ++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/tests/Validation/ValidationForEachTest.php b/tests/Validation/ValidationForEachTest.php index 876b9d0987b3..c15950a76d57 100644 --- a/tests/Validation/ValidationForEachTest.php +++ b/tests/Validation/ValidationForEachTest.php @@ -200,6 +200,32 @@ public function testForEachCallbacksCanReturnDifferentRules() 'items.1.discounts.0.discount' => ['validation.numeric'], ], $v->getMessageBag()->toArray()); } + + public function testNestedCallbacksDoNotBreakRegexRules() + { + $data = [ + 'items' => [ + ['users' => [['type' => 'super'], ['type' => 'admin']]], + ], + ]; + + $rules = [ + 'items.*' => Rule::forEach(function () { + return ['users.*.type' => 'regex:/^(super|admin)$/i']; + }), + ]; + + $trans = $this->getIlluminateArrayTranslator(); + + $v = new Validator($trans, $data, $rules); + + $this->assertFalse($v->passes()); + + $this->assertEquals([ + 'items.0.users.0.type' => ['validation.regex'], + 'items.0.users.1.type' => ['validation.regex'], + ], $v->getMessageBag()->toArray()); + } protected function getTranslator() { From 8256e57d239f2836ecb250d896357ce32364c3d0 Mon Sep 17 00:00:00 2001 From: StyleCI Bot Date: Thu, 10 Feb 2022 12:48:25 +0000 Subject: [PATCH 02/12] Apply fixes from StyleCI --- tests/Validation/ValidationForEachTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Validation/ValidationForEachTest.php b/tests/Validation/ValidationForEachTest.php index c15950a76d57..6a02cb88f3d9 100644 --- a/tests/Validation/ValidationForEachTest.php +++ b/tests/Validation/ValidationForEachTest.php @@ -200,7 +200,7 @@ public function testForEachCallbacksCanReturnDifferentRules() 'items.1.discounts.0.discount' => ['validation.numeric'], ], $v->getMessageBag()->toArray()); } - + public function testNestedCallbacksDoNotBreakRegexRules() { $data = [ From c28499e4d0169b4c413eba1be8abca0e31d19ce9 Mon Sep 17 00:00:00 2001 From: Steve Bauman Date: Thu, 10 Feb 2022 10:33:05 -0500 Subject: [PATCH 03/12] Wrap rule before processing and fix test case --- src/Illuminate/Validation/ValidationRuleParser.php | 2 +- tests/Validation/ValidationForEachTest.php | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/Illuminate/Validation/ValidationRuleParser.php b/src/Illuminate/Validation/ValidationRuleParser.php index 4895f922614c..8ced0d7f530e 100644 --- a/src/Illuminate/Validation/ValidationRuleParser.php +++ b/src/Illuminate/Validation/ValidationRuleParser.php @@ -156,7 +156,7 @@ protected function explodeWildcardRules($results, $attribute, $rules) } else { $this->implicitAttributes[$attribute][] = $key; - $results = $this->mergeRules($results, $key, $rule); + $results = $this->mergeRules($results, $key, Arr::wrap($rule)); } } } diff --git a/tests/Validation/ValidationForEachTest.php b/tests/Validation/ValidationForEachTest.php index 6a02cb88f3d9..e3834ff3b71e 100644 --- a/tests/Validation/ValidationForEachTest.php +++ b/tests/Validation/ValidationForEachTest.php @@ -205,7 +205,7 @@ public function testNestedCallbacksDoNotBreakRegexRules() { $data = [ 'items' => [ - ['users' => [['type' => 'super'], ['type' => 'admin']]], + ['users' => [['type' => 'super'], ['type' => 'invalid']]], ], ]; @@ -222,7 +222,6 @@ public function testNestedCallbacksDoNotBreakRegexRules() $this->assertFalse($v->passes()); $this->assertEquals([ - 'items.0.users.0.type' => ['validation.regex'], 'items.0.users.1.type' => ['validation.regex'], ], $v->getMessageBag()->toArray()); } From b16297e38f31b0cd4b63fb183b62419050623a44 Mon Sep 17 00:00:00 2001 From: Steve Bauman Date: Thu, 10 Feb 2022 10:44:56 -0500 Subject: [PATCH 04/12] Don't explode regex rules --- src/Illuminate/Validation/ValidationRuleParser.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Illuminate/Validation/ValidationRuleParser.php b/src/Illuminate/Validation/ValidationRuleParser.php index 8ced0d7f530e..2230d8332547 100644 --- a/src/Illuminate/Validation/ValidationRuleParser.php +++ b/src/Illuminate/Validation/ValidationRuleParser.php @@ -85,7 +85,7 @@ protected function explodeRules($rules) protected function explodeExplicitRule($rule, $attribute) { if (is_string($rule)) { - return explode('|', $rule); + return explode('|', $rule, ...array_filter([str_contains($rule, 'regex')])); } elseif (is_object($rule)) { return Arr::wrap($this->prepareRule($rule, $attribute)); } @@ -156,7 +156,7 @@ protected function explodeWildcardRules($results, $attribute, $rules) } else { $this->implicitAttributes[$attribute][] = $key; - $results = $this->mergeRules($results, $key, Arr::wrap($rule)); + $results = $this->mergeRules($results, $key, $rule); } } } From b3e60851110552c0554de390b7c63d9d8a0bba93 Mon Sep 17 00:00:00 2001 From: Steve Bauman Date: Thu, 10 Feb 2022 11:08:57 -0500 Subject: [PATCH 05/12] Refactor determination of regex validation rule --- .../Validation/ValidationRuleParser.php | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/Illuminate/Validation/ValidationRuleParser.php b/src/Illuminate/Validation/ValidationRuleParser.php index 2230d8332547..addc7ac09e8d 100644 --- a/src/Illuminate/Validation/ValidationRuleParser.php +++ b/src/Illuminate/Validation/ValidationRuleParser.php @@ -85,7 +85,9 @@ protected function explodeRules($rules) protected function explodeExplicitRule($rule, $attribute) { if (is_string($rule)) { - return explode('|', $rule, ...array_filter([str_contains($rule, 'regex')])); + [$name] = static::parseStringRule($rule); + + return explode('|', $rule, ...array_filter([static::ruleIsRegex($name)])); } elseif (is_object($rule)) { return Arr::wrap($this->prepareRule($rule, $attribute)); } @@ -272,15 +274,24 @@ protected static function parseStringRule($rule) */ protected static function parseParameters($rule, $parameter) { - $rule = strtolower($rule); - - if (in_array($rule, ['regex', 'not_regex', 'notregex'], true)) { + if (static::ruleIsRegex($rule)) { return [$parameter]; } return str_getcsv($parameter); } + /** + * Determine if the rule is a regular expression. + * + * @param string $rule + * @return bool + */ + protected static function ruleIsRegex($rule) + { + return in_array(strtolower($rule), ['regex', 'not_regex', 'notregex'], true); + } + /** * Normalizes a rule so that we can accept short types. * From 7fc4d442c0b75c3bf7e1f1ebaa91cf492bf9319f Mon Sep 17 00:00:00 2001 From: Steve Bauman Date: Thu, 10 Feb 2022 11:26:51 -0500 Subject: [PATCH 06/12] Add more previously failing test cases --- tests/Validation/ValidationForEachTest.php | 33 +++++++++++++++++++- tests/Validation/ValidationValidatorTest.php | 3 ++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/tests/Validation/ValidationForEachTest.php b/tests/Validation/ValidationForEachTest.php index e3834ff3b71e..c836844680b5 100644 --- a/tests/Validation/ValidationForEachTest.php +++ b/tests/Validation/ValidationForEachTest.php @@ -201,7 +201,7 @@ public function testForEachCallbacksCanReturnDifferentRules() ], $v->getMessageBag()->toArray()); } - public function testNestedCallbacksDoNotBreakRegexRules() + public function testForEachCallbacksDoNotBreakRegexRules() { $data = [ 'items' => [ @@ -226,6 +226,37 @@ public function testNestedCallbacksDoNotBreakRegexRules() ], $v->getMessageBag()->toArray()); } + public function testForEachCallbacksCanContainMultipleRegexRules() + { + $data = [ + 'items' => [ + ['users' => [['type' => 'super'], ['type' => 'invalid']]], + ], + ]; + + $rules = [ + 'items.*' => Rule::forEach(function () { + return ['users.*.type' => [ + 'regex:/^(super)$/i', + 'notregex:/^(invalid)$/i', + ]]; + }), + ]; + + $trans = $this->getIlluminateArrayTranslator(); + + $v = new Validator($trans, $data, $rules); + + $this->assertFalse($v->passes()); + + $this->assertEquals([ + 'items.0.users.1.type' => [ + 'validation.regex', + 'validation.notregex', + ], + ], $v->getMessageBag()->toArray()); + } + protected function getTranslator() { return m::mock(TranslatorContract::class); diff --git a/tests/Validation/ValidationValidatorTest.php b/tests/Validation/ValidationValidatorTest.php index 113c01f521a3..a0bf005441fa 100755 --- a/tests/Validation/ValidationValidatorTest.php +++ b/tests/Validation/ValidationValidatorTest.php @@ -3820,6 +3820,9 @@ public function testValidateRegex() $v = new Validator($trans, ['x' => 12], ['x' => 'Regex:/^12$/i']); $this->assertTrue($v->passes()); + + $v = new Validator($trans, ['x' => ['y' => ['z' => 'james']]], ['x.*.z' => ['Regex:/^(taylor|james)$/i']]); + $this->assertTrue($v->passes()); } public function testValidateNotRegex() From befe010dbaf115dd4d99edb17e44490081d6acb3 Mon Sep 17 00:00:00 2001 From: Steve Bauman Date: Thu, 10 Feb 2022 11:32:02 -0500 Subject: [PATCH 07/12] Remove unneeded elseif --- src/Illuminate/Validation/ValidationRuleParser.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Illuminate/Validation/ValidationRuleParser.php b/src/Illuminate/Validation/ValidationRuleParser.php index addc7ac09e8d..d277d5254b2c 100644 --- a/src/Illuminate/Validation/ValidationRuleParser.php +++ b/src/Illuminate/Validation/ValidationRuleParser.php @@ -88,7 +88,9 @@ protected function explodeExplicitRule($rule, $attribute) [$name] = static::parseStringRule($rule); return explode('|', $rule, ...array_filter([static::ruleIsRegex($name)])); - } elseif (is_object($rule)) { + } + + if (is_object($rule)) { return Arr::wrap($this->prepareRule($rule, $attribute)); } From 8738167e5d9422e7126772ff1858f682ce145593 Mon Sep 17 00:00:00 2001 From: Steve Bauman Date: Thu, 10 Feb 2022 11:45:16 -0500 Subject: [PATCH 08/12] Add more tests with an expected failure test --- tests/Validation/ValidationRuleParserTest.php | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/tests/Validation/ValidationRuleParserTest.php b/tests/Validation/ValidationRuleParserTest.php index c54f56278f3b..b8cca9726f5d 100644 --- a/tests/Validation/ValidationRuleParserTest.php +++ b/tests/Validation/ValidationRuleParserTest.php @@ -89,6 +89,42 @@ public function testEmptyConditionalRulesArePreserved() ], $rules); } + public function testExplodeProperlyParsesSingleRegexRule() + { + $data = ['items' => [['type' => 'foo']]]; + + $exploded = (new ValidationRuleParser($data))->explode( + ['items.*.type' => 'regex:/^(super|admin)$/i'] + ); + + $this->assertEquals('regex:/^(super|admin)$/i', $exploded->rules['items.0.type'][0]); + } + + public function testExplodeProperlyParsesRegexWithArrayOfRules() + { + $data = ['items' => [['type' => 'foo']]]; + + $exploded = (new ValidationRuleParser($data))->explode( + ['items.*.type' => ['in:foo', 'regex:/^(super|admin)$/i']] + ); + + $this->assertEquals('in:foo', $exploded->rules['items.0.type'][0]); + $this->assertEquals('regex:/^(super|admin)$/i', $exploded->rules['items.0.type'][1]); + } + + public function testExplodeFailsParsingRegexWithOtherRulesInSingleString() + { + $data = ['items' => [['type' => 'foo']]]; + + $exploded = (new ValidationRuleParser($data))->explode( + ['items.*.type' => 'in:foo|regex:/^(super|admin)$/i'] + ); + + $this->assertEquals('in:foo', $exploded->rules['items.0.type'][0]); + $this->assertEquals('regex:/^(super', $exploded->rules['items.0.type'][1]); + $this->assertEquals('admin)$/i', $exploded->rules['items.0.type'][2]); + } + public function testExplodeGeneratesNestedRules() { $parser = (new ValidationRuleParser([ From aa8f9d6e10af60faf7d4a6207b2969cf50804bf2 Mon Sep 17 00:00:00 2001 From: Steve Bauman Date: Thu, 10 Feb 2022 12:09:00 -0500 Subject: [PATCH 09/12] Add another test case and unify other tests --- tests/Validation/ValidationRuleParserTest.php | 26 ++++++++++++++----- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/tests/Validation/ValidationRuleParserTest.php b/tests/Validation/ValidationRuleParserTest.php index b8cca9726f5d..7490b5150b43 100644 --- a/tests/Validation/ValidationRuleParserTest.php +++ b/tests/Validation/ValidationRuleParserTest.php @@ -94,10 +94,10 @@ public function testExplodeProperlyParsesSingleRegexRule() $data = ['items' => [['type' => 'foo']]]; $exploded = (new ValidationRuleParser($data))->explode( - ['items.*.type' => 'regex:/^(super|admin)$/i'] + ['items.*.type' => 'regex:/^(foo|bar)$/i'] ); - $this->assertEquals('regex:/^(super|admin)$/i', $exploded->rules['items.0.type'][0]); + $this->assertEquals('regex:/^(foo|bar)$/i', $exploded->rules['items.0.type'][0]); } public function testExplodeProperlyParsesRegexWithArrayOfRules() @@ -105,11 +105,23 @@ public function testExplodeProperlyParsesRegexWithArrayOfRules() $data = ['items' => [['type' => 'foo']]]; $exploded = (new ValidationRuleParser($data))->explode( - ['items.*.type' => ['in:foo', 'regex:/^(super|admin)$/i']] + ['items.*.type' => ['in:foo', 'regex:/^(foo|bar)$/i']] ); $this->assertEquals('in:foo', $exploded->rules['items.0.type'][0]); - $this->assertEquals('regex:/^(super|admin)$/i', $exploded->rules['items.0.type'][1]); + $this->assertEquals('regex:/^(foo|bar)$/i', $exploded->rules['items.0.type'][1]); + } + + public function testExplodeProperlyParsesRegexThatDoesNotContainPipe() + { + $data = ['items' => [['type' => 'foo']]]; + + $exploded = (new ValidationRuleParser($data))->explode( + ['items.*.type' => 'in:foo|regex:/^(bar)$/i'] + ); + + $this->assertEquals('in:foo', $exploded->rules['items.0.type'][0]); + $this->assertEquals('regex:/^(bar)$/i', $exploded->rules['items.0.type'][1]); } public function testExplodeFailsParsingRegexWithOtherRulesInSingleString() @@ -117,12 +129,12 @@ public function testExplodeFailsParsingRegexWithOtherRulesInSingleString() $data = ['items' => [['type' => 'foo']]]; $exploded = (new ValidationRuleParser($data))->explode( - ['items.*.type' => 'in:foo|regex:/^(super|admin)$/i'] + ['items.*.type' => 'in:foo|regex:/^(foo|bar)$/i'] ); $this->assertEquals('in:foo', $exploded->rules['items.0.type'][0]); - $this->assertEquals('regex:/^(super', $exploded->rules['items.0.type'][1]); - $this->assertEquals('admin)$/i', $exploded->rules['items.0.type'][2]); + $this->assertEquals('regex:/^(foo', $exploded->rules['items.0.type'][1]); + $this->assertEquals('bar)$/i', $exploded->rules['items.0.type'][2]); } public function testExplodeGeneratesNestedRules() From ae6b101abbf4665396a860e52537c2ea7c3fd6c8 Mon Sep 17 00:00:00 2001 From: Steve Bauman Date: Thu, 10 Feb 2022 15:01:14 -0500 Subject: [PATCH 10/12] Simplify string rule parsing --- src/Illuminate/Validation/ValidationRuleParser.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Illuminate/Validation/ValidationRuleParser.php b/src/Illuminate/Validation/ValidationRuleParser.php index d277d5254b2c..3f7fc2de39b6 100644 --- a/src/Illuminate/Validation/ValidationRuleParser.php +++ b/src/Illuminate/Validation/ValidationRuleParser.php @@ -87,7 +87,7 @@ protected function explodeExplicitRule($rule, $attribute) if (is_string($rule)) { [$name] = static::parseStringRule($rule); - return explode('|', $rule, ...array_filter([static::ruleIsRegex($name)])); + return static::ruleIsRegex($name) ? [$rule] : explode('|', $rule); } if (is_object($rule)) { From 703bc37358fce694ae84a307329b8357a61072b4 Mon Sep 17 00:00:00 2001 From: Taylor Otwell Date: Thu, 10 Feb 2022 15:47:36 -0600 Subject: [PATCH 11/12] formatting --- src/Illuminate/Validation/ValidationRuleParser.php | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/Illuminate/Validation/ValidationRuleParser.php b/src/Illuminate/Validation/ValidationRuleParser.php index 3f7fc2de39b6..653cbd6bd59d 100644 --- a/src/Illuminate/Validation/ValidationRuleParser.php +++ b/src/Illuminate/Validation/ValidationRuleParser.php @@ -276,11 +276,7 @@ protected static function parseStringRule($rule) */ protected static function parseParameters($rule, $parameter) { - if (static::ruleIsRegex($rule)) { - return [$parameter]; - } - - return str_getcsv($parameter); + return static::ruleIsRegex($rule) ? [$parameter] : str_getcsv($parameter); } /** From 860b923a3563bfb4f962bb44c3015a255cb24402 Mon Sep 17 00:00:00 2001 From: Steve Bauman Date: Fri, 11 Feb 2022 09:26:48 -0500 Subject: [PATCH 12/12] Add test for flattening rule arrays of arrays --- tests/Validation/ValidationRuleParserTest.php | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/Validation/ValidationRuleParserTest.php b/tests/Validation/ValidationRuleParserTest.php index 7490b5150b43..49b36dbf168f 100644 --- a/tests/Validation/ValidationRuleParserTest.php +++ b/tests/Validation/ValidationRuleParserTest.php @@ -137,6 +137,18 @@ public function testExplodeFailsParsingRegexWithOtherRulesInSingleString() $this->assertEquals('bar)$/i', $exploded->rules['items.0.type'][2]); } + public function testExplodeProperlyFlattensRuleArraysOfArrays() + { + $data = ['items' => [['type' => 'foo']]]; + + $exploded = (new ValidationRuleParser($data))->explode( + ['items.*.type' => ['in:foo', [[['regex:/^(foo|bar)$/i']]]]] + ); + + $this->assertEquals('in:foo', $exploded->rules['items.0.type'][0]); + $this->assertEquals('regex:/^(foo|bar)$/i', $exploded->rules['items.0.type'][1]); + } + public function testExplodeGeneratesNestedRules() { $parser = (new ValidationRuleParser([