bug #29800 [Validator] Only traverse arrays that are cascaded into (corphi)

fabpot · fabpot · commit 1f48f7bd8283 · 2019-04-07T10:20:18.000+02:00
This PR was squashed before being merged into the 3.4 branch (closes #29800). Discussion ---------- [Validator] Only traverse arrays that are cascaded into | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #27090 | License | MIT | Doc PR | - Previously, array properties were traversed even if they were not annotated `Valid`. Commits ------- 7db9200 [Validator] Only traverse arrays that are cascaded into
diff --git a/src/Symfony/Component/Validator/Constraints/Collection.php b/src/Symfony/Component/Validator/Constraints/Collection.php
@@ -68,7 +68,7 @@ protected function initializeNestedConstraints()
             }
 
             if (!$field instanceof Optional && !$field instanceof Required) {
-                $this->fields[$fieldName] = $field = new Required($field);
+                $this->fields[$fieldName] = new Required($field);
             }
         }
     }
diff --git a/src/Symfony/Component/Validator/Tests/Validator/AbstractValidatorTest.php b/src/Symfony/Component/Validator/Tests/Validator/AbstractValidatorTest.php
@@ -589,6 +589,30 @@ public function testRecursiveArrayReference()
         $this->assertNull($violations[0]->getCode());
     }
 
+    public function testOnlyCascadedArraysAreTraversed()
+    {
+        $entity = new Entity();
+        $entity->reference = ['key' => new Reference()];
+
+        $callback = function ($value, ExecutionContextInterface $context) {
+            $context->addViolation('Message %param%', ['%param%' => 'value']);
+        };
+
+        $this->metadata->addPropertyConstraint('reference', new Callback([
+            'callback' => function () {},
+            'groups' => 'Group',
+        ]));
+        $this->referenceMetadata->addConstraint(new Callback([
+            'callback' => $callback,
+            'groups' => 'Group',
+        ]));
+
+        $violations = $this->validate($entity, null, 'Group');
+
+        /* @var ConstraintViolationInterface[] $violations */
+        $this->assertCount(0, $violations);
+    }
+
     public function testArrayTraversalCannotBeDisabled()
     {
         $entity = new Entity();
diff --git a/src/Symfony/Component/Validator/Validator/RecursiveContextualValidator.php b/src/Symfony/Component/Validator/Validator/RecursiveContextualValidator.php
@@ -352,24 +352,18 @@ private function validateObject($object, $propertyPath, array $groups, $traversa
      * Validates each object in a collection against the constraints defined
      * for their classes.
      *
-     * If the parameter $recursive is set to true, nested {@link \Traversable}
-     * objects are iterated as well. Nested arrays are always iterated,
-     * regardless of the value of $recursive.
+     * Nested arrays are also iterated.
      *
      * @param iterable                  $collection   The collection
      * @param string                    $propertyPath The current property path
      * @param (string|GroupSequence)[]  $groups       The validated groups
      * @param ExecutionContextInterface $context      The current execution context
-     *
-     * @see ClassNode
-     * @see CollectionNode
      */
     private function validateEachObjectIn($collection, $propertyPath, array $groups, ExecutionContextInterface $context)
     {
         foreach ($collection as $key => $value) {
             if (\is_array($value)) {
-                // Arrays are always cascaded, independent of the specified
-                // traversal strategy
+                // Also traverse nested arrays
                 $this->validateEachObjectIn(
                     $value,
                     $propertyPath.'['.$key.']',
@@ -599,7 +593,8 @@ private function validateClassNode($object, $cacheKey, ClassMetadataInterface $m
      * in the passed metadata object. Then, if the value is an instance of
      * {@link \Traversable} and the selected traversal strategy permits it,
      * the value is traversed and each nested object validated against its own
-     * constraints. Arrays are always traversed.
+     * constraints. If the value is an array, it is traversed regardless of
+     * the given strategy.
      *
      * @param mixed                     $value             The validated value
      * @param object|null               $object            The current object
@@ -658,8 +653,8 @@ private function validateGenericNode($value, $object, $cacheKey, MetadataInterfa
 
         $cascadingStrategy = $metadata->getCascadingStrategy();
 
-        // Quit unless we have an array or a cascaded object
-        if (!\is_array($value) && !($cascadingStrategy & CascadingStrategy::CASCADE)) {
+        // Quit unless we cascade
+        if (!($cascadingStrategy & CascadingStrategy::CASCADE)) {
             return;
         }
 

Original file line number	Diff line number	Diff line change
`@@ -68,7 +68,7 @@ protected function initializeNestedConstraints()`
`68`	`68`	`}`
`69`	`69`
`70`	`70`	`if (!$field instanceof Optional && !$field instanceof Required) {`
`71`		`- $this->fields[$fieldName] = $field = new Required($field);`
	`71`	`+ $this->fields[$fieldName] = new Required($field);`
`72`	`72`	`}`
`73`	`73`	`}`
`74`	`74`	`}`