diff --git a/lib/Doctrine/ORM/Cache/DefaultQueryCache.php b/lib/Doctrine/ORM/Cache/DefaultQueryCache.php index d431313ef0..866ea0db25 100644 --- a/lib/Doctrine/ORM/Cache/DefaultQueryCache.php +++ b/lib/Doctrine/ORM/Cache/DefaultQueryCache.php @@ -22,6 +22,7 @@ use Doctrine\Common\Collections\ArrayCollection; use Doctrine\ORM\Cache\Persister\CachedPersister; +use Doctrine\ORM\Cache\Persister\Entity\CachedEntityPersister; use Doctrine\ORM\EntityManagerInterface; use Doctrine\ORM\Query\ResultSetMapping; use Doctrine\ORM\Mapping\ClassMetadata; @@ -29,6 +30,7 @@ use Doctrine\Common\Proxy\Proxy; use Doctrine\ORM\Cache; use Doctrine\ORM\Query; +use function assert; /** * Default query cache implementation. @@ -106,25 +108,27 @@ public function get(QueryCacheKey $key, ResultSetMapping $rsm, array $hints = [] $result = []; $entityName = reset($rsm->aliasMap); - $hasRelation = ( ! empty($rsm->relationMap)); + $hasRelation = ! empty($rsm->relationMap); $persister = $this->uow->getEntityPersister($entityName); - $region = $persister->getCacheRegion(); - $regionName = $region->getName(); + assert($persister instanceof CachedEntityPersister); + + $region = $persister->getCacheRegion(); + $regionName = $region->getName(); $cm = $this->em->getClassMetadata($entityName); - $generateKeys = function (array $entry) use ($cm): EntityCacheKey { + $generateKeys = static function (array $entry) use ($cm) : EntityCacheKey { return new EntityCacheKey($cm->rootEntityName, $entry['identifier']); }; $cacheKeys = new CollectionCacheEntry(array_map($generateKeys, $cacheEntry->result)); - $entries = $region->getMultiple($cacheKeys); + $entries = $region->getMultiple($cacheKeys) ?? []; // @TODO - move to cache hydration component foreach ($cacheEntry->result as $index => $entry) { - $entityEntry = is_array($entries) && array_key_exists($index, $entries) ? $entries[$index] : null; + $entityEntry = $entries[$index] ?? null; - if ($entityEntry === null) { + if (! $entityEntry instanceof EntityCacheEntry) { if ($this->cacheLogger !== null) { $this->cacheLogger->entityCacheMiss($regionName, $cacheKeys->identifiers[$index]); } @@ -145,9 +149,11 @@ public function get(QueryCacheKey $key, ResultSetMapping $rsm, array $hints = [] $data = $entityEntry->data; foreach ($entry['associations'] as $name => $assoc) { - $assocPersister = $this->uow->getEntityPersister($assoc['targetEntity']); - $assocRegion = $assocPersister->getCacheRegion(); - $assocMetadata = $this->em->getClassMetadata($assoc['targetEntity']); + $assocPersister = $this->uow->getEntityPersister($assoc['targetEntity']); + assert($assocPersister instanceof CachedEntityPersister); + + $assocRegion = $assocPersister->getCacheRegion(); + $assocMetadata = $this->em->getClassMetadata($assoc['targetEntity']); if ($assoc['type'] & ClassMetadata::TO_ONE) { @@ -267,7 +273,7 @@ public function put(QueryCacheKey $key, ResultSetMapping $rsm, $result, array $h $rootAlias = key($rsm->aliasMap); $persister = $this->uow->getEntityPersister($entityName); - if ( ! ($persister instanceof CachedPersister)) { + if (! $persister instanceof CachedEntityPersister) { throw CacheException::nonCacheableEntity($entityName); } diff --git a/lib/Doctrine/ORM/Cache/Region/DefaultMultiGetRegion.php b/lib/Doctrine/ORM/Cache/Region/DefaultMultiGetRegion.php index 2cd94292b4..7b5f28ecf3 100644 --- a/lib/Doctrine/ORM/Cache/Region/DefaultMultiGetRegion.php +++ b/lib/Doctrine/ORM/Cache/Region/DefaultMultiGetRegion.php @@ -21,6 +21,7 @@ namespace Doctrine\ORM\Cache\Region; use Doctrine\Common\Cache\MultiGetCache; +use Doctrine\ORM\Cache\CacheEntry; use Doctrine\ORM\Cache\CollectionCacheEntry; /** @@ -67,7 +68,12 @@ public function getMultiple(CollectionCacheEntry $collection) } $returnableItems = []; + foreach ($keysToRetrieve as $index => $key) { + if (! $items[$key] instanceof CacheEntry) { + return null; + } + $returnableItems[$index] = $items[$key]; } diff --git a/lib/Doctrine/ORM/Cache/Region/DefaultRegion.php b/lib/Doctrine/ORM/Cache/Region/DefaultRegion.php index be62a6fb95..ab069cefb5 100644 --- a/lib/Doctrine/ORM/Cache/Region/DefaultRegion.php +++ b/lib/Doctrine/ORM/Cache/Region/DefaultRegion.php @@ -94,7 +94,13 @@ public function contains(CacheKey $key) */ public function get(CacheKey $key) { - return $this->cache->fetch($this->getCacheEntryKey($key)) ?: null; + $entry = $this->cache->fetch($this->getCacheEntryKey($key)); + + if (! $entry instanceof CacheEntry) { + return null; + } + + return $entry; } /** @@ -108,7 +114,7 @@ public function getMultiple(CollectionCacheEntry $collection) $entryKey = $this->getCacheEntryKey($key); $entryValue = $this->cache->fetch($entryKey); - if ($entryValue === false) { + if (! $entryValue instanceof CacheEntry) { return null; } diff --git a/tests/Doctrine/Tests/ORM/Cache/DefaultQueryCacheTest.php b/tests/Doctrine/Tests/ORM/Cache/DefaultQueryCacheTest.php index 58eddd3750..ecfcc5207f 100644 --- a/tests/Doctrine/Tests/ORM/Cache/DefaultQueryCacheTest.php +++ b/tests/Doctrine/Tests/ORM/Cache/DefaultQueryCacheTest.php @@ -354,6 +354,34 @@ public function testGetWithAssociation() $this->assertEquals('Bar', $result[1]->getName()); } + public function testGetWithAssociationCacheMiss() : void + { + $rsm = new ResultSetMappingBuilder($this->em); + $key = new QueryCacheKey('query.key1', 0); + $entry = new QueryCacheEntry( + [ + ['identifier' => ['id' => 1]], + ['identifier' => ['id' => 2]], + ] + ); + + $this->region->addReturn('get', $entry); + + $this->region->addReturn( + 'getMultiple', + [ + new EntityCacheEntry(Country::class, ['id' => 1, 'name' => 'Foo']), + false, + ] + ); + + $rsm->addRootEntityFromClassMetadata(Country::class, 'c'); + + $result = $this->queryCache->get($key, $rsm); + + self::assertNull($result); + } + public function testCancelPutResultIfEntityPutFails() { $result = []; diff --git a/tests/Doctrine/Tests/ORM/Cache/DefaultRegionTest.php b/tests/Doctrine/Tests/ORM/Cache/DefaultRegionTest.php index 97365dfcf2..e50c2a55c3 100644 --- a/tests/Doctrine/Tests/ORM/Cache/DefaultRegionTest.php +++ b/tests/Doctrine/Tests/ORM/Cache/DefaultRegionTest.php @@ -95,6 +95,32 @@ public function testGetMulti() $this->assertEquals($value1, $actual[0]); $this->assertEquals($value2, $actual[1]); } + + /** + * @test + * @group GH7266 + */ + public function corruptedDataDoesNotLeakIntoApplicationWhenGettingSingleEntry() : void + { + $key1 = new CacheKeyMock('key.1'); + $this->cache->save($this->region->getName() . '_' . $key1->hash, 'a-very-invalid-value'); + + self::assertTrue($this->region->contains($key1)); + self::assertNull($this->region->get($key1)); + } + + /** + * @test + * @group GH7266 + */ + public function corruptedDataDoesNotLeakIntoApplicationWhenGettingMultipleEntries() : void + { + $key1 = new CacheKeyMock('key.1'); + $this->cache->save($this->region->getName() . '_' . $key1->hash, 'a-very-invalid-value'); + + self::assertTrue($this->region->contains($key1)); + self::assertNull($this->region->getMultiple(new CollectionCacheEntry([$key1]))); + } } /** diff --git a/tests/Doctrine/Tests/ORM/Cache/MultiGetRegionTest.php b/tests/Doctrine/Tests/ORM/Cache/MultiGetRegionTest.php index 0d98665d0d..5f45de4714 100644 --- a/tests/Doctrine/Tests/ORM/Cache/MultiGetRegionTest.php +++ b/tests/Doctrine/Tests/ORM/Cache/MultiGetRegionTest.php @@ -39,4 +39,17 @@ public function testGetMulti() $this->assertEquals($value1, $actual[0]); $this->assertEquals($value2, $actual[1]); } + + /** + * @test + * @group GH7266 + */ + public function corruptedDataDoesNotLeakIntoApplication() : void + { + $key1 = new CacheKeyMock('key.1'); + $this->cache->save($this->region->getName() . '_' . $key1->hash, 'a-very-invalid-value'); + + self::assertTrue($this->region->contains($key1)); + self::assertNull($this->region->getMultiple(new CollectionCacheEntry([$key1]))); + } }