Skip to content

Commit bd66015

Browse files
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 093c236 + fb8a95a commit bd66015

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,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;
@@ -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
}

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)