bug #29839 [Validator] fix comparisons with null values at property paths (xabbuh)

xabbuh · xabbuh · commit a073606ea80b · 2019-12-16T09:55:48.000+01:00
This PR was merged into the 3.4 branch. Discussion ---------- [Validator] fix comparisons with null values at property paths | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #29831 | License | MIT | Doc PR | Commits ------- ffa5db7 fix comparisons with null values at property paths
diff --git a/src/Symfony/Component/Validator/Constraints/GreaterThanOrEqualValidator.php b/src/Symfony/Component/Validator/Constraints/GreaterThanOrEqualValidator.php
@@ -24,7 +24,7 @@ class GreaterThanOrEqualValidator extends AbstractComparisonValidator
      */
     protected function compareValues($value1, $value2)
     {
-        return $value1 >= $value2;
+        return null === $value2 || $value1 >= $value2;
     }
 
     /**
diff --git a/src/Symfony/Component/Validator/Constraints/GreaterThanValidator.php b/src/Symfony/Component/Validator/Constraints/GreaterThanValidator.php
@@ -24,7 +24,7 @@ class GreaterThanValidator extends AbstractComparisonValidator
      */
     protected function compareValues($value1, $value2)
     {
-        return $value1 > $value2;
+        return null === $value2 || $value1 > $value2;
     }
 
     /**
diff --git a/src/Symfony/Component/Validator/Constraints/LessThanOrEqualValidator.php b/src/Symfony/Component/Validator/Constraints/LessThanOrEqualValidator.php
@@ -24,7 +24,7 @@ class LessThanOrEqualValidator extends AbstractComparisonValidator
      */
     protected function compareValues($value1, $value2)
     {
-        return $value1 <= $value2;
+        return null === $value2 || $value1 <= $value2;
     }
 
     /**
diff --git a/src/Symfony/Component/Validator/Constraints/LessThanValidator.php b/src/Symfony/Component/Validator/Constraints/LessThanValidator.php
@@ -24,7 +24,7 @@ class LessThanValidator extends AbstractComparisonValidator
      */
     protected function compareValues($value1, $value2)
     {
-        return $value1 < $value2;
+        return null === $value2 || $value1 < $value2;
     }
 
     /**
diff --git a/src/Symfony/Component/Validator/Tests/Constraints/AbstractComparisonValidatorTestCase.php b/src/Symfony/Component/Validator/Tests/Constraints/AbstractComparisonValidatorTestCase.php
@@ -234,6 +234,31 @@ public function throwsOnInvalidStringDatesProvider()
         ];
     }
 
+    /**
+     * @dataProvider provideComparisonsToNullValueAtPropertyPath
+     */
+    public function testCompareWithNullValueAtPropertyAt($dirtyValue, $dirtyValueAsString, $isValid)
+    {
+        $constraint = $this->createConstraint(['propertyPath' => 'value']);
+        $constraint->message = 'Constraint Message';
+
+        $object = new ComparisonTest_Class(null);
+        $this->setObject($object);
+
+        $this->validator->validate($dirtyValue, $constraint);
+
+        if ($isValid) {
+            $this->assertNoViolation();
+        } else {
+            $this->buildViolation('Constraint Message')
+                ->setParameter('{{ value }}', $dirtyValueAsString)
+                ->setParameter('{{ compared_value }}', 'null')
+                ->setParameter('{{ compared_value_type }}', 'NULL')
+                ->setCode($this->getErrorCode())
+                ->assertRaised();
+        }
+    }
+
     /**
      * @return array
      */
@@ -255,6 +280,8 @@ public function provideAllInvalidComparisons()
      */
     abstract public function provideInvalidComparisons();
 
+    abstract public function provideComparisonsToNullValueAtPropertyPath();
+
     /**
      * @param array|null $options Options for the constraint
      *
diff --git a/src/Symfony/Component/Validator/Tests/Constraints/EqualToValidatorTest.php b/src/Symfony/Component/Validator/Tests/Constraints/EqualToValidatorTest.php
@@ -75,4 +75,11 @@ public function provideInvalidComparisons()
             [new ComparisonTest_Class(4), '4', new ComparisonTest_Class(5), '5', __NAMESPACE__.'\ComparisonTest_Class'],
         ];
     }
+
+    public function provideComparisonsToNullValueAtPropertyPath()
+    {
+        return [
+            [5, '5', false],
+        ];
+    }
 }
diff --git a/src/Symfony/Component/Validator/Tests/Constraints/GreaterThanOrEqualValidatorTest.php b/src/Symfony/Component/Validator/Tests/Constraints/GreaterThanOrEqualValidatorTest.php
@@ -78,4 +78,11 @@ public function provideInvalidComparisons()
             ['b', '"b"', 'c', '"c"', 'string'],
         ];
     }
+
+    public function provideComparisonsToNullValueAtPropertyPath()
+    {
+        return [
+            [5, '5', true],
+        ];
+    }
 }
diff --git a/src/Symfony/Component/Validator/Tests/Constraints/GreaterThanValidatorTest.php b/src/Symfony/Component/Validator/Tests/Constraints/GreaterThanValidatorTest.php
@@ -80,4 +80,11 @@ public function provideInvalidComparisons()
             ['22', '"22"', '22', '"22"', 'string'],
         ];
     }
+
+    public function provideComparisonsToNullValueAtPropertyPath()
+    {
+        return [
+            [5, '5', true],
+        ];
+    }
 }
diff --git a/src/Symfony/Component/Validator/Tests/Constraints/IdenticalToValidatorTest.php b/src/Symfony/Component/Validator/Tests/Constraints/IdenticalToValidatorTest.php
@@ -93,4 +93,11 @@ public function provideInvalidComparisons()
             [new ComparisonTest_Class(4), '4', new ComparisonTest_Class(5), '5', __NAMESPACE__.'\ComparisonTest_Class'],
         ];
     }
+
+    public function provideComparisonsToNullValueAtPropertyPath()
+    {
+        return [
+            [5, '5', false],
+        ];
+    }
 }
diff --git a/src/Symfony/Component/Validator/Tests/Constraints/LessThanOrEqualValidatorTest.php b/src/Symfony/Component/Validator/Tests/Constraints/LessThanOrEqualValidatorTest.php
@@ -81,4 +81,11 @@ public function provideInvalidComparisons()
             ['c', '"c"', 'b', '"b"', 'string'],
         ];
     }
+
+    public function provideComparisonsToNullValueAtPropertyPath()
+    {
+        return [
+            [5, '5', true],
+        ];
+    }
 }
diff --git a/src/Symfony/Component/Validator/Tests/Constraints/LessThanValidatorTest.php b/src/Symfony/Component/Validator/Tests/Constraints/LessThanValidatorTest.php
@@ -79,4 +79,11 @@ public function provideInvalidComparisons()
             ['333', '"333"', '22', '"22"', 'string'],
         ];
     }
+
+    public function provideComparisonsToNullValueAtPropertyPath()
+    {
+        return [
+            [5, '5', true],
+        ];
+    }
 }
diff --git a/src/Symfony/Component/Validator/Tests/Constraints/NotEqualToValidatorTest.php b/src/Symfony/Component/Validator/Tests/Constraints/NotEqualToValidatorTest.php
@@ -75,4 +75,11 @@ public function provideInvalidComparisons()
             [new ComparisonTest_Class(5), '5', new ComparisonTest_Class(5), '5', __NAMESPACE__.'\ComparisonTest_Class'],
         ];
     }
+
+    public function provideComparisonsToNullValueAtPropertyPath()
+    {
+        return [
+            [5, '5', true],
+        ];
+    }
 }
diff --git a/src/Symfony/Component/Validator/Tests/Constraints/NotIdenticalToValidatorTest.php b/src/Symfony/Component/Validator/Tests/Constraints/NotIdenticalToValidatorTest.php
@@ -93,4 +93,11 @@ public function provideInvalidComparisons()
 
         return $comparisons;
     }
+
+    public function provideComparisonsToNullValueAtPropertyPath()
+    {
+        return [
+            [5, '5', true],
+        ];
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -24,7 +24,7 @@ class GreaterThanOrEqualValidator extends AbstractComparisonValidator`
`24`	`24`	`*/`
`25`	`25`	`protected function compareValues($value1, $value2)`
`26`	`26`	`{`
`27`		`- return $value1 >= $value2;`
	`27`	`+ return null === $value2 \|\| $value1 >= $value2;`
`28`	`28`	`}`
`29`	`29`
`30`	`30`	`/**`
Original file line number	Diff line number	Diff line change
`@@ -24,7 +24,7 @@ class GreaterThanValidator extends AbstractComparisonValidator`
`24`	`24`	`*/`
`25`	`25`	`protected function compareValues($value1, $value2)`
`26`	`26`	`{`
`27`		`- return $value1 > $value2;`
	`27`	`+ return null === $value2 \|\| $value1 > $value2;`
`28`	`28`	`}`
`29`	`29`
`30`	`30`	`/**`
Original file line number	Diff line number	Diff line change
`@@ -24,7 +24,7 @@ class LessThanOrEqualValidator extends AbstractComparisonValidator`
`24`	`24`	`*/`
`25`	`25`	`protected function compareValues($value1, $value2)`
`26`	`26`	`{`
`27`		`- return $value1 <= $value2;`
	`27`	`+ return null === $value2 \|\| $value1 <= $value2;`
`28`	`28`	`}`
`29`	`29`
`30`	`30`	`/**`
Original file line number	Diff line number	Diff line change
`@@ -24,7 +24,7 @@ class LessThanValidator extends AbstractComparisonValidator`
`24`	`24`	`*/`
`25`	`25`	`protected function compareValues($value1, $value2)`
`26`	`26`	`{`
`27`		`- return $value1 < $value2;`
	`27`	`+ return null === $value2 \|\| $value1 < $value2;`
`28`	`28`	`}`
`29`	`29`
`30`	`30`	`/**`
Original file line number	Diff line number	Diff line change
`@@ -75,4 +75,11 @@ public function provideInvalidComparisons()`
`75`	`75`	`[new ComparisonTest_Class(4), '4', new ComparisonTest_Class(5), '5', __NAMESPACE__.'\ComparisonTest_Class'],`
`76`	`76`	`];`
`77`	`77`	`}`
	`78`	`+`
	`79`	`+ public function provideComparisonsToNullValueAtPropertyPath()`
	`80`	`+ {`
	`81`	`+ return [`
	`82`	`+ [5, '5', false],`
	`83`	`+ ];`
	`84`	`+ }`
`78`	`85`	`}`
Original file line number	Diff line number	Diff line change
`@@ -78,4 +78,11 @@ public function provideInvalidComparisons()`
`78`	`78`	`['b', '"b"', 'c', '"c"', 'string'],`
`79`	`79`	`];`
`80`	`80`	`}`
	`81`	`+`
	`82`	`+ public function provideComparisonsToNullValueAtPropertyPath()`
	`83`	`+ {`
	`84`	`+ return [`
	`85`	`+ [5, '5', true],`
	`86`	`+ ];`
	`87`	`+ }`
`81`	`88`	`}`
Original file line number	Diff line number	Diff line change
`@@ -80,4 +80,11 @@ public function provideInvalidComparisons()`
`80`	`80`	`['22', '"22"', '22', '"22"', 'string'],`
`81`	`81`	`];`
`82`	`82`	`}`
	`83`	`+`
	`84`	`+ public function provideComparisonsToNullValueAtPropertyPath()`
	`85`	`+ {`
	`86`	`+ return [`
	`87`	`+ [5, '5', true],`
	`88`	`+ ];`
	`89`	`+ }`
`83`	`90`	`}`
Original file line number	Diff line number	Diff line change
`@@ -93,4 +93,11 @@ public function provideInvalidComparisons()`
`93`	`93`	`[new ComparisonTest_Class(4), '4', new ComparisonTest_Class(5), '5', __NAMESPACE__.'\ComparisonTest_Class'],`
`94`	`94`	`];`
`95`	`95`	`}`
	`96`	`+`
	`97`	`+ public function provideComparisonsToNullValueAtPropertyPath()`
	`98`	`+ {`
	`99`	`+ return [`
	`100`	`+ [5, '5', false],`
	`101`	`+ ];`
	`102`	`+ }`
`96`	`103`	`}`
Original file line number	Diff line number	Diff line change
`@@ -81,4 +81,11 @@ public function provideInvalidComparisons()`
`81`	`81`	`['c', '"c"', 'b', '"b"', 'string'],`
`82`	`82`	`];`
`83`	`83`	`}`
	`84`	`+`
	`85`	`+ public function provideComparisonsToNullValueAtPropertyPath()`
	`86`	`+ {`
	`87`	`+ return [`
	`88`	`+ [5, '5', true],`
	`89`	`+ ];`
	`90`	`+ }`
`84`	`91`	`}`
Original file line number	Diff line number	Diff line change
`@@ -79,4 +79,11 @@ public function provideInvalidComparisons()`
`79`	`79`	`['333', '"333"', '22', '"22"', 'string'],`
`80`	`80`	`];`
`81`	`81`	`}`
	`82`	`+`
	`83`	`+ public function provideComparisonsToNullValueAtPropertyPath()`
	`84`	`+ {`
	`85`	`+ return [`
	`86`	`+ [5, '5', true],`
	`87`	`+ ];`
	`88`	`+ }`
`82`	`89`	`}`