Skip to content

Commit

Permalink
Simplify constraint usage
Browse files Browse the repository at this point in the history
  • Loading branch information
@MatTheCat
MatTheCat committed Feb 13, 2023
1 parent 25bb90d commit f717ff3
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 75 deletions.
33 changes: 5 additions & 28 deletions src/Symfony/Component/Validator/Constraints/NoSuspiciousCharacters.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,34 +25,22 @@ class NoSuspiciousCharacters extends Constraint
{
public const RESTRICTION_LEVEL_ERROR = '1ece07dc-dca2-45f1-ba47-8d7dc3a12774';
public const INVISIBLE_ERROR = '6ed60e6c-179b-4e93-8a6c-667d85c6de5e';
public const CHAR_LIMIT_ERROR = 'ae6e496e-b315-4fdf-bea5-657accac631d';
public const MIXED_NUMBERS_ERROR = '9f01fc26-3bc4-44b1-a6b1-c08e2412053a';
public const HIDDEN_OVERLAY_ERROR = '56380dc5-0476-4f04-bbaa-b68cd1c2d974';

protected const ERROR_NAMES = [
self::RESTRICTION_LEVEL_ERROR => 'RESTRICTION_LEVEL_ERROR',
self::INVISIBLE_ERROR => 'INVISIBLE_ERROR',
self::CHAR_LIMIT_ERROR => 'CHAR_LIMIT_ERROR',
self::MIXED_NUMBERS_ERROR => 'MIXED_NUMBERS_ERROR',
self::HIDDEN_OVERLAY_ERROR => 'INVALID_CASE_ERROR',
];

/**
* Check that a string satisfies the requirements for the specified restriction level.
* It defaults to {@see self::RESTRICTION_LEVEL_MODERATE} when using ICU >= 58,
* and is locked to {@see self::RESTRICTION_LEVEL_SINGLE_SCRIPT} on older versions.
*/
public const CHECK_RESTRICTION_LEVEL = 16;

/**
* Check a string for the presence of invisible characters such as zero-width spaces,
* or character sequences that are likely not to display such as multiple occurrences of the same non-spacing mark.
*/
public const CHECK_INVISIBLE = 32;

/** Check a string contains only characters from the configured locales. */
public const CHECK_CHAR_LIMIT = 64;

/**
* Check that a string does not mix numbers from different numbering systems;
* for example “8” (Digit Eight) and “৪” (Bengali Digit Four).
Expand Down Expand Up @@ -83,13 +71,12 @@ class NoSuspiciousCharacters extends Constraint
/** @see https://unicode.org/reports/tr39/#unrestricted */
public const RESTRICTION_LEVEL_NONE = 1610612736;

public string $restrictionLevelMessage = 'Restriction level check failed.';
public string $invisibleMessage = 'Invisible check failed.';
public string $charLimitMessage = 'Char limit check failed.';
public string $mixedNumbersMessage = 'Mixed numbers check failed.';
public string $hiddenOverlayMessage = 'Hidden overlay check failed.';
public string $restrictionLevelMessage = 'This value contains characters that are not allowed by the current restriction-level.';
public string $invisibleMessage = 'Using invisible characters is not allowed.';
public string $mixedNumbersMessage = 'Mixing numbers from different scripts is not allowed.';
public string $hiddenOverlayMessage = 'Using hidden overlay characters is not allowed.';

public int $checks = self::CHECK_RESTRICTION_LEVEL | self::CHECK_INVISIBLE | self::CHECK_CHAR_LIMIT | self::CHECK_MIXED_NUMBERS | self::CHECK_HIDDEN_OVERLAY;
public int $checks = self::CHECK_INVISIBLE | self::CHECK_MIXED_NUMBERS | self::CHECK_HIDDEN_OVERLAY;
public ?int $restrictionLevel = null;
public ?array $locales = null;

Expand All @@ -101,7 +88,6 @@ public function __construct(
array $options = null,
string $restrictionLevelMessage = null,
string $invisibleMessage = null,
string $charLimitMessage = null,
string $mixedNumbersMessage = null,
string $hiddenOverlayMessage = null,
int $checks = null,
Expand All @@ -118,19 +104,10 @@ public function __construct(

$this->restrictionLevelMessage ??= $restrictionLevelMessage;
$this->invisibleMessage ??= $invisibleMessage;
$this->charLimitMessage ??= $charLimitMessage;
$this->mixedNumbersMessage ??= $mixedNumbersMessage;
$this->hiddenOverlayMessage ??= $hiddenOverlayMessage;
$this->checks ??= $checks;
$this->restrictionLevel ??= $restrictionLevel;
$this->locales ??= $locales;

if (method_exists(\Spoofchecker::class, 'setRestrictionLevel')) {
$this->restrictionLevel ??= self::RESTRICTION_LEVEL_MODERATE;
} elseif ($this->restrictionLevel && self::RESTRICTION_LEVEL_SINGLE_SCRIPT !== $this->restrictionLevel) {
throw new LogicException('ICU <= 58 only supports single-script restriction level.');
} else {
$this->restrictionLevel = null;
}
}
}
34 changes: 25 additions & 9 deletions src/Symfony/Component/Validator/Constraints/NoSuspiciousCharactersValidator.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

use Symfony\Component\Validator\Constraint;
use Symfony\Component\Validator\ConstraintValidator;
use Symfony\Component\Validator\Exception\LogicException;
use Symfony\Component\Validator\Exception\UnexpectedTypeException;
use Symfony\Component\Validator\Exception\UnexpectedValueException;

Expand All @@ -21,18 +22,22 @@
*/
class NoSuspiciousCharactersValidator extends ConstraintValidator
{
private const CHECK_RESTRICTION_LEVEL = 16;
private const CHECK_SINGLE_SCRIPT = 16;
private const CHECK_CHAR_LIMIT = 64;

private const CHECK_ERROR = [
NoSuspiciousCharacters::CHECK_RESTRICTION_LEVEL => [
self::CHECK_RESTRICTION_LEVEL => [
'code' => NoSuspiciousCharacters::RESTRICTION_LEVEL_ERROR,
'messageProperty' => 'restrictionLevelMessage',
],
NoSuspiciousCharacters::CHECK_INVISIBLE => [
'code' => NoSuspiciousCharacters::INVISIBLE_ERROR,
'messageProperty' => 'invisibleMessage',
],
NoSuspiciousCharacters::CHECK_CHAR_LIMIT => [
'code' => NoSuspiciousCharacters::CHAR_LIMIT_ERROR,
'messageProperty' => 'charLimitMessage',
self::CHECK_CHAR_LIMIT => [
'code' => NoSuspiciousCharacters::RESTRICTION_LEVEL_ERROR,
'messageProperty' => 'restrictionLevelMessage',
],
NoSuspiciousCharacters::CHECK_MIXED_NUMBERS => [
'code' => NoSuspiciousCharacters::MIXED_NUMBERS_ERROR,
Expand Down Expand Up @@ -70,20 +75,31 @@ public function validate(mixed $value, Constraint $constraint)
}

$checker = new \Spoofchecker();

if ($constraint->restrictionLevel) {
$checker->setRestrictionLevel($constraint->restrictionLevel);
$checks = $constraint->checks;

if (method_exists($checker, 'setRestrictionLevel')) {
$checks |= self::CHECK_RESTRICTION_LEVEL;
$checker->setRestrictionLevel($constraint->restrictionLevel ?? NoSuspiciousCharacters::RESTRICTION_LEVEL_MODERATE);
} elseif (NoSuspiciousCharacters::RESTRICTION_LEVEL_MINIMAL === $constraint->restrictionLevel) {
$checks |= self::CHECK_CHAR_LIMIT;
} elseif (NoSuspiciousCharacters::RESTRICTION_LEVEL_SINGLE_SCRIPT === $constraint->restrictionLevel) {
$checks |= self::CHECK_SINGLE_SCRIPT | self::CHECK_CHAR_LIMIT;
} elseif ($constraint->restrictionLevel) {
throw new LogicException('You can only use one of RESTRICTION_LEVEL_NONE, RESTRICTION_LEVEL_MINIMAL or RESTRICTION_LEVEL_SINGLE_SCRIPT with intl compiled against ICU < 58.');
} else {
$checks |= self::CHECK_CHAR_LIMIT;
}

$checker->setAllowedLocales(implode(',', $constraint->locales ?? $this->defaultLocales));

$checker->setChecks($constraint->checks);
$checker->setChecks($checks);

if (!$checker->isSuspicious($value)) {
return;
}

foreach (self::CHECK_ERROR as $check => $error) {
if (!($constraint->checks & $check)) {
if (!($checks & $check)) {
continue;
}

Expand Down
53 changes: 15 additions & 38 deletions src/Symfony/Component/Validator/Tests/Constraints/NoSuspiciousCharactersValidatorTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ class NoSuspiciousCharactersValidatorTest extends ConstraintValidatorTestCase
{
protected function createValidator(): NoSuspiciousCharactersValidator
{
return new NoSuspiciousCharactersValidator(['en']);
return new NoSuspiciousCharactersValidator();
}

/**
Expand All @@ -44,26 +44,13 @@ public static function provideNonSuspiciousStrings(): iterable
['restrictionLevel' => NoSuspiciousCharacters::RESTRICTION_LEVEL_SINGLE_SCRIPT],
];

yield 'Empty profile cannot make CHAR_LIMIT fail' => [
yield 'RESTRICTION_LEVEL_MINIMAL cannot fail without configured locales' => [
'àㄚԱπ৪',
[
'checks' => NoSuspiciousCharacters::CHECK_CHAR_LIMIT,
'restrictionLevel' => NoSuspiciousCharacters::RESTRICTION_LEVEL_MINIMAL,
'locales' => [],
],
];

yield 'Override setAllowedLocales implicit CHAR_LIMIT check' => [
'Ա',
['checks' => NoSuspiciousCharacters::CHECK_HIDDEN_OVERLAY],
];

yield 'Override setRestrictionLevel implicit RESTRICTION_LEVEL check' => [
'à',
[
'checks' => 0,
'restrictionLevel' => NoSuspiciousCharacters::RESTRICTION_LEVEL_ASCII,
],
];
}

/**
Expand All @@ -85,7 +72,7 @@ public static function provideSuspiciousStrings(): iterable
'à',
['restrictionLevel' => NoSuspiciousCharacters::RESTRICTION_LEVEL_ASCII],
NoSuspiciousCharacters::RESTRICTION_LEVEL_ERROR,
'Restriction level check failed.',
'This value contains characters that are not allowed by the current restriction-level.',
];

yield 'Fails RESTRICTION_LEVEL check because of mixed-script string' => [
Expand All @@ -95,37 +82,37 @@ public static function provideSuspiciousStrings(): iterable
'locales' => ['en', 'zh_Hant_TW'],
],
NoSuspiciousCharacters::RESTRICTION_LEVEL_ERROR,
'Restriction level check failed.',
'This value contains characters that are not allowed by the current restriction-level.',
];

yield 'Fails RESTRICTION_LEVEL check because of disallowed Armenian script' => [
yield 'Fails RESTRICTION_LEVEL check because RESTRICTION_LEVEL_HIGH disallows Armenian script' => [
'àԱ',
[
'restrictionLevel' => NoSuspiciousCharacters::RESTRICTION_LEVEL_HIGH,
'locales' => ['en', 'hy_AM'],
],
NoSuspiciousCharacters::RESTRICTION_LEVEL_ERROR,
'Restriction level check failed.',
'This value contains characters that are not allowed by the current restriction-level.',
];

yield 'Fails RESTRICTION_LEVEL check because of disallowed Greek script' => [
yield 'Fails RESTRICTION_LEVEL check because RESTRICTION_LEVEL_MODERATE disallows Greek script' => [
'àπ',
[
'restrictionLevel' => NoSuspiciousCharacters::RESTRICTION_LEVEL_MODERATE,
'locales' => ['en', 'el_GR'],
],
NoSuspiciousCharacters::RESTRICTION_LEVEL_ERROR,
'Restriction level check failed.',
'This value contains characters that are not allowed by the current restriction-level.',
];

yield 'Fails RESTRICTION_LEVEL check because of Greek script absent from profile' => [
yield 'Fails RESTRICTION_LEVEL check because of characters missing from the configured locales’ scripts' => [
'àπ',
[
'checks' => NoSuspiciousCharacters::CHECK_RESTRICTION_LEVEL,
'restrictionLevel' => NoSuspiciousCharacters::RESTRICTION_LEVEL_MINIMAL,
'locales' => ['en'],
],
NoSuspiciousCharacters::RESTRICTION_LEVEL_ERROR,
'Restriction level check failed.',
'This value contains characters that are not allowed by the current restriction-level.',
];

yield 'Fails INVISIBLE check because of duplicated non-spacing mark' => [
Expand All @@ -134,16 +121,7 @@ public static function provideSuspiciousStrings(): iterable
'checks' => NoSuspiciousCharacters::CHECK_INVISIBLE,
],
NoSuspiciousCharacters::INVISIBLE_ERROR,
'Invisible check failed.',
];

yield 'Fails CHAR_LIMIT check because of Greek script absent from profile' => [
'àπ',
[
'checks' => NoSuspiciousCharacters::CHECK_CHAR_LIMIT,
],
NoSuspiciousCharacters::CHAR_LIMIT_ERROR,
'Char limit check failed.',
'Using invisible characters is not allowed.',
];

yield 'Fails MIXED_NUMBERS check because of different numbering systems' => [
Expand All @@ -152,7 +130,7 @@ public static function provideSuspiciousStrings(): iterable
'checks' => NoSuspiciousCharacters::CHECK_MIXED_NUMBERS,
],
NoSuspiciousCharacters::MIXED_NUMBERS_ERROR,
'Mixed numbers check failed.',
'Mixing numbers from different scripts is not allowed.',
];

yield 'Fails HIDDEN_OVERLAY check because of hidden combining character' => [
Expand All @@ -161,14 +139,13 @@ public static function provideSuspiciousStrings(): iterable
'checks' => NoSuspiciousCharacters::CHECK_HIDDEN_OVERLAY,
],
NoSuspiciousCharacters::HIDDEN_OVERLAY_ERROR,
'Hidden overlay check failed.',
'Using hidden overlay characters is not allowed.',
];
}

public function testConstants()
{
$this->assertSame(\Spoofchecker::INVISIBLE, NoSuspiciousCharacters::CHECK_INVISIBLE);
$this->assertSame(\Spoofchecker::CHAR_LIMIT, NoSuspiciousCharacters::CHECK_CHAR_LIMIT);

$this->assertSame(\Spoofchecker::ASCII, NoSuspiciousCharacters::RESTRICTION_LEVEL_ASCII);
$this->assertSame(\Spoofchecker::SINGLE_SCRIPT_RESTRICTIVE, NoSuspiciousCharacters::RESTRICTION_LEVEL_SINGLE_SCRIPT);
Expand Down

0 comments on commit f717ff3

Please sign in to comment.