Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit c82c997

Browse filesBrowse files
xabbuhxabbuh
committed
Merge branch '5.2' into 5.x
* 5.2: prevent hash collisions caused by reused object hashes autoconfigure behavior describing tags on decorators [Validator][RecursiveContextualValidator] Prevent validated hash collisions
2 parents 430b916 + cc130e1 commit c82c997
Copy full SHA for c82c997

File tree

9 files changed

+209
-24
lines changed
Filter options

9 files changed

+209
-24
lines changed

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

Copy file name to clipboardExpand all lines: src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
+8Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -533,6 +533,14 @@ public function load(array $configs, ContainerBuilder $container)
533533

534534
$container->registerForAutoconfiguration(RouteLoaderInterface::class)
535535
->addTag('routing.route_loader');
536+
537+
$container->setParameter('container.behavior_describing_tags', [
538+
'container.service_locator',
539+
'container.service_subscriber',
540+
'kernel.event_subscriber',
541+
'kernel.locale_aware',
542+
'kernel.reset',
543+
]);
536544
}
537545

538546
/**

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

Copy file name to clipboardExpand all lines: src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php
+14Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1622,6 +1622,20 @@ public function testHttpClientMockResponseFactory()
16221622
$this->assertSame('my_response_factory', (string) $argument);
16231623
}
16241624

1625+
public function testRegisterParameterCollectingBehaviorDescribingTags()
1626+
{
1627+
$container = $this->createContainerFromFile('default_config');
1628+
1629+
$this->assertTrue($container->hasParameter('container.behavior_describing_tags'));
1630+
$this->assertEquals([
1631+
'container.service_locator',
1632+
'container.service_subscriber',
1633+
'kernel.event_subscriber',
1634+
'kernel.locale_aware',
1635+
'kernel.reset',
1636+
], $container->getParameter('container.behavior_describing_tags'));
1637+
}
1638+
16251639
protected function createContainer(array $data = [])
16261640
{
16271641
return new ContainerBuilder(new EnvPlaceholderParameterBag(array_merge([

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Compiler/ResolveInstanceofConditionalsPass.php
+16-6Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveInstanceofConditionalsPassTest.php
+60Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Form/Extension/Validator/Constraints/FormValidator.php
+2-11Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Form/composer.json
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030
},
3131
"require-dev": {
3232
"doctrine/collections": "~1.0",
33-
"symfony/validator": "^4.4.12|^5.1.6",
33+
"symfony/validator": "^4.4.17|^5.1.9",
3434
"symfony/dependency-injection": "^4.4|^5.0",
3535
"symfony/expression-language": "^4.4|^5.0",
3636
"symfony/config": "^4.4|^5.0",

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Validator/Context/ExecutionContext.php
+18Lines 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

Copy file name to clipboardExpand all lines: src/Symfony/Component/Validator/Tests/Validator/RecursiveValidatorTest.php
+66Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,15 @@
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\Cascade;
1920
use Symfony\Component\Validator\Constraints\Collection;
2021
use Symfony\Component\Validator\Constraints\Expression;
2122
use Symfony\Component\Validator\Constraints\GroupSequence;
23+
use Symfony\Component\Validator\Constraints\IsFalse;
24+
use Symfony\Component\Validator\Constraints\IsNull;
2225
use Symfony\Component\Validator\Constraints\IsTrue;
2326
use Symfony\Component\Validator\Constraints\Length;
2427
use Symfony\Component\Validator\Constraints\NotBlank;
@@ -28,6 +31,7 @@
2831
use Symfony\Component\Validator\Constraints\Traverse;
2932
use Symfony\Component\Validator\Constraints\Type;
3033
use Symfony\Component\Validator\Constraints\Valid;
34+
use Symfony\Component\Validator\ConstraintValidator;
3135
use Symfony\Component\Validator\ConstraintValidatorFactory;
3236
use Symfony\Component\Validator\ConstraintViolationInterface;
3337
use Symfony\Component\Validator\Context\ExecutionContextFactory;
@@ -2330,4 +2334,66 @@ public function testValidateWithExplicitCascade()
23302334

23312335
CascadingEntity::$staticChild = null;
23322336
}
2337+
2338+
public function testValidatedConstraintsHashesDoNotCollide()
2339+
{
2340+
$metadata = new ClassMetadata(Entity::class);
2341+
$metadata->addPropertyConstraint('initialized', new NotNull(['groups' => 'should_pass']));
2342+
$metadata->addPropertyConstraint('initialized', new IsNull(['groups' => 'should_fail']));
2343+
2344+
$this->metadataFactory->addMetadata($metadata);
2345+
2346+
$entity = new Entity();
2347+
$entity->data = new \stdClass();
2348+
2349+
$this->assertCount(2, $this->validator->validate($entity, new TestConstraintHashesDoNotCollide()));
2350+
}
2351+
2352+
public function testValidatedConstraintsHashesDoNotCollideWithSameConstraintValidatingDifferentProperties()
2353+
{
2354+
$value = new \stdClass();
2355+
2356+
$entity = new Entity();
2357+
$entity->firstName = $value;
2358+
$entity->setLastName($value);
2359+
2360+
$validator = $this->validator->startContext($entity);
2361+
2362+
$constraint = new IsNull();
2363+
$validator->atPath('firstName')
2364+
->validate($entity->firstName, $constraint);
2365+
$validator->atPath('lastName')
2366+
->validate($entity->getLastName(), $constraint);
2367+
2368+
$this->assertCount(2, $validator->getViolations());
2369+
}
2370+
}
2371+
2372+
final class TestConstraintHashesDoNotCollide extends Constraint
2373+
{
2374+
}
2375+
2376+
final class TestConstraintHashesDoNotCollideValidator extends ConstraintValidator
2377+
{
2378+
/**
2379+
* {@inheritdoc}
2380+
*/
2381+
public function validate($value, Constraint $constraint)
2382+
{
2383+
if (!$value instanceof Entity) {
2384+
throw new \LogicException();
2385+
}
2386+
2387+
$this->context->getValidator()
2388+
->inContext($this->context)
2389+
->atPath('data')
2390+
->validate($value, new NotNull())
2391+
->validate($value, new NotNull())
2392+
->validate($value, new IsFalse());
2393+
2394+
$this->context->getValidator()
2395+
->inContext($this->context)
2396+
->validate($value, null, new GroupSequence(['should_pass']))
2397+
->validate($value, null, new GroupSequence(['should_fail']));
2398+
}
23332399
}

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.