From 45e202701223306c4dc967b0f4b261afab281cec Mon Sep 17 00:00:00 2001 From: kenjis Date: Sun, 29 Oct 2023 16:57:55 +0900 Subject: [PATCH 1/5] test: add tests for current behavior to matches and differs --- tests/system/Validation/RulesTest.php | 35 ++++++++++--- .../Validation/StrictRules/RulesTest.php | 51 +++++++++++++++++++ 2 files changed, 79 insertions(+), 7 deletions(-) diff --git a/tests/system/Validation/RulesTest.php b/tests/system/Validation/RulesTest.php index 23358d08904b..346b323563e5 100644 --- a/tests/system/Validation/RulesTest.php +++ b/tests/system/Validation/RulesTest.php @@ -290,7 +290,7 @@ public static function providePermitEmpty(): iterable } /** - * @dataProvider provideMatchesCases + * @dataProvider provideMatches */ public function testMatches(array $data, bool $expected): void { @@ -298,12 +298,18 @@ public function testMatches(array $data, bool $expected): void $this->assertSame($expected, $this->validation->run($data)); } - public static function provideMatchesCases(): iterable + public static function provideMatches(): iterable { yield from [ - [['foo' => null, 'bar' => null], true], - [['foo' => 'match', 'bar' => 'match'], true], - [['foo' => 'match', 'bar' => 'nope'], false], + 'foo bar not exist' => [[], false], + 'bar not exist' => [['foo' => null], false], + 'foo not exist' => [['bar' => null], true], // should be false? + 'foo bar null' => [['foo' => null, 'bar' => null], true], + 'foo bar string match' => [['foo' => 'match', 'bar' => 'match'], true], + 'foo bar string not match' => [['foo' => 'match', 'bar' => 'nope'], false], + 'foo bar float match' => [['foo' => 1.2, 'bar' => 1.2], false], // should be true + 'foo bar float not match' => [['foo' => 1.2, 'bar' => 2.3], false], + 'foo bar bool match' => [['foo' => true, 'bar' => true], false], // should be true ]; } @@ -325,12 +331,27 @@ public static function provideMatchesNestedCases(): iterable } /** - * @dataProvider provideMatchesCases + * @dataProvider provideDiffers */ public function testDiffers(array $data, bool $expected): void { $this->validation->setRules(['foo' => 'differs[bar]']); - $this->assertSame(! $expected, $this->validation->run($data)); + $this->assertSame($expected, $this->validation->run($data)); + } + + public static function provideDiffers(): iterable + { + yield from [ + 'foo bar not exist' => [[], false], + 'bar not exist' => [['foo' => null], false], + 'foo not exist' => [['bar' => null], false], + 'foo bar null' => [['foo' => null, 'bar' => null], false], + 'foo bar string match' => [['foo' => 'match', 'bar' => 'match'], false], + 'foo bar string not match' => [['foo' => 'match', 'bar' => 'nope'], true], + 'foo bar float match' => [['foo' => 1.2, 'bar' => 1.2], true], // should be false + 'foo bar float not match' => [['foo' => 1.2, 'bar' => 2.3], true], + 'foo bar bool match' => [['foo' => true, 'bar' => true], true], // should be false + ]; } /** diff --git a/tests/system/Validation/StrictRules/RulesTest.php b/tests/system/Validation/StrictRules/RulesTest.php index 495fa02e20e9..62b7241331a4 100644 --- a/tests/system/Validation/StrictRules/RulesTest.php +++ b/tests/system/Validation/StrictRules/RulesTest.php @@ -198,4 +198,55 @@ public static function provideLessEqualThanStrict(): iterable [true, '0', false], ]; } + + /** + * @dataProvider provideMatches + */ + public function testMatches(array $data, bool $expected): void + { + $this->validation->setRules(['foo' => 'matches[bar]']); + $this->assertSame($expected, $this->validation->run($data)); + } + + public static function provideMatches(): iterable + { + yield from [ + 'foo bar not exist' => [[], false], + 'bar not exist' => [['foo' => null], false], + 'foo not exist' => [['bar' => null], true], + 'foo bar null' => [['foo' => null, 'bar' => null], true], + 'foo bar string match' => [['foo' => 'match', 'bar' => 'match'], true], + 'foo bar string not match' => [['foo' => 'match', 'bar' => 'nope'], false], + // TypeError: CodeIgniter\Validation\Rules::matches(): Argument #1 ($str) must be of type ?string, float given + // 'foo bar float match' => [['foo' => 1.2, 'bar' => 1.2], true], + // TypeError: CodeIgniter\Validation\Rules::matches(): Argument #1 ($str) must be of type ?string, float given + // 'foo bar float not match' => [['foo' => 1.2, 'bar' => 2.3], false], + // TypeError: CodeIgniter\Validation\Rules::matches(): Argument #1 ($str) must be of type ?string, float given + // 'foo bar bool match' => [['foo' => true, 'bar' => true], true], + ]; + } + + /** + * @dataProvider provideDiffers + */ + public function testDiffers(array $data, bool $expected): void + { + $this->validation->setRules(['foo' => 'differs[bar]']); + $this->assertSame($expected, $this->validation->run($data)); + } + + public static function provideDiffers(): iterable + { + yield from [ + 'foo bar not exist' => [[], false], + 'bar not exist' => [['foo' => null], false], + 'foo not exist' => [['bar' => null], false], + 'foo bar null' => [['foo' => null, 'bar' => null], false], + 'foo bar string match' => [['foo' => 'match', 'bar' => 'match'], false], + 'foo bar string not match' => [['foo' => 'match', 'bar' => 'nope'], true], + 'foo bar float match' => [['foo' => 1.2, 'bar' => 1.2], false], + 'foo bar float not match' => [['foo' => 1.2, 'bar' => 2.3], false], + 'foo bar bool match' => [['foo' => true, 'bar' => true], false], + ]; + } } From c06e0b3a3c7889374120ab80f698243f853c8424 Mon Sep 17 00:00:00 2001 From: kenjis Date: Sun, 29 Oct 2023 21:00:36 +0900 Subject: [PATCH 2/5] fix: strict validation rule matches and differs --- system/Validation/StrictRules/Rules.php | 44 ++++++++++++++++--- tests/system/Validation/RulesTest.php | 10 ++--- .../Validation/StrictRules/RulesTest.php | 13 +++--- 3 files changed, 47 insertions(+), 20 deletions(-) diff --git a/system/Validation/StrictRules/Rules.php b/system/Validation/StrictRules/Rules.php index 0887eff28ada..70d081975a62 100644 --- a/system/Validation/StrictRules/Rules.php +++ b/system/Validation/StrictRules/Rules.php @@ -36,13 +36,26 @@ public function __construct() * @param array|bool|float|int|object|string|null $str * @param array $data Other field/value pairs */ - public function differs($str, string $field, array $data): bool - { - if (! is_string($str)) { + public function differs( + $str, + string $otherField, + array $data, + ?string $error = null, + ?string $field = null + ): bool { + if (strpos($otherField, '.') !== false) { + return $str !== dot_array_search($otherField, $data); + } + + if (! array_key_exists($field, $data)) { return false; } - return $this->nonStrictRules->differs($str, $field, $data); + if (! array_key_exists($otherField, $data)) { + return false; + } + + return $str !== ($data[$otherField] ?? null); } /** @@ -254,9 +267,26 @@ public function less_than_equal_to($str, string $max): bool * @param array|bool|float|int|object|string|null $str * @param array $data Other field/value pairs */ - public function matches($str, string $field, array $data): bool - { - return $this->nonStrictRules->matches($str, $field, $data); + public function matches( + $str, + string $otherField, + array $data, + ?string $error = null, + ?string $field = null + ): bool { + if (strpos($otherField, '.') !== false) { + return $str === dot_array_search($otherField, $data); + } + + if (! array_key_exists($field, $data)) { + return false; + } + + if (! array_key_exists($otherField, $data)) { + return false; + } + + return $str === ($data[$otherField] ?? null); } /** diff --git a/tests/system/Validation/RulesTest.php b/tests/system/Validation/RulesTest.php index 346b323563e5..601be2d0e7ca 100644 --- a/tests/system/Validation/RulesTest.php +++ b/tests/system/Validation/RulesTest.php @@ -303,13 +303,13 @@ public static function provideMatches(): iterable yield from [ 'foo bar not exist' => [[], false], 'bar not exist' => [['foo' => null], false], - 'foo not exist' => [['bar' => null], true], // should be false? + 'foo not exist' => [['bar' => null], true], // Strict Rule: false 'foo bar null' => [['foo' => null, 'bar' => null], true], 'foo bar string match' => [['foo' => 'match', 'bar' => 'match'], true], 'foo bar string not match' => [['foo' => 'match', 'bar' => 'nope'], false], - 'foo bar float match' => [['foo' => 1.2, 'bar' => 1.2], false], // should be true + 'foo bar float match' => [['foo' => 1.2, 'bar' => 1.2], false], // Strict Rule: true 'foo bar float not match' => [['foo' => 1.2, 'bar' => 2.3], false], - 'foo bar bool match' => [['foo' => true, 'bar' => true], false], // should be true + 'foo bar bool match' => [['foo' => true, 'bar' => true], false], // Strict Rule: true ]; } @@ -348,9 +348,9 @@ public static function provideDiffers(): iterable 'foo bar null' => [['foo' => null, 'bar' => null], false], 'foo bar string match' => [['foo' => 'match', 'bar' => 'match'], false], 'foo bar string not match' => [['foo' => 'match', 'bar' => 'nope'], true], - 'foo bar float match' => [['foo' => 1.2, 'bar' => 1.2], true], // should be false + 'foo bar float match' => [['foo' => 1.2, 'bar' => 1.2], true], // Strict Rule: false 'foo bar float not match' => [['foo' => 1.2, 'bar' => 2.3], true], - 'foo bar bool match' => [['foo' => true, 'bar' => true], true], // should be false + 'foo bar bool match' => [['foo' => true, 'bar' => true], true], // Strict Rule: false ]; } diff --git a/tests/system/Validation/StrictRules/RulesTest.php b/tests/system/Validation/StrictRules/RulesTest.php index 62b7241331a4..74ed4d6cecd6 100644 --- a/tests/system/Validation/StrictRules/RulesTest.php +++ b/tests/system/Validation/StrictRules/RulesTest.php @@ -213,16 +213,13 @@ public static function provideMatches(): iterable yield from [ 'foo bar not exist' => [[], false], 'bar not exist' => [['foo' => null], false], - 'foo not exist' => [['bar' => null], true], + 'foo not exist' => [['bar' => null], false], 'foo bar null' => [['foo' => null, 'bar' => null], true], 'foo bar string match' => [['foo' => 'match', 'bar' => 'match'], true], 'foo bar string not match' => [['foo' => 'match', 'bar' => 'nope'], false], - // TypeError: CodeIgniter\Validation\Rules::matches(): Argument #1 ($str) must be of type ?string, float given - // 'foo bar float match' => [['foo' => 1.2, 'bar' => 1.2], true], - // TypeError: CodeIgniter\Validation\Rules::matches(): Argument #1 ($str) must be of type ?string, float given - // 'foo bar float not match' => [['foo' => 1.2, 'bar' => 2.3], false], - // TypeError: CodeIgniter\Validation\Rules::matches(): Argument #1 ($str) must be of type ?string, float given - // 'foo bar bool match' => [['foo' => true, 'bar' => true], true], + 'foo bar float match' => [['foo' => 1.2, 'bar' => 1.2], true], + 'foo bar float not match' => [['foo' => 1.2, 'bar' => 2.3], false], + 'foo bar bool match' => [['foo' => true, 'bar' => true], true], ]; } @@ -245,7 +242,7 @@ public static function provideDiffers(): iterable 'foo bar string match' => [['foo' => 'match', 'bar' => 'match'], false], 'foo bar string not match' => [['foo' => 'match', 'bar' => 'nope'], true], 'foo bar float match' => [['foo' => 1.2, 'bar' => 1.2], false], - 'foo bar float not match' => [['foo' => 1.2, 'bar' => 2.3], false], + 'foo bar float not match' => [['foo' => 1.2, 'bar' => 2.3], true], 'foo bar bool match' => [['foo' => true, 'bar' => true], false], ]; } From c83e2ea981d5a8814749e64f7c2ebe03894c565c Mon Sep 17 00:00:00 2001 From: kenjis Date: Mon, 30 Oct 2023 06:39:42 +0900 Subject: [PATCH 3/5] fix: traditional validation rule matches and differs Match CI3 behaviros. --- system/Validation/Rules.php | 12 +++++++----- tests/system/Validation/RulesTest.php | 12 ++++++------ 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/system/Validation/Rules.php b/system/Validation/Rules.php index b7c585f56c30..6a809dc534cc 100644 --- a/system/Validation/Rules.php +++ b/system/Validation/Rules.php @@ -24,9 +24,10 @@ class Rules /** * The value does not match another field in $data. * - * @param array $data Other field/value pairs + * @param string|null $str + * @param array $data Other field/value pairs */ - public function differs(?string $str, string $field, array $data): bool + public function differs($str, string $field, array $data): bool { if (strpos($field, '.') !== false) { return $str !== dot_array_search($field, $data); @@ -177,15 +178,16 @@ public function less_than_equal_to(?string $str, string $max): bool /** * Matches the value of another field in $data. * - * @param array $data Other field/value pairs + * @param string|null $str + * @param array $data Other field/value pairs */ - public function matches(?string $str, string $field, array $data): bool + public function matches($str, string $field, array $data): bool { if (strpos($field, '.') !== false) { return $str === dot_array_search($field, $data); } - return array_key_exists($field, $data) && $str === $data[$field]; + return isset($data[$field]) ? ($str === $data[$field]) : false; } /** diff --git a/tests/system/Validation/RulesTest.php b/tests/system/Validation/RulesTest.php index 601be2d0e7ca..ac2f34bf825a 100644 --- a/tests/system/Validation/RulesTest.php +++ b/tests/system/Validation/RulesTest.php @@ -303,13 +303,13 @@ public static function provideMatches(): iterable yield from [ 'foo bar not exist' => [[], false], 'bar not exist' => [['foo' => null], false], - 'foo not exist' => [['bar' => null], true], // Strict Rule: false - 'foo bar null' => [['foo' => null, 'bar' => null], true], + 'foo not exist' => [['bar' => null], false], + 'foo bar null' => [['foo' => null, 'bar' => null], false], // Strict Rule: true 'foo bar string match' => [['foo' => 'match', 'bar' => 'match'], true], 'foo bar string not match' => [['foo' => 'match', 'bar' => 'nope'], false], - 'foo bar float match' => [['foo' => 1.2, 'bar' => 1.2], false], // Strict Rule: true + 'foo bar float match' => [['foo' => 1.2, 'bar' => 1.2], true], 'foo bar float not match' => [['foo' => 1.2, 'bar' => 2.3], false], - 'foo bar bool match' => [['foo' => true, 'bar' => true], false], // Strict Rule: true + 'foo bar bool match' => [['foo' => true, 'bar' => true], true], ]; } @@ -348,9 +348,9 @@ public static function provideDiffers(): iterable 'foo bar null' => [['foo' => null, 'bar' => null], false], 'foo bar string match' => [['foo' => 'match', 'bar' => 'match'], false], 'foo bar string not match' => [['foo' => 'match', 'bar' => 'nope'], true], - 'foo bar float match' => [['foo' => 1.2, 'bar' => 1.2], true], // Strict Rule: false + 'foo bar float match' => [['foo' => 1.2, 'bar' => 1.2], false], 'foo bar float not match' => [['foo' => 1.2, 'bar' => 2.3], true], - 'foo bar bool match' => [['foo' => true, 'bar' => true], true], // Strict Rule: false + 'foo bar bool match' => [['foo' => true, 'bar' => true], false], ]; } From fdd9ec88d8e267db6d3245e3c5255642268b0252 Mon Sep 17 00:00:00 2001 From: kenjis Date: Mon, 30 Oct 2023 06:53:20 +0900 Subject: [PATCH 4/5] refactor: by rector --- system/Validation/Rules.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/system/Validation/Rules.php b/system/Validation/Rules.php index 6a809dc534cc..eae6d961ce7d 100644 --- a/system/Validation/Rules.php +++ b/system/Validation/Rules.php @@ -187,7 +187,7 @@ public function matches($str, string $field, array $data): bool return $str === dot_array_search($field, $data); } - return isset($data[$field]) ? ($str === $data[$field]) : false; + return isset($data[$field]) && $str === $data[$field]; } /** From db0ad7e22f28587f9ec062a4859bb4af42e26bd4 Mon Sep 17 00:00:00 2001 From: kenjis Date: Thu, 2 Nov 2023 16:57:20 +0900 Subject: [PATCH 5/5] docs: add changelog and upgrade --- user_guide_src/source/changelogs/v4.4.4.rst | 6 ++++++ user_guide_src/source/installation/upgrade_444.rst | 11 +++++++++++ 2 files changed, 17 insertions(+) diff --git a/user_guide_src/source/changelogs/v4.4.4.rst b/user_guide_src/source/changelogs/v4.4.4.rst index cfb0aeb4f121..bb4250f1e63f 100644 --- a/user_guide_src/source/changelogs/v4.4.4.rst +++ b/user_guide_src/source/changelogs/v4.4.4.rst @@ -21,6 +21,12 @@ A validation rule with the wildcard ``*`` now validates only data in correct dimensions as "dot array syntax". See :ref:`Upgrading ` for details. +Validation rules matches and differs +==================================== + +Bugs have been fixed in the case where ``matches`` and ``differs`` in the Strict +and Traditional rules validate data of non-string types. + *************** Message Changes *************** diff --git a/user_guide_src/source/installation/upgrade_444.rst b/user_guide_src/source/installation/upgrade_444.rst index c0b4def6e34d..b3773947efd9 100644 --- a/user_guide_src/source/installation/upgrade_444.rst +++ b/user_guide_src/source/installation/upgrade_444.rst @@ -39,6 +39,17 @@ The following code explains details: If you have code that depends on the bug, fix the the rule key. +Validation rules matches and differs +==================================== + +Because bugs have been fixed in the case where ``matches`` and ``differs`` in +the Strict and Traditional rules validate data of non-string types, if you are +using these rules and validate non-string data, the validation results might be +changed (fixed). + +Note that Traditional Rules should not be used to validate data that is not a +string. + ********************* Breaking Enhancements *********************