Skip to content

[Validator] Unexpected side effect when applying Assert/Cidr to array #57301

Closed
@imba28

Description

@imba28

Symfony version(s) affected

7.1

Description

Hello there,

after upgrading our application from 7.0 to 7.1, a few API tests started breaking. After digging through the code I could pin-point the problem to the following lines: https://github.com/symfony/symfony/blob/7.2/src/Symfony/Component/Validator/Constraints/CidrValidator.php#L74-L76 The code mentioned above overrides the upper bound of the netmask. Now, that's no problem when validating a single scalar value. However, it introduces a side-effect which results in unexpected behaviour when the underlying constraint #[Assert/Cidr] is reused. For example, when applying the constraint to an array of IP addresses.

How to reproduce

Imagine an endpoint that expects a request formatted according to the following DTO:

final class Dto
{
    /**
     * @var string[]
     */
    #[Assert\All(new Assert\Cidr)]
    public array $items;
}

Upon receiving

  • {"items":["1.2.3.4/28"]} as expected, there's no validation error.
  • {"items":["::1/64", "1.2.3.4/28"]} the IPv6 is validated first. No validation error either. Still good.
  • {"items":["1.2.3.4/28", "::1/64"]} Symfony triggers a validation error complaining about ::1/64's net mask being larger than 32. This is unexpected since new Assert\Cidr does not explicitly define an upper bound.

I created the following project that hopefully helps to reproduce the bug. https://github.com/imba28/symfony-validator-cidr-bug

Possible Solution

Off the top of my head, a naive solution would be to store the upper bound of the netmask in a local variable instead of overriding $constraint->netmaskMax. This should ensure the underlying constraint is not modified and can be applied repeatedly without any side effects.

diff --git a/src/Symfony/Component/Validator/Constraints/CidrValidator.php b/src/Symfony/Component/Validator/Constraints/CidrValidator.php
index 4fc78a7828..bfc0ba4c52 100644
--- a/src/Symfony/Component/Validator/Constraints/CidrValidator.php
+++ b/src/Symfony/Component/Validator/Constraints/CidrValidator.php
@@ -71,15 +71,16 @@ class CidrValidator extends ConstraintValidator
             return;
         }
 
-        if (filter_var($ipAddress, \FILTER_VALIDATE_IP, \FILTER_FLAG_IPV4) && $constraint->netmaskMax > 32) {
-            $constraint->netmaskMax = 32;
+        $netmaskMax = $constraint->netmaskMax;
+        if (filter_var($ipAddress, \FILTER_VALIDATE_IP, \FILTER_FLAG_IPV4) && $netmaskMax > 32) {
+            $netmaskMax = 32;
         }
 
-        if ($netmask < $constraint->netmaskMin || $netmask > $constraint->netmaskMax) {
+        if ($netmask < $constraint->netmaskMin || $netmask > $netmaskMax) {
             $this->context
                 ->buildViolation($constraint->netmaskRangeViolationMessage)
                 ->setParameter('{{ min }}', $constraint->netmaskMin)
-                ->setParameter('{{ max }}', $constraint->netmaskMax)
+                ->setParameter('{{ max }}', $netmaskMax)
                 ->setCode(Cidr::OUT_OF_RANGE_ERROR)
                 ->addViolation();
         }

Additional Context

No response

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions