Skip to content

Commit 93b9edc

Browse files
committed
Merge branch '5.1' into 5.2
* 5.1: prevent hash collisions caused by reused object hashes autoconfigure behavior describing tags on decorators [Validator][RecursiveContextualValidator] Prevent validated hash collisions
2 parents d66cfc3 + bd66015 commit 93b9edc

File tree

3 files changed

+108
-6
lines changed

3 files changed

+108
-6
lines changed

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
}

Tests/Validator/RecursiveValidatorTest.php

Lines 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
}

Validator/RecursiveContextualValidator.php

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ public function validate($value, $constraints = null, $groups = null)
108108
$this->validateGenericNode(
109109
$value,
110110
$previousObject,
111-
\is_object($value) ? spl_object_hash($value) : null,
111+
\is_object($value) ? $this->generateCacheKey($value) : null,
112112
$metadata,
113113
$this->defaultPropertyPath,
114114
$groups,
@@ -176,7 +176,7 @@ public function validateProperty($object, string $propertyName, $groups = null)
176176

177177
$propertyMetadatas = $classMetadata->getPropertyMetadata($propertyName);
178178
$groups = $groups ? $this->normalizeGroups($groups) : $this->defaultGroups;
179-
$cacheKey = spl_object_hash($object);
179+
$cacheKey = $this->generateCacheKey($object);
180180
$propertyPath = PropertyPath::append($this->defaultPropertyPath, $propertyName);
181181

182182
$previousValue = $this->context->getValue();
@@ -224,7 +224,7 @@ public function validatePropertyValue($objectOrClass, string $propertyName, $val
224224
if (\is_object($objectOrClass)) {
225225
$object = $objectOrClass;
226226
$class = \get_class($object);
227-
$cacheKey = spl_object_hash($objectOrClass);
227+
$cacheKey = $this->generateCacheKey($objectOrClass);
228228
$propertyPath = PropertyPath::append($this->defaultPropertyPath, $propertyName);
229229
} else {
230230
// $objectOrClass contains a class name
@@ -311,7 +311,7 @@ private function validateObject($object, string $propertyPath, array $groups, in
311311

312312
$this->validateClassNode(
313313
$object,
314-
spl_object_hash($object),
314+
$this->generateCacheKey($object),
315315
$classMetadata,
316316
$propertyPath,
317317
$groups,
@@ -425,7 +425,7 @@ private function validateClassNode(object $object, ?string $cacheKey, ClassMetad
425425
$defaultOverridden = false;
426426

427427
// Use the object hash for group sequences
428-
$groupHash = \is_object($group) ? spl_object_hash($group) : $group;
428+
$groupHash = \is_object($group) ? $this->generateCacheKey($group, true) : $group;
429429

430430
if ($context->isGroupValidated($cacheKey, $groupHash)) {
431431
// Skip this group when validating the properties and when
@@ -730,7 +730,7 @@ private function validateInGroup($value, ?string $cacheKey, MetadataInterface $m
730730
// Prevent duplicate validation of constraints, in the case
731731
// that constraints belong to multiple validated groups
732732
if (null !== $cacheKey) {
733-
$constraintHash = spl_object_hash($constraint);
733+
$constraintHash = $this->generateCacheKey($constraint, true);
734734
// instanceof Valid: In case of using a Valid constraint with many groups
735735
// it makes a reference object get validated by each group
736736
if ($constraint instanceof Composite || $constraint instanceof Valid) {
@@ -762,4 +762,22 @@ private function validateInGroup($value, ?string $cacheKey, MetadataInterface $m
762762
}
763763
}
764764
}
765+
766+
/**
767+
* @param object $object
768+
*/
769+
private function generateCacheKey($object, bool $dependsOnPropertyPath = false): string
770+
{
771+
if ($this->context instanceof ExecutionContext) {
772+
$cacheKey = $this->context->generateCacheKey($object);
773+
} else {
774+
$cacheKey = spl_object_hash($object);
775+
}
776+
777+
if ($dependsOnPropertyPath) {
778+
$cacheKey .= $this->context->getPropertyPath();
779+
}
780+
781+
return $cacheKey;
782+
}
765783
}

0 commit comments

Comments
 (0)