minor #54062 [Validator] Simplify NoSuspiciousCharactersValidator (…

…MatTheCat) This PR was merged into the 7.0 branch. Discussion ---------- [Validator] Simplify `NoSuspiciousCharactersValidator` | Q | A | ------------- | --- | Branch? | 7.0 | Bug fix? | no | New feature? | no | Deprecations? | no | Issues | N/A | License | MIT php/php-src#10647 has been fixed in PHP 8.1.17. Now that Symfony requires PHP ≥ 8.2, we can avoid calling `Spoofchecker::isSuspicious` for every check by leveraging its `$errorCode` parameter. Commits ------- c24e3e7 [Validator] Simplify `NoSuspiciousCharactersValidator`
symfony · Feb 27, 2024 · f8611cf · f8611cf
2 parents 8debf88 + c24e3e7
commit f8611cf
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 29 deletions.
diff --git a/src/Symfony/Component/Validator/Constraints/NoSuspiciousCharactersValidator.php b/src/Symfony/Component/Validator/Constraints/NoSuspiciousCharactersValidator.php
@@ -18,7 +18,7 @@
 use Symfony\Component\Validator\Exception\UnexpectedValueException;
 
 /**
- * @author Mathieu Lechat <mathieu.lechat@les-tilleuls.coop>
+ * @author Mathieu Lechat <math.lechat@gmail.com>
  */
 class NoSuspiciousCharactersValidator extends ConstraintValidator
 {
@@ -94,18 +94,12 @@ public function validate(mixed $value, Constraint $constraint): void
 
         $checker->setChecks($checks);
 
-        if (!$checker->isSuspicious($value)) {
+        if (!$checker->isSuspicious($value, $errorCode)) {
             return;
         }
 
         foreach (self::CHECK_ERROR as $check => $error) {
-            if (!($checks & $check)) {
-                continue;
-            }
-
-            $checker->setChecks($check);
-
-            if (!$checker->isSuspicious($value)) {
+            if (!($errorCode & $check)) {
                 continue;
             }
 

diff --git a/src/Symfony/Component/Validator/Tests/Constraints/NoSuspiciousCharactersValidatorTest.php b/src/Symfony/Component/Validator/Tests/Constraints/NoSuspiciousCharactersValidatorTest.php
@@ -56,23 +56,31 @@ public static function provideNonSuspiciousStrings(): iterable
     /**
      * @dataProvider provideSuspiciousStrings
      */
-    public function testSuspiciousStrings(string $string, array $options, string $errorCode, string $errorMessage)
+    public function testSuspiciousStrings(string $string, array $options, array $errors)
     {
         $this->validator->validate($string, new NoSuspiciousCharacters($options));
 
-        $this->buildViolation($errorMessage)
-            ->setCode($errorCode)
+        $violations = $this->buildViolation(reset($errors))
+            ->setCode(key($errors))
             ->setParameter('{{ value }}', '"'.$string.'"')
-            ->assertRaised();
+        ;
+
+        while ($message = next($errors)) {
+            $violations = $violations->buildNextViolation($message)
+                ->setCode(key($errors))
+                ->setParameter('{{ value }}', '"'.$string.'"')
+            ;
+        }
+
+        $violations->assertRaised();
     }
 
     public static function provideSuspiciousStrings(): iterable
     {
         yield 'Fails RESTRICTION_LEVEL check because of character outside ASCII range' => [
             'à',
             ['restrictionLevel' => NoSuspiciousCharacters::RESTRICTION_LEVEL_ASCII],
-            NoSuspiciousCharacters::RESTRICTION_LEVEL_ERROR,
-            'This value contains characters that are not allowed by the current restriction-level.',
+            [NoSuspiciousCharacters::RESTRICTION_LEVEL_ERROR => 'This value contains characters that are not allowed by the current restriction-level.'],
         ];
 
         yield 'Fails RESTRICTION_LEVEL check because of mixed-script string' => [
@@ -81,8 +89,7 @@ public static function provideSuspiciousStrings(): iterable
                 'restrictionLevel' => NoSuspiciousCharacters::RESTRICTION_LEVEL_SINGLE_SCRIPT,
                 'locales' => ['en', 'zh_Hant_TW'],
             ],
-            NoSuspiciousCharacters::RESTRICTION_LEVEL_ERROR,
-            'This value contains characters that are not allowed by the current restriction-level.',
+            [NoSuspiciousCharacters::RESTRICTION_LEVEL_ERROR => 'This value contains characters that are not allowed by the current restriction-level.'],
         ];
 
         yield 'Fails RESTRICTION_LEVEL check because RESTRICTION_LEVEL_HIGH disallows Armenian script' => [
@@ -91,8 +98,7 @@ public static function provideSuspiciousStrings(): iterable
                 'restrictionLevel' => NoSuspiciousCharacters::RESTRICTION_LEVEL_HIGH,
                 'locales' => ['en', 'hy_AM'],
             ],
-            NoSuspiciousCharacters::RESTRICTION_LEVEL_ERROR,
-            'This value contains characters that are not allowed by the current restriction-level.',
+            [NoSuspiciousCharacters::RESTRICTION_LEVEL_ERROR => 'This value contains characters that are not allowed by the current restriction-level.'],
         ];
 
         yield 'Fails RESTRICTION_LEVEL check because RESTRICTION_LEVEL_MODERATE disallows Greek script' => [
@@ -101,8 +107,7 @@ public static function provideSuspiciousStrings(): iterable
                 'restrictionLevel' => NoSuspiciousCharacters::RESTRICTION_LEVEL_MODERATE,
                 'locales' => ['en', 'el_GR'],
             ],
-            NoSuspiciousCharacters::RESTRICTION_LEVEL_ERROR,
-            'This value contains characters that are not allowed by the current restriction-level.',
+            [NoSuspiciousCharacters::RESTRICTION_LEVEL_ERROR => 'This value contains characters that are not allowed by the current restriction-level.'],
         ];
 
         yield 'Fails RESTRICTION_LEVEL check because of characters missing from the configured locales’ scripts' => [
@@ -111,35 +116,43 @@ public static function provideSuspiciousStrings(): iterable
                 'restrictionLevel' => NoSuspiciousCharacters::RESTRICTION_LEVEL_MINIMAL,
                 'locales' => ['en'],
             ],
-            NoSuspiciousCharacters::RESTRICTION_LEVEL_ERROR,
-            'This value contains characters that are not allowed by the current restriction-level.',
+            [NoSuspiciousCharacters::RESTRICTION_LEVEL_ERROR => 'This value contains characters that are not allowed by the current restriction-level.'],
         ];
 
         yield 'Fails INVISIBLE check because of duplicated non-spacing mark' => [
             'à̀',
             [
                 'checks' => NoSuspiciousCharacters::CHECK_INVISIBLE,
             ],
-            NoSuspiciousCharacters::INVISIBLE_ERROR,
-            'Using invisible characters is not allowed.',
+            [NoSuspiciousCharacters::INVISIBLE_ERROR => 'Using invisible characters is not allowed.'],
         ];
 
         yield 'Fails MIXED_NUMBERS check because of different numbering systems' => [
             '8৪',
             [
                 'checks' => NoSuspiciousCharacters::CHECK_MIXED_NUMBERS,
             ],
-            NoSuspiciousCharacters::MIXED_NUMBERS_ERROR,
-            'Mixing numbers from different scripts is not allowed.',
+            [NoSuspiciousCharacters::MIXED_NUMBERS_ERROR => 'Mixing numbers from different scripts is not allowed.'],
         ];
 
         yield 'Fails HIDDEN_OVERLAY check because of hidden combining character' => [
             'i̇',
             [
                 'checks' => NoSuspiciousCharacters::CHECK_HIDDEN_OVERLAY,
             ],
-            NoSuspiciousCharacters::HIDDEN_OVERLAY_ERROR,
-            'Using hidden overlay characters is not allowed.',
+            [NoSuspiciousCharacters::HIDDEN_OVERLAY_ERROR => 'Using hidden overlay characters is not allowed.'],
+        ];
+
+        yield 'Fails both HIDDEN_OVERLAY and RESTRICTION_LEVEL checks' => [
+            'i̇',
+            [
+                'checks' => NoSuspiciousCharacters::CHECK_HIDDEN_OVERLAY,
+                'restrictionLevel' => NoSuspiciousCharacters::RESTRICTION_LEVEL_ASCII,
+            ],
+            [
+                NoSuspiciousCharacters::RESTRICTION_LEVEL_ERROR => 'This value contains characters that are not allowed by the current restriction-level.',
+                NoSuspiciousCharacters::HIDDEN_OVERLAY_ERROR => 'Using hidden overlay characters is not allowed.',
+            ],
         ];
     }