diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php index ea5e139e3f..a599a684af 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php @@ -533,6 +533,14 @@ class FrameworkExtension extends Extension $container->registerForAutoconfiguration(RouteLoaderInterface::class) ->addTag('routing.route_loader'); + + $container->setParameter('container.behavior_describing_tags', [ + 'container.service_locator', + 'container.service_subscriber', + 'kernel.event_subscriber', + 'kernel.locale_aware', + 'kernel.reset', + ]); } /** diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php index b902d3b636..e9d141d528 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php @@ -1622,6 +1622,20 @@ abstract class FrameworkExtensionTest extends TestCase $this->assertSame('my_response_factory', (string) $argument); } + public function testRegisterParameterCollectingBehaviorDescribingTags() + { + $container = $this->createContainerFromFile('default_config'); + + $this->assertTrue($container->hasParameter('container.behavior_describing_tags')); + $this->assertEquals([ + 'container.service_locator', + 'container.service_subscriber', + 'kernel.event_subscriber', + 'kernel.locale_aware', + 'kernel.reset', + ], $container->getParameter('container.behavior_describing_tags')); + } + protected function createContainer(array $data = []) { return new ContainerBuilder(new EnvPlaceholderParameterBag(array_merge([ diff --git a/src/Symfony/Component/DependencyInjection/Compiler/ResolveInstanceofConditionalsPass.php b/src/Symfony/Component/DependencyInjection/Compiler/ResolveInstanceofConditionalsPass.php index 60d059fb29..4aa33aad1a 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/ResolveInstanceofConditionalsPass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/ResolveInstanceofConditionalsPass.php @@ -35,12 +35,22 @@ class ResolveInstanceofConditionalsPass implements CompilerPassInterface } } + $tagsToKeep = []; + + if ($container->hasParameter('container.behavior_describing_tags')) { + $tagsToKeep = $container->getParameter('container.behavior_describing_tags'); + } + foreach ($container->getDefinitions() as $id => $definition) { - $container->setDefinition($id, $this->processDefinition($container, $id, $definition)); + $container->setDefinition($id, $this->processDefinition($container, $id, $definition, $tagsToKeep)); + } + + if ($container->hasParameter('container.behavior_describing_tags')) { + $container->getParameterBag()->remove('container.behavior_describing_tags'); } } - private function processDefinition(ContainerBuilder $container, string $id, Definition $definition): Definition + private function processDefinition(ContainerBuilder $container, string $id, Definition $definition, array $tagsToKeep): Definition { $instanceofConditionals = $definition->getInstanceofConditionals(); $autoconfiguredInstanceof = $definition->isAutoconfigured() ? $container->getAutoconfiguredInstanceof() : []; @@ -114,10 +124,10 @@ class ResolveInstanceofConditionalsPass implements CompilerPassInterface } // Don't add tags to service decorators - if (null === $definition->getDecoratedService()) { - $i = \count($instanceofTags); - while (0 <= --$i) { - foreach ($instanceofTags[$i] as $k => $v) { + $i = \count($instanceofTags); + while (0 <= --$i) { + foreach ($instanceofTags[$i] as $k => $v) { + if (null === $definition->getDecoratedService() || \in_array($k, $tagsToKeep, true)) { foreach ($v as $v) { if ($definition->hasTag($k) && \in_array($v, $definition->getTag($k))) { continue; diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveInstanceofConditionalsPassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveInstanceofConditionalsPassTest.php index 3acfbed776..99aa65b138 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveInstanceofConditionalsPassTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveInstanceofConditionalsPassTest.php @@ -12,12 +12,16 @@ namespace Symfony\Component\DependencyInjection\Tests\Compiler; use PHPUnit\Framework\TestCase; +use Symfony\Component\Config\Resource\ResourceInterface; +use Symfony\Component\Config\ResourceCheckerInterface; use Symfony\Component\DependencyInjection\Argument\BoundArgument; use Symfony\Component\DependencyInjection\ChildDefinition; use Symfony\Component\DependencyInjection\Compiler\ResolveChildDefinitionsPass; use Symfony\Component\DependencyInjection\Compiler\ResolveInstanceofConditionalsPass; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Reference; +use Symfony\Contracts\Service\ResetInterface; +use Symfony\Contracts\Service\ServiceSubscriberInterface; class ResolveInstanceofConditionalsPassTest extends TestCase { @@ -325,4 +329,60 @@ class ResolveInstanceofConditionalsPassTest extends TestCase $this->assertSame(['manual' => [[]]], $container->getDefinition('decorator')->getTags()); } + + public function testDecoratorsKeepBehaviorDescribingTags() + { + $container = new ContainerBuilder(); + + $container->setParameter('container.behavior_describing_tags', [ + 'container.service_subscriber', + 'kernel.reset', + ]); + + $container->register('decorator', DecoratorWithBehavior::class) + ->setAutoconfigured(true) + ->setDecoratedService('decorated') + ; + + $container->registerForAutoconfiguration(ResourceCheckerInterface::class) + ->addTag('config_cache.resource_checker') + ; + $container->registerForAutoconfiguration(ServiceSubscriberInterface::class) + ->addTag('container.service_subscriber') + ; + $container->registerForAutoconfiguration(ResetInterface::class) + ->addTag('kernel.reset', ['method' => 'reset']) + ; + + (new ResolveInstanceofConditionalsPass())->process($container); + + $this->assertEquals([ + 'container.service_subscriber' => [0 => []], + 'kernel.reset' => [ + [ + 'method' => 'reset', + ], + ], + ], $container->getDefinition('decorator')->getTags()); + $this->assertFalse($container->hasParameter('container.behavior_describing_tags')); + } +} + +class DecoratorWithBehavior implements ResetInterface, ResourceCheckerInterface, ServiceSubscriberInterface +{ + public function reset() + { + } + + public function supports(ResourceInterface $metadata) + { + } + + public function isFresh(ResourceInterface $resource, $timestamp) + { + } + + public static function getSubscribedServices() + { + } } diff --git a/src/Symfony/Component/Form/Extension/Validator/Constraints/FormValidator.php b/src/Symfony/Component/Form/Extension/Validator/Constraints/FormValidator.php index 1c63ecf5f4..43eaeee4c8 100644 --- a/src/Symfony/Component/Form/Extension/Validator/Constraints/FormValidator.php +++ b/src/Symfony/Component/Form/Extension/Validator/Constraints/FormValidator.php @@ -25,7 +25,6 @@ use Symfony\Component\Validator\Exception\UnexpectedTypeException; class FormValidator extends ConstraintValidator { private $resolvedGroups; - private $fieldFormConstraints; /** * {@inheritdoc} @@ -68,7 +67,6 @@ class FormValidator extends ConstraintValidator if ($hasChildren && $form->isRoot()) { $this->resolvedGroups = new \SplObjectStorage(); - $this->fieldFormConstraints = []; } if ($groups instanceof GroupSequence) { @@ -93,7 +91,6 @@ class FormValidator extends ConstraintValidator $this->resolvedGroups[$field] = (array) $group; $fieldFormConstraint = new Form(); $fieldFormConstraint->groups = $group; - $this->fieldFormConstraints[] = $fieldFormConstraint; $this->context->setNode($this->context->getValue(), $field, $this->context->getMetadata(), $this->context->getPropertyPath()); $validator->atPath(sprintf('children[%s]', $field->getName()))->validate($field, $fieldFormConstraint, $group); } @@ -139,10 +136,8 @@ class FormValidator extends ConstraintValidator foreach ($form->all() as $field) { if ($field->isSubmitted()) { $this->resolvedGroups[$field] = $groups; - $fieldFormConstraint = new Form(); - $this->fieldFormConstraints[] = $fieldFormConstraint; $this->context->setNode($this->context->getValue(), $field, $this->context->getMetadata(), $this->context->getPropertyPath()); - $validator->atPath(sprintf('children[%s]', $field->getName()))->validate($field, $fieldFormConstraint); + $validator->atPath(sprintf('children[%s]', $field->getName()))->validate($field, $formConstraint); } } } @@ -150,7 +145,6 @@ class FormValidator extends ConstraintValidator if ($hasChildren && $form->isRoot()) { // destroy storage to avoid memory leaks $this->resolvedGroups = new \SplObjectStorage(); - $this->fieldFormConstraints = []; } } elseif (!$form->isSynchronized()) { $childrenSynchronized = true; @@ -159,11 +153,8 @@ class FormValidator extends ConstraintValidator foreach ($form as $child) { if (!$child->isSynchronized()) { $childrenSynchronized = false; - - $fieldFormConstraint = new Form(); - $this->fieldFormConstraints[] = $fieldFormConstraint; $this->context->setNode($this->context->getValue(), $child, $this->context->getMetadata(), $this->context->getPropertyPath()); - $validator->atPath(sprintf('children[%s]', $child->getName()))->validate($child, $fieldFormConstraint); + $validator->atPath(sprintf('children[%s]', $child->getName()))->validate($child, $formConstraint); } } diff --git a/src/Symfony/Component/Form/composer.json b/src/Symfony/Component/Form/composer.json index b17ef98049..8d9d8f9623 100644 --- a/src/Symfony/Component/Form/composer.json +++ b/src/Symfony/Component/Form/composer.json @@ -29,7 +29,7 @@ }, "require-dev": { "doctrine/collections": "~1.0", - "symfony/validator": "^4.4.12|^5.1.6", + "symfony/validator": "^4.4.17|^5.1.9", "symfony/dependency-injection": "^4.4|^5.0", "symfony/expression-language": "^4.4|^5.0", "symfony/config": "^4.4|^5.0", diff --git a/src/Symfony/Component/Validator/Context/ExecutionContext.php b/src/Symfony/Component/Validator/Context/ExecutionContext.php index 74ed391034..4560793eb5 100644 --- a/src/Symfony/Component/Validator/Context/ExecutionContext.php +++ b/src/Symfony/Component/Validator/Context/ExecutionContext.php @@ -128,6 +128,7 @@ class ExecutionContext implements ExecutionContextInterface * @var array */ private $initializedObjects; + private $cachedObjectsRefs; /** * @param mixed $root The root value of the validated object graph @@ -141,6 +142,7 @@ class ExecutionContext implements ExecutionContextInterface $this->translator = $translator; $this->translationDomain = $translationDomain; $this->violations = new ConstraintViolationList(); + $this->cachedObjectsRefs = new \SplObjectStorage(); } /** @@ -346,4 +348,20 @@ class ExecutionContext implements ExecutionContextInterface { return isset($this->initializedObjects[$cacheKey]); } + + /** + * @internal + * + * @param object $object + * + * @return string + */ + public function generateCacheKey($object) + { + if (!isset($this->cachedObjectsRefs[$object])) { + $this->cachedObjectsRefs[$object] = spl_object_hash($object); + } + + return $this->cachedObjectsRefs[$object]; + } } diff --git a/src/Symfony/Component/Validator/Tests/Validator/RecursiveValidatorTest.php b/src/Symfony/Component/Validator/Tests/Validator/RecursiveValidatorTest.php index f543ddf7d6..6866a88084 100644 --- a/src/Symfony/Component/Validator/Tests/Validator/RecursiveValidatorTest.php +++ b/src/Symfony/Component/Validator/Tests/Validator/RecursiveValidatorTest.php @@ -13,12 +13,15 @@ namespace Symfony\Component\Validator\Tests\Validator; use PHPUnit\Framework\TestCase; use Symfony\Component\Translation\IdentityTranslator; +use Symfony\Component\Validator\Constraint; use Symfony\Component\Validator\Constraints\All; use Symfony\Component\Validator\Constraints\Callback; use Symfony\Component\Validator\Constraints\Cascade; use Symfony\Component\Validator\Constraints\Collection; use Symfony\Component\Validator\Constraints\Expression; use Symfony\Component\Validator\Constraints\GroupSequence; +use Symfony\Component\Validator\Constraints\IsFalse; +use Symfony\Component\Validator\Constraints\IsNull; use Symfony\Component\Validator\Constraints\IsTrue; use Symfony\Component\Validator\Constraints\Length; use Symfony\Component\Validator\Constraints\NotBlank; @@ -28,6 +31,7 @@ use Symfony\Component\Validator\Constraints\Required; use Symfony\Component\Validator\Constraints\Traverse; use Symfony\Component\Validator\Constraints\Type; use Symfony\Component\Validator\Constraints\Valid; +use Symfony\Component\Validator\ConstraintValidator; use Symfony\Component\Validator\ConstraintValidatorFactory; use Symfony\Component\Validator\ConstraintViolationInterface; use Symfony\Component\Validator\Context\ExecutionContextFactory; @@ -2330,4 +2334,66 @@ class RecursiveValidatorTest extends TestCase CascadingEntity::$staticChild = null; } + + public function testValidatedConstraintsHashesDoNotCollide() + { + $metadata = new ClassMetadata(Entity::class); + $metadata->addPropertyConstraint('initialized', new NotNull(['groups' => 'should_pass'])); + $metadata->addPropertyConstraint('initialized', new IsNull(['groups' => 'should_fail'])); + + $this->metadataFactory->addMetadata($metadata); + + $entity = new Entity(); + $entity->data = new \stdClass(); + + $this->assertCount(2, $this->validator->validate($entity, new TestConstraintHashesDoNotCollide())); + } + + public function testValidatedConstraintsHashesDoNotCollideWithSameConstraintValidatingDifferentProperties() + { + $value = new \stdClass(); + + $entity = new Entity(); + $entity->firstName = $value; + $entity->setLastName($value); + + $validator = $this->validator->startContext($entity); + + $constraint = new IsNull(); + $validator->atPath('firstName') + ->validate($entity->firstName, $constraint); + $validator->atPath('lastName') + ->validate($entity->getLastName(), $constraint); + + $this->assertCount(2, $validator->getViolations()); + } +} + +final class TestConstraintHashesDoNotCollide extends Constraint +{ +} + +final class TestConstraintHashesDoNotCollideValidator extends ConstraintValidator +{ + /** + * {@inheritdoc} + */ + public function validate($value, Constraint $constraint) + { + if (!$value instanceof Entity) { + throw new \LogicException(); + } + + $this->context->getValidator() + ->inContext($this->context) + ->atPath('data') + ->validate($value, new NotNull()) + ->validate($value, new NotNull()) + ->validate($value, new IsFalse()); + + $this->context->getValidator() + ->inContext($this->context) + ->validate($value, null, new GroupSequence(['should_pass'])) + ->validate($value, null, new GroupSequence(['should_fail'])); + } } diff --git a/src/Symfony/Component/Validator/Validator/RecursiveContextualValidator.php b/src/Symfony/Component/Validator/Validator/RecursiveContextualValidator.php index eaa1309fd0..b1f1384ffb 100644 --- a/src/Symfony/Component/Validator/Validator/RecursiveContextualValidator.php +++ b/src/Symfony/Component/Validator/Validator/RecursiveContextualValidator.php @@ -108,7 +108,7 @@ class RecursiveContextualValidator implements ContextualValidatorInterface $this->validateGenericNode( $value, $previousObject, - \is_object($value) ? spl_object_hash($value) : null, + \is_object($value) ? $this->generateCacheKey($value) : null, $metadata, $this->defaultPropertyPath, $groups, @@ -176,7 +176,7 @@ class RecursiveContextualValidator implements ContextualValidatorInterface $propertyMetadatas = $classMetadata->getPropertyMetadata($propertyName); $groups = $groups ? $this->normalizeGroups($groups) : $this->defaultGroups; - $cacheKey = spl_object_hash($object); + $cacheKey = $this->generateCacheKey($object); $propertyPath = PropertyPath::append($this->defaultPropertyPath, $propertyName); $previousValue = $this->context->getValue(); @@ -224,7 +224,7 @@ class RecursiveContextualValidator implements ContextualValidatorInterface if (\is_object($objectOrClass)) { $object = $objectOrClass; $class = \get_class($object); - $cacheKey = spl_object_hash($objectOrClass); + $cacheKey = $this->generateCacheKey($objectOrClass); $propertyPath = PropertyPath::append($this->defaultPropertyPath, $propertyName); } else { // $objectOrClass contains a class name @@ -311,7 +311,7 @@ class RecursiveContextualValidator implements ContextualValidatorInterface $this->validateClassNode( $object, - spl_object_hash($object), + $this->generateCacheKey($object), $classMetadata, $propertyPath, $groups, @@ -425,7 +425,7 @@ class RecursiveContextualValidator implements ContextualValidatorInterface $defaultOverridden = false; // Use the object hash for group sequences - $groupHash = \is_object($group) ? spl_object_hash($group) : $group; + $groupHash = \is_object($group) ? $this->generateCacheKey($group, true) : $group; if ($context->isGroupValidated($cacheKey, $groupHash)) { // Skip this group when validating the properties and when @@ -730,7 +730,7 @@ class RecursiveContextualValidator implements ContextualValidatorInterface // Prevent duplicate validation of constraints, in the case // that constraints belong to multiple validated groups if (null !== $cacheKey) { - $constraintHash = spl_object_hash($constraint); + $constraintHash = $this->generateCacheKey($constraint, true); // instanceof Valid: In case of using a Valid constraint with many groups // it makes a reference object get validated by each group if ($constraint instanceof Composite || $constraint instanceof Valid) { @@ -762,4 +762,22 @@ class RecursiveContextualValidator implements ContextualValidatorInterface } } } + + /** + * @param object $object + */ + private function generateCacheKey($object, bool $dependsOnPropertyPath = false): string + { + if ($this->context instanceof ExecutionContext) { + $cacheKey = $this->context->generateCacheKey($object); + } else { + $cacheKey = spl_object_hash($object); + } + + if ($dependsOnPropertyPath) { + $cacheKey .= $this->context->getPropertyPath(); + } + + return $cacheKey; + } }