8000 Merge branch '4.4' into 5.1 · symfony/symfony@355f18d · GitHub
[go: up one dir, main page]
Skip to content

Commit 355f18d

Browse files
xabbuhxabbuh
committed
Merge branch '4.4' into 5.1
* 4.4: prevent hash collisions caused by reused object hashes autoconfigure behavior describing tags on decorators [Validator][RecursiveContextualValidator] Prevent validated hash collisions
2 parents b8afc7c + c72f853 commit 355f18d

File tree

9 files changed

+209
-24
lines changed

9 files changed

+209
-24
lines changed

‎src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -493,6 +493,14 @@ public function load(array $configs, ContainerBuilder $container)
493493

494494
$container->registerForAutoconfiguration(RouteLoaderInterface::class)
495495
->addTag('routing.route_loader');
496+
497+
$container->setParameter('container.behavior_describing_tags', [
498+
'container.service_locator',
499+
'container.service_subscriber',
500+
'kernel.event_subscriber',
501+
'kernel.locale_aware',
502+
'kernel.reset',
503+
]);
496504
}
497505

498506
/**

‎src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1477,6 +1477,20 @@ public function testMailerWithSpecificMessageBus(): void
14771477
$this->assertEquals(new Reference('app.another_bus'), $container->getDefinition('mailer.mailer')->getArgument(1));
14781478
}
14791479

1480+
public function testRegisterParameterCollectingBehaviorDescribingTags()
1481+
{
1482+
$container = $this->createContainerFromFile('default_config');
1483+
1484+
$this->assertTrue($container->hasParameter('container.behavior_describing_tags'));
1485+
$this->assertEquals([
1486+
'container.service_locator',
1487+
'container.service_subscriber',
1488+
'kernel.event_subscriber',
1489+
'kernel.locale_aware',
1490+
'kernel.reset',
1491+
], $container->getParameter('container.behavior_describing_tags'));
1492+
}
1493+
14801494
protected function createContainer(array $data = [])
14811495
{
14821496
return new ContainerBuilder(new EnvPlaceholderParameterBag(array_merge([

‎src/Symfony/Component/DependencyInjection/Compiler/ResolveInstanceofConditionalsPass.php

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,12 +35,22 @@ public function process(ContainerBuilder $container)
3535
}
3636
}
3737

38+
$tagsToKeep = [];
39+
40+
if ($container->hasParameter('container.behavior_describing_tags')) {
41+
$tagsToKeep = $container->getParameter('container.behavior_describing_tags');
42+
}
43+
3844
foreach ($container->getDefinitions() as $id => $definition) {
39-
$container->setDefinition($id, $this->processDefinition($container, $id, $definition));
45+
$container->setDefinition($id, $this->processDefinition($container, $id, $definition, $tagsToKeep));
46+
}
47+
48+
if ($container->hasParameter('container.behavior_describing_tags')) {
49+
$container->getParameterBag()->remove('container.behavior_describing_tags');
4050
}
4151
}
4252

43-
private function processDefinition(ContainerBuilder $container, string $id, Definition $definition): Definition
53+
private function processDefinition(ContainerBuilder $container, string $id, Definition $definition, array $tagsToKeep): Definition
4454
{
4555
$instanceofConditionals = $definition->getInstanceofConditionals();
4656
$autoconfiguredInstanceof = $definition->isAutoconfigured() ? $container->getAutoconfiguredInstanceof() : [];
@@ -114,10 +124,10 @@ private function processDefinition(ContainerBuilder $container, string $id, Defi
114124
}
115125

116126
// Don't add tags to service decorators
117-
if (null === $definition->getDecoratedService()) {
118-
$i = \count($instanceofTags);
119-
while (0 <= --$i) {
120-
foreach ($instanceofTags[$i] as $k => $v) {
127+
$i = \count($instanceofTags);
128+
while (0 <= --$i) {
129+
foreach ($instanceofTags[$i] as $k => $v) {
130+
if (null === $definition->getDecoratedService() || \in_array($k, $tagsToKeep, true)) {
121131
foreach ($v as $v) {
122132
if ($definition->hasTag($k) && \in_array($v, $definition->getTag($k))) {
123133
continue;

‎src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveInstanceofConditionalsPassTest.php

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,16 @@
1212
namespace Symfony\Component\DependencyInjection\Tests\Compiler;
1313

1414
use PHPUnit\Framework\TestCase;
15+
use Symfony\Component\Config\Resource\ResourceInterface;
16+
use Symfony\Component\Config\ResourceCheckerInterface;
1517
use Symfony\Component\DependencyInjection\Argument\BoundArgument;
1618
use Symfony\Component\DependencyInjection\ChildDefinition;
1719
use Symfony\Component\DependencyInjection\Compiler\ResolveChildDefinitionsPass;
1820
use Symfony\Component\DependencyInjection\Compiler\ResolveInstanceofConditionalsPass;
1921
use Symfony\Component\DependencyInjection\ContainerBuilder;
2022
use Symfony\Component\DependencyInjection\Reference;
23+
use Symfony\Contracts\Service\ResetInterface;
24+
use Symfony\Contracts\Service\ServiceSubscriberInterface;
2125

2226
class ResolveInstanceofConditionalsPassTest extends TestCase
2327
{
@@ -325,4 +329,60 @@ public function testDecoratorsAreNotAutomaticallyTagged()
325329

326330
$this->assertSame(['manual' => [[]]], $container->getDefinition('decorator')->getTags());
327331
}
332+
333+
public function testDecoratorsKeepBehaviorDescribingTags()
334+
{
335+
$container = new ContainerBuilder();
336+
337+
$container->setParameter('container.behavior_describing_tags', [
338+
'container.service_subscriber',
339+
'kernel.reset',
340+
]);
341+
342+
$container->register('decorator', DecoratorWithBehavior::class)
343+
->setAutoconfigured(true)
344+
->setDecoratedService('decorated')
345+
;
346+
347+
$container->registerForAutoconfiguration(ResourceCheckerInterface::class)
348+
->addTag('config_cache.resource_checker')
349+
;
350+
$container->registerForAutoconfiguration(ServiceSubscriberInterface::class)
351+
->addTag('container.service_subscriber')
352+
;
353+
$container->registerForAutoconfiguration(ResetInterface::class)
354+
->addTag('kernel.reset', ['method' => 'reset'])
355+
;
356+
357+
(new ResolveInstanceofConditionalsPass())->process($container);
358+
359+
$this->assertEquals([
360+
'container.service_subscriber' => [0 => []],
361+
'kernel.reset' => [
362+
[
363+
'method' => 'reset',
364+
],
365+
],
366+
], $container->getDefinition('decorator')->getTags());
367+
$this->assertFalse($container->hasParameter('container.behavior_describing_tags'));
368+
}
369+
}
370+
371+
class DecoratorWithBehavior implements ResetInterface, ResourceCheckerInterface, ServiceSubscriberInterface
372+
{
373+
public function reset()
374+
{
375+
}
376+
377+
public function supports(ResourceInterface $metadata)
378+
{
379+
}
380+
381+
public function isFresh(ResourceInterface $resource, $timestamp)
382+
{
383+
}
384+
385+
public static function getSubscribedServices()
386+
{
387+
}
328388
}

‎src/Symfony/Component/Form/Extension/Validator/Constraints/FormValidator.php

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
class FormValidator extends ConstraintValidator
2626
{
2727
private $resolvedGroups;
28-
private $fieldFormConstraints;
2928

3029
/**
3130
* {@inheritdoc}
@@ -68,7 +67,6 @@ public function validate($form, Constraint $formConstraint)
6867

6968
if ($hasChildren && $form->isRoot()) {
7069
$this->resolvedGroups = new \SplObjectStorage();
71-
$this->fieldFormConstraints = [];
7270
}
7371

7472
if ($groups instanceof GroupSequence) {
@@ -93,7 +91,6 @@ public function validate($form, Constraint $formConstraint)
9391
$this->resolvedGroups[$field] = (array) $group;
9492
$fieldFormConstraint = new Form();
9593
$fieldFormConstraint->groups = $group;
96-
$this->fieldFormConstraints[] = $fieldFormConstraint;
9794
$this->context->setNode($this->context->getValue(), $field, $this->context->getMetadata(), $this->context->getPropertyPath());
9895
$validator->atPath(sprintf('children[%s]', $field->getName()))->validate($field, $fieldFormConstraint, $group);
9996
}
@@ -139,18 +136,15 @@ public function validate($form, Constraint $formConstraint)
139136
foreach ($form->all() as $field) {
140137
if ($field->isSubmitted()) {
141138
$this->resolvedGroups[$field] = $groups;
142-
$fieldFormConstraint = new Form();
143-
$this->fieldFormConstraints[] = $fieldFormConstraint;
144139
$this->context->setNode($this->context->getValue(), $field, $this->context->getMetadata(), $this->context->getPropertyPath());
145-
$validator->atPath(sprintf('children[%s]', $field->getName()))->validate($field, $fieldFormConstraint);
140+
$validator->atPath(sprintf('children[%s]', $field->getName()))->validate($field, $formConstraint);
146141
}
147142
}
148143
}
149144

150145
if ($hasChildren && $form->isRoot()) {
151146
// destroy storage to avoid memory leaks
152147
$this->resolvedGroups = new \SplObjectStorage();
153-
$this->fieldFormConstraints = [];
154148
}
155149
} elseif (!$form->isSynchronized()) {
156150
$childrenSynchronized = true;
@@ -159,11 +153,8 @@ public function validate($form, Constraint $formConstraint)
159153
foreach ($form as $child) {
160154
if (!$child->isSynchronized()) {
161155
$childrenSynchronized = false;
162-
163-
$fieldFormConstraint = new Form();
164-
$this->fieldFormConstraints[] = $fieldFormConstraint;
165156
$this->context->setNode($this->context->getValue(), $child, $this->context->getMetadata(), $this->context->getPropertyPath());
166-
$validator->atPath(sprintf('children[%s]', $child->getName()))->validate($child, $fieldFormConstraint);
157+
$validator->atPath(sprintf('children[%s]', $child->getName()))->validate($child, $formConstraint);
167158
}
168159
}
169160

‎src/Symfony/Component/Form/composer.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
},
3030
"require-dev": {
3131
"doctrine/collections": "~1.0",
32-
"symfony/validator": "^4.4.12|^5.1.6",
32+
"symfony/validator": "^4.4.17|^5.1.9",
3333
"symfony/dependency-injection": "^4.4|^5.0",
3434
"symfony/expression-language": "^4.4|^5.0",
3535
"symfony/config": "^4.4|^5.0",

‎src/Symfony/Component/Validator/Context/ExecutionContext.php

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,7 @@ class ExecutionContext implements ExecutionContextInterface
128128
* @var array
129129
*/
130130
private $initializedObjects;
131+
private $cachedObjectsRefs;
131132

132133
/**
133134
* @param mixed $root The root value of the validated object graph
@@ -141,6 +142,7 @@ public function __construct(ValidatorInterface $validator, $root, TranslatorInte
141142
$this->translator = $translator;
142143
$this->translationDomain = $translationDomain;
143144
$this->violations = new ConstraintViolationList();
145+
$this->cachedObjectsRefs = new \SplObjectStorage();
144146
}
145147

146148
/**
@@ -346,4 +348,20 @@ public function isObjectInitialized(string $cacheKey): bool
346348
{
347349
return isset($this->initializedObjects[$cacheKey]);
348350
}
351+
352+
/**
353+
* @internal
354+
*
355+
* @param object $object
356+
*
357+
* @return string
358+
*/
359+
public function generateCacheKey($object)
360+
{
361+
if (!isset($this->cachedObjectsRefs[$object])) {
362+
$this->cachedObjectsRefs[$object] = spl_object_hash($object);
363+
}
364+
365+
return $this->cachedObjectsRefs[$object];
366+
}
349367
}

‎src/Symfony/Component/Validator/Tests/Validator/RecursiveValidatorTest.php

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,14 @@
1313

1414
use PHPUnit\Framework\TestCase;
1515
use Symfony\Component\Translation\IdentityTranslator;
16+
use Symfony\Component\Validator\Constraint;
1617
use Symfony\Component\Validator\Constraints\All;
1718
use Symfony\Component\Validator\Constraints\Callback;
1819
use Symfony\Component\Validator\Constraints\Collection;
1920
use Symfony\Component\Validator\Constraints\Expression;
2021
use Symfony\Component\Validator\Constraints\GroupSequence;
22+
use Symfony\Component\Validator\Constraints\IsFalse;
23+
use Symfony\Component\Validator\Constraints\IsNull;
2124
use Symfony\Component\Validator\Constraints\IsTrue;
2225
use Symfony\Component\Validator\Constraints\Length;
2326
use Symfony\Component\Validator\Constraints\NotBlank;
@@ -26,6 +29,7 @@
2629
use Symfony\Component\Validator\Constraints\Required;
2730
use Symfony\Component\Validator\Constraints\Traverse;
2831
use Symfony\Component\Validator\Constraints\Valid;
32+
use Symfony\Component\Validator\ConstraintValidator;
2933
use Symfony\Component\Validator\ConstraintValidatorFactory;
3034
use Symfony\Component\Validator\ConstraintViolationInterface;
3135
use Symfony\Component\Validator\Context\ExecutionContextFactory;
< 10000 /div>
@@ -2135,4 +2139,66 @@ public function testOptionalConstraintIsIgnored()
21352139

21362140
$this->assertCount(0, $violations);
21372141
}
2142+
2143+
public function testValidatedConstraintsHashesDoNotCollide()
2144+
{
2145+
$metadata = new ClassMetadata(Entity::class);
2146+
$metadata->addPropertyConstraint('initialized', new NotNull(['groups' => 'should_pass']));
2147+
$metadata->addPropertyConstraint('initialized', new IsNull(['groups' => 'should_fail']));
2148+
2149+
$this->metadataFactory->addMetadata($metadata);
2150+
2151+
$entity = new Entity();
2152+
$entity->data = new \stdClass();
2153+
2154+
$this->assertCount(2, $this->validator->validate($entity, new TestConstraintHashesDoNotCollide()));
2155+
}
2156+
2157+
public function testValidatedConstraintsHashesDoNotCollideWithSameConstraintValidatingDifferentProperties()
2158+
{
2159+
$value = new \stdClass();
2160+
2161+
$entity = new Entity();
2162+
$entity->firstName = $value;
2163+
$entity->setLastName($value);
2164+
2165+
$validator = $this->validator->startContext($entity);
2166+
2167+
$constraint = new IsNull();
2168+
$validator->atPath('firstName')
2169+
->validate($entity->firstName, $constraint);
2170+
$validator->atPath('lastName')
2171+
->validate($entity->getLastName(), $constraint);
2172+
2173+
$this->assertCount(2, $validator->getViolations());
2174+
}
2175+
}
2176+
2177+
final class TestConstraintHashesDoNotCollide extends Constraint
2178+
{
2179+
}
2180+
2181+
final class TestConstraintHashesDoNotCollideValidator extends ConstraintValidator
2182+
{
2183+
/**
2184+
* {@inheritdoc}
2185+
*/
2186+
public function validate($value, Constraint $constraint)
2187+
{
2188+
if (!$value instanceof Entity) {
2189+
throw new \LogicException();
2190+
}
2191+
2192+
$this->context->getValidator()
2193+
->inContext($this->context)
2194+
->atPath('data')
2195+
->validate($value, new NotNull())
2196+
->validate($value, new NotNull())
2197+
->validate($value, new IsFalse());
2198+
2199+
$this->context->getValidator()
2200+
->inContext($this->context)
2201+
->validate($value, null, new GroupSequence(['should_pass']))
2202+
->validate($value, null, new GroupSequence(['should_fail']));
2203+
}
21382204
}

0 commit comments

Comments
 (0)
0