Skip to content

Commit 5f69f1d

Browse files
cryptiklemurcryptiklemur
committed
fix(cache): treat unserialize failures as cache misses
1 parent 6394f7e commit 5f69f1d

25 files changed

Lines changed: 412 additions & 52 deletions

‎src/Adapter/Apcu/ApcuCachePool.php‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
use Cache\Adapter\Common\AbstractCachePool;
1515
use Cache\Adapter\Common\PhpCacheItem;
16+
use Cache\Adapter\Common\PhpUnserializer;
1617
use Cache\Adapter\Common\TagSupportWithArray;
1718

1819
/**
@@ -36,8 +37,10 @@ protected function fetchObjectFromCache(string $key): array
3637
}
3738

3839
$success = false;
39-
$record = apcu_fetch($key, $success);
40-
if (!$success || !\is_array($record) || !array_is_list($record) || 3 !== \count($record)) {
40+
$complete = PhpUnserializer::decodeWith(static function () use ($key, &$success): mixed {
41+
return apcu_fetch($key, $success);
42+
}, $record);
43+
if (!$complete || !$success || !\is_array($record) || !array_is_list($record) || 3 !== \count($record)) {
4144
return [false, null, [], null];
4245
}
4346

@@ -102,7 +105,7 @@ private function skipIfCli(): bool
102105

103106
public function getDirectValue(string $name): mixed
104107
{
105-
return apcu_fetch($name);
108+
return PhpUnserializer::decodeWith(static fn (): mixed => apcu_fetch($name), $value) ? $value : null;
106109
}
107110

108111
public function setDirectValue(string $name, mixed $value): bool

‎src/Adapter/Apcu/Tests/ApcuCachePoolTest.php‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,22 +12,47 @@
1212
namespace Cache\Adapter\Apcu\Tests;
1313

1414
use Cache\Adapter\Apcu\ApcuCachePool;
15+
use Cache\Adapter\Common\Exception\CachePoolException;
1516
use PHPUnit\Framework\Attributes\DataProvider;
1617
use PHPUnit\Framework\Attributes\PreserveGlobalState;
1718
use PHPUnit\Framework\Attributes\RunInSeparateProcess;
1819
use PHPUnit\Framework\TestCase;
1920

2021
final class ApcuFunctionStub
2122
{
23+
public static ?\Throwable $exception = null;
24+
25+
public static ?string $missingClass = null;
26+
27+
public static ?string $serializedValue = null;
28+
2229
public static mixed $storedValue = null;
2330

2431
public static bool $success = true;
2532

33+
public static bool $throwUnserializationException = false;
34+
2635
public static ?string $storedKey = null;
2736

2837
public static ?int $storedTtl = null;
2938
}
3039

40+
final class ApcuUnserializationFailure
41+
{
42+
public function __unserialize(array $data)
43+
{
44+
throw new \Error('cached payload could not be decoded');
45+
}
46+
}
47+
48+
final class ApcuUnserializationException
49+
{
50+
public function __unserialize(array $data)
51+
{
52+
throw new \RuntimeException('cached payload could not be decoded');
53+
}
54+
}
55+
3156
final class ApcuCachePoolTest extends TestCase
3257
{
3358
#[RunInSeparateProcess]
@@ -80,4 +105,59 @@ public static function invalidPayloads(): iterable
80105
yield 'invalid tags' => [['value', [42], null]];
81106
yield 'invalid expiration' => [['value', [], 'tomorrow']];
82107
}
108+
109+
#[RunInSeparateProcess]
110+
#[PreserveGlobalState(false)]
111+
public function testIncompleteClassPayloadIsACacheMiss()
112+
{
113+
require_once __DIR__.'/Fixtures/apcu_functions.php';
114+
115+
ApcuFunctionStub::$missingClass = 'GoneType';
116+
ApcuFunctionStub::$storedValue = ['value', [], null];
117+
ApcuFunctionStub::$success = true;
118+
119+
self::assertFalse((new ApcuCachePool())->getItem('incomplete')->isHit());
120+
}
121+
122+
#[RunInSeparateProcess]
123+
#[PreserveGlobalState(false)]
124+
public function testUnserializationErrorIsACacheMiss()
125+
{
126+
require_once __DIR__.'/Fixtures/apcu_functions.php';
127+
128+
ApcuFunctionStub::$serializedValue = serialize([new ApcuUnserializationFailure(), [], null]);
129+
ApcuFunctionStub::$success = true;
130+
131+
self::assertFalse((new ApcuCachePool())->getItem('broken')->isHit());
132+
}
133+
134+
#[RunInSeparateProcess]
135+
#[PreserveGlobalState(false)]
136+
public function testUnserializationExceptionIsACacheMiss()
137+
{
138+
require_once __DIR__.'/Fixtures/apcu_functions.php';
139+
140+
ApcuFunctionStub::$storedValue = ['value', [], null];
141+
ApcuFunctionStub::$success = true;
142+
ApcuFunctionStub::$throwUnserializationException = true;
143+
144+
self::assertFalse((new ApcuCachePool())->getItem('broken')->isHit());
145+
}
146+
147+
#[RunInSeparateProcess]
148+
#[PreserveGlobalState(false)]
149+
public function testBackendFetchExceptionIsNotTreatedAsCacheMiss()
150+
{
151+
require_once __DIR__.'/Fixtures/apcu_functions.php';
152+
153+
$backendException = new \RuntimeException('backend failed');
154+
ApcuFunctionStub::$exception = $backendException;
155+
156+
try {
157+
(new ApcuCachePool())->getItem('key')->isHit();
158+
self::fail('The backend exception was not propagated.');
159+
} catch (CachePoolException $exception) {
160+
self::assertSame($backendException, $exception->getPrevious());
161+
}
162+
}
83163
}

‎src/Adapter/Apcu/Tests/Fixtures/apcu_functions.php‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,26 @@
1212
namespace Cache\Adapter\Apcu;
1313

1414
use Cache\Adapter\Apcu\Tests\ApcuFunctionStub;
15+
use Cache\Adapter\Apcu\Tests\ApcuUnserializationException;
1516

1617
function apcu_fetch(mixed $key, ?bool &$success = null): mixed
1718
{
19+
if (null !== ApcuFunctionStub::$exception) {
20+
throw ApcuFunctionStub::$exception;
21+
}
22+
if (null !== ApcuFunctionStub::$missingClass) {
23+
spl_autoload_call(ApcuFunctionStub::$missingClass);
24+
}
25+
if (ApcuFunctionStub::$throwUnserializationException) {
26+
(new ApcuUnserializationException())->__unserialize([]);
27+
}
28+
1829
$success = ApcuFunctionStub::$success;
1930

31+
if (null !== ApcuFunctionStub::$serializedValue) {
32+
return unserialize(ApcuFunctionStub::$serializedValue);
33+
}
34+
2035
return ApcuFunctionStub::$storedValue;
2136
}
2237

‎src/Adapter/Common/PhpUnserializer.php‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
<?php
2+
3+
/*
4+
* This file is part of php-cache organization.
5+
*
6+
* (c) 2015 Aaron Scherer <aequasi@gmail.com>, Tobias Nyholm <tobias.nyholm@gmail.com>
7+
*
8+
* This source file is subject to the MIT license that is bundled
9+
* with this source code in the file LICENSE.
10+
*/
11+
12+
namespace Cache\Adapter\Common;
13+
14+
final class PhpUnserializer
15+
{
16+
public static function unserialize(string $payload, mixed &$value): bool
17+
{
18+
try {
19+
return self::decodeWith(static fn (): mixed => @unserialize($payload), $value)
20+
&& (false !== $value || 'b:0;' === $payload);
21+
} catch (\Throwable) {
22+
return false;
23+
}
24+
}
25+
26+
/**
27+
* @param \Closure(): mixed $decoder
28+
*/
29+
public static function decodeWith(\Closure $decoder, mixed &$value): bool
30+
{
31+
$autoloadedClasses = [];
32+
$trackAutoload = static function (string $class) use (&$autoloadedClasses) {
33+
$autoloadedClasses[$class] = true;
34+
};
35+
spl_autoload_register($trackAutoload, true, true);
36+
37+
try {
38+
try {
39+
$value = $decoder();
40+
} catch (\Throwable $exception) {
41+
if ($exception instanceof \Error || self::isUnserializationFailure($exception)) {
42+
return false;
43+
}
44+
45+
throw $exception;
46+
}
47+
} finally {
48+
spl_autoload_unregister($trackAutoload);
49+
}
50+
51+
foreach (array_keys($autoloadedClasses) as $class) {
52+
if (!class_exists($class, false)) {
53+
return false;
54+
}
55+
}
56+
57+
return true;
58+
}
59+
60+
private static function isUnserializationFailure(\Throwable $exception): bool
61+
{
62+
foreach ($exception->getTrace() as $frame) {
63+
if (\in_array($frame['function'], ['unserialize', '__unserialize', '__wakeup'], true)) {
64+
return true;
65+
}
66+
}
67+
68+
return false;
69+
}
70+
}

‎src/Adapter/Common/Tests/Fixtures/AutoloadedValue.php‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
<?php
2+
3+
/*
4+
* This file is part of php-cache organization.
5+
*
6+
* (c) 2015 Aaron Scherer <aequasi@gmail.com>, Tobias Nyholm <tobias.nyholm@gmail.com>
7+
*
8+
* This source file is subject to the MIT license that is bundled
9+
* with this source code in the file LICENSE.
10+
*/
11+
12+
namespace Cache\Adapter\Common\Tests\Fixtures;
13+
14+
final class AutoloadedValue
15+
{
16+
}

‎src/Adapter/Common/Tests/PhpUnserializerTest.php‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
<?php
2+
3+
/*
4+
* This file is part of php-cache organization.
5+
*
6+
* (c) 2015 Aaron Scherer <aequasi@gmail.com>, Tobias Nyholm <tobias.nyholm@gmail.com>
7+
*
8+
* This source file is subject to the MIT license that is bundled
9+
* with this source code in the file LICENSE.
10+
*/
11+
12+
namespace Cache\Adapter\Common\Tests;
13+
14+
use Cache\Adapter\Common\PhpUnserializer;
15+
use Cache\Adapter\Common\Tests\Fixtures\AutoloadedValue;
16+
use PHPUnit\Framework\TestCase;
17+
18+
final class PhpUnserializerTest extends TestCase
19+
{
20+
public function testRejectsMalformedPayloadThatDecodesToFalse()
21+
{
22+
set_error_handler(static fn (): bool => true);
23+
24+
try {
25+
self::assertFalse(PhpUnserializer::unserialize('not serialized', $value));
26+
} finally {
27+
restore_error_handler();
28+
}
29+
}
30+
31+
public function testAcceptsSerializedFalse()
32+
{
33+
self::assertTrue(PhpUnserializer::unserialize('b:0;', $value));
34+
self::assertFalse($value);
35+
}
36+
37+
public function testRejectsIncompleteClassInInternalObjectStorage()
38+
{
39+
$payload = str_replace('stdClass', 'GoneType', serialize(new \ArrayObject([new \stdClass()])));
40+
41+
self::assertFalse(PhpUnserializer::unserialize($payload, $value));
42+
}
43+
44+
public function testAcceptsClassResolvedByAutoloader()
45+
{
46+
$class = AutoloadedValue::class;
47+
self::assertFalse(class_exists($class, false));
48+
$autoload = static function (string $requestedClass) use ($class) {
49+
if ($class === $requestedClass) {
50+
require_once __DIR__.'/Fixtures/AutoloadedValue.php';
51+
}
52+
};
53+
spl_autoload_register($autoload);
54+
55+
try {
56+
$payload = \sprintf('O:%d:"%s":0:{}', \strlen($class), $class);
57+
58+
self::assertTrue(PhpUnserializer::unserialize($payload, $value));
59+
self::assertInstanceOf($class, $value);
60+
} finally {
61+
spl_autoload_unregister($autoload);
62+
}
63+
}
64+
65+
public function testRejectsReserializedIncompleteClass()
66+
{
67+
$incomplete = @unserialize('O:8:"GoneType":0:{}');
68+
69+
self::assertFalse(PhpUnserializer::unserialize(serialize($incomplete), $value));
70+
}
71+
}

‎src/Adapter/Doctrine/DoctrineCachePool.php‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
use Cache\Adapter\Common\AbstractCachePool;
1515
use Cache\Adapter\Common\PhpCacheItem;
16+
use Cache\Adapter\Common\PhpUnserializer;
1617
use Doctrine\Common\Cache\Cache;
1718
use Doctrine\Common\Cache\FlushableCache;
1819

@@ -38,9 +39,7 @@ protected function fetchObjectFromCache(string $key): array
3839
return [false, null, [], null];
3940
}
4041

41-
try {
42-
$record = @unserialize($payload);
43-
} catch (\Throwable) {
42+
if (!PhpUnserializer::unserialize($payload, $record)) {
4443
return [false, null, [], null];
4544
}
4645

‎src/Adapter/Doctrine/Tests/DoctrineAdapterTest.php‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
namespace Cache\Adapter\Doctrine\Tests;
1313

14+
use Cache\Adapter\Common\Exception\CachePoolException;
1415
use Cache\Adapter\Doctrine\DoctrineCachePool;
1516
use Doctrine\Common\Cache\Cache;
1617
use Doctrine\Common\Cache\FlushableCache;
@@ -69,6 +70,7 @@ public static function invalidPayloads(): iterable
6970
yield 'invalid hit marker' => [serialize([false, 'value', [], null])];
7071
yield 'invalid tags' => [serialize([true, 'value', [42], null])];
7172
yield 'invalid expiration' => [serialize([true, 'value', [], 'tomorrow'])];
73+
yield 'incomplete class' => [str_replace('stdClass', 'GoneType', serialize([true, new \stdClass(), [], null]))];
7274
}
7375

7476
public function testCorruptTagListIsIgnored()
@@ -79,6 +81,19 @@ public function testCorruptTagListIsIgnored()
7981
self::assertTrue($this->pool->invalidateTag('corrupt'));
8082
}
8183

84+
public function testBackendFetchExceptionIsNotTreatedAsCacheMiss()
85+
{
86+
$backendException = new \RuntimeException('backend failed');
87+
$this->mockDoctrine->shouldReceive('fetch')->once()->with('key')->andThrow($backendException);
88+
89+
try {
90+
$this->pool->getItem('key')->isHit();
91+
self::fail('The backend exception was not propagated.');
92+
} catch (CachePoolException $exception) {
93+
self::assertSame($backendException, $exception->getPrevious());
94+
}
95+
}
96+
8297
public function testInvalidatingAnExistingTagReportsABackendFailure()
8398
{
8499
$this->mockDoctrine->shouldReceive('fetch')->once()->with('tag!tag')->andReturn([]);

0 commit comments

Comments
 (0)