Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Validator] Only traverse arrays that are cascaded into #29800

Merged
merged 1 commit into from Apr 7, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Symfony/Component/Validator/Constraints/Collection.php
Expand Up @@ -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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a legit bug fix nevertheless.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may be a big BC as well, I'm not sure we should merge such change.

}
}
}
Expand Down
Expand Up @@ -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();
Expand Down
Expand Up @@ -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.']',
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)) {
xabbuh marked this conversation as resolved.
Show resolved Hide resolved
return;
}

Expand Down