8000 bug #19567 [Cache] Handle unserialize() failures gracefully (nicolas-… · symfony/symfony@74b8561 · GitHub
[go: up one dir, main page]
Skip to content

Commit 74b8561

Browse files
fabpotfabpot
committed
bug #19567 [Cache] Handle unserialize() failures gracefully (nicolas-grekas)
This PR was merged into the 3.1 branch. Discussion ---------- [Cache] Handle unserialize() failures gracefully | Q | A | ------------- | --- | Branch? | 3.1 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - This makes fetching cached items more resilient: `__PHP_Incomplete_Class` "objects" and other errors triggered by unserialize should be turned to cache misses. Commits ------- 47db638 [Cache] Handle unserialize() failures gracefully
2 parents 8e3967d + 47db638 commit 74b8561

File tree

8 files changed

+144
-23
lines changed

8 files changed

+144
-23
lines changed

‎src/Symfony/Component/Cache/Adapter/AbstractAdapter.php

Lines changed: 43 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -350,6 +350,33 @@ public function __destruct()
350350
}
351351
}
352352

353+
/**
354+
* Like the native unserialize() function but throws an exception if anything goes wrong.
355+
*
356+
* @param string $value
357+
*
358+
* @return mixed
359+
*
360+
* @throws \Exception
361+
*/
362+
protected static function unserialize($value)
363+
{
364+
if ('b:0;' === $value) {
365+
return false;
366+
}
367+
$unserializeCallbackHandler = ini_set('unserialize_callback_func', __CLASS__.'::handleUnserializeCallback');
368+
try {
369+
if (false !== $value = unserialize($value)) {
370+
return $value;
371+
}
372+
throw new \DomainException('Failed to unserialize cached value');
373+
} catch (\Error $e) {
374+
throw new \ErrorException($e->getMessage(), $e->getCode(), E_ERROR, $e->getFile(), $e->getLine());
375+
} finally {
376+
ini_set('unserialize_callback_func', $unserializeCallbackHandler);
377+
}
378+
}
379+
353380
private function getId($key)
354381
{
355382
CacheItem::validateKey($key);
@@ -361,13 +388,26 @@ private function generateItems($items, &$keys)
361388
{
362389
$f = $this->createCacheItem;
363390

364-
foreach ($items as $id => $value) {
365-
yield $keys[$id] => $f($keys[$id], $value, true);
366-
unset($keys[$id]);
391+
try {
392+
foreach ($items as $id => $value) {
393+
$key = $keys[$id];
394+
unset($keys[$id]);
395+
yield $key => $f($key, $value, true);
396+
}
397+
} catch (\Exception $e) {
398+
CacheItem::log($this->logger, 'Failed to fetch requested items', array('keys' => array_values($keys), 'exception' => $e));
367399
}
368400

369401
foreach ($keys as $key) {
370402
yield $key => $f($key, null, false);
371403
}
372404
}
405+
406+
/**
407+
* @internal
408+
*/
409+
public static function handleUnserializeCallback($class)
410+
{
411+
throw new \DomainException('Class not found: '.$class);
412+
}
373413
}

‎src/Symfony/Component/Cache/Adapter/ApcuAdapter.php

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,11 @@ public function __construct($namespace = '', $defaultLifetime = 0, $version = nu
4949
*/
5050
protected function doFetch(array $ids)
5151
{
52-
return apcu_fetch($ids);
52+
try {
53+
return apcu_fetch($ids);
54+
} catch (\Error $e) {
55+
throw new \ErrorException($e->getMessage(), $e->getCode(), E_ERROR, $e->getFile(), $e->getLine());
56+
}
5357
}
5458

5559
/**

‎src/Symfony/Component/Cache/Adapter/ArrayAdapter.php

Lines changed: 35 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -55,12 +55,22 @@ function ($key, $value, $isHit) use ($defaultLifetime) {
5555
*/
5656
public function getItem($key)
5757
{
58-
if (!$isHit = $this->hasItem($key)) {
58+
$isHit = $this->hasItem($key);
59+
try {
60+
if (!$isHit) {
61+
$value = null;
62+
} elseif (!$this->storeSerialized) {
63+
$value = $this->values[$key];
64+
} elseif ('b:0;' === $value = $this->values[$key]) {
65+
$value = false;
66+
} elseif (false === $value = unserialize($value)) {
67+
$value = null;
68+
$isHit = false;
69+
}
70+
} catch (\Exception $e) {
71+
CacheItem::log($this->logger, 'Failed to unserialize key "{key}"', array('key' => $key, 'exception' => $e));
5972
$value = null;
60-
} elseif ($this->storeSerialized) {
61-
$value = unserialize($this->values[$key]);
62-
} else {
63-
$value = $this->values[$key];
73+
$isHit = false;
6474
}
6575
$f = $this->createCacheItem;
6676

@@ -181,16 +191,30 @@ private function generateItems(array $keys, $now)
181191
{
182192
$f = $this->createCacheItem;
183193

184-
foreach ($keys as $key) {
185-
if (!$isHit = isset($this->expiries[$key]) && ($this->expiries[$key] >= $now || !$this->deleteItem($key))) {
194+
foreach ($keys as $i => $key) {
195+
try {
196+
if (!$isHit = isset($this->expiries[$key]) && ($this->expiries[$key] >= $now || !$this->deleteItem($key))) {
197+
$value = null;
198+
} elseif (!$this->storeSerialized) {
199+
$value = $this->values[$key];
200+
} elseif ('b:0;' === $value = $this->values[$key]) {
201+
$value = false;
202+
} elseif (false === $value = unserialize($value)) {
203+
$value = null;
204+
$isHit = false;
205+
}
206+
} catch (\Exception $e) {
207+
CacheItem::log($this->logger, 'Failed to unserialize key "{key}"', array('key' => $key, 'exception' => $e));
186208
$value = null;
187-
} elseif ($this->storeSerialized) {
188-
$value = unserialize($this->values[$key]);
189-
} else {
190-
$value = $this->values[$key];
209+
$isHit = false;
191210
}
211+
unset($keys[$i]);
192212

193213
yield $key => $f($key, $value, $isHit);
194214
}
215+
216+
foreach ($keys as $key) {
217+
yield $key => $f($key, null, false);
218+
}
195219
}
196220
}

‎src/Symfony/Component/Cache/Adapter/DoctrineAdapter.php

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,25 @@ public function __construct(CacheProvider $provider, $namespace = '', $defaultLi
3232
*/
3333
protected function doFetch(array $ids)
3434
{
35-
return $this->provider->fetchMultiple($ids);
35+
$unserializeCallbackHandler = ini_set('unserialize_callback_func', parent::class.'::handleUnserializeCallback');
36+
try {
37+
return $this->provider->fetchMultiple($ids);
38+
} catch (\Error $e) {
39+
$trace = $e->getTrace();
40+
41+
if (isset($trace[0]['function']) && !isset($trace[0]['class'])) {
42+
switch ($trace[0]['function']) {
43+
case 'unserialize':
44+
case 'apcu_fetch':
45+
case 'apc_fetch':
46+
throw new \ErrorException($e->getMessage(), $e->getCode(), E_ERROR, $e->getFile(), $e->getLine());
47+
}
48+
}
49+
50+
throw $e;
51+
} finally {
52+
ini_set('unserialize_callback_func', $unserializeCallbackHandler);
53+
}
3654
}
3755

3856
/**

‎src/Symfony/Component/Cache/Adapter/FilesystemAdapter.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ protected function doFetch(array $ids)
6060

6161
foreach ($ids as $id) {
6262
$file = $this->getFile($id);
63-
if (!$h = @fopen($file, 'rb')) {
63+
if (!file_exists($file) || !$h = @fopen($file, 'rb')) {
6464
continue;
6565
}
6666
if ($now >= (int) $expiresAt = fgets($h)) {
@@ -73,7 +73,7 @@ protected function doFetch(array $ids)
7373
$value = stream_get_contents($h);
7474
fclose($h);
7575
if ($i === $id) {
76-
$values[$id] = unserialize($value);
76+
$values[$id] = parent::unserialize($value);
7777
}
7878
}
7979
}

‎src/Symfony/Component/Cache/Adapter/RedisAdapter.php

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -134,19 +134,15 @@ public static function createConnection($dsn, array $options = array())
134134
*/
135135
protected function doFetch(array $ids)
136136
{
137-
$result = array();
138-
139137
if ($ids) {
140138
$values = $this->redis->mGet($ids);
141139
$index = 0;
142140
foreach ($ids as $id) {
143141
if ($value = $values[$index++]) {
144-
$result[$id] = unserialize($value);
142+
yield $id => parent::unserialize($value);
145143
}
146144
}
147145
}
148-
149-
return $result;
150146
}
151147

152148
/**

‎src/Symfony/Component/Cache/Tests/Adapter/AdapterTestCase.php

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,4 +46,42 @@ public function testDefaultLifeTime()
4646
$item = $cache->getItem('key.dlt');
4747
$this->assertFalse($item->isHit());
4848
}
49+
50+
public function testNotUnserializable()
51+
{
52+
if (isset($this->skippedTests[__FUNCTION__])) {
53+
$this->markTestSkipped($this->skippedTests[__FUNCTION__]);
54+
55+
return;
56+
}
57+
58+
$cache = $this->createCachePool();
59+
60+
$item = $cache->getItem('foo');
61+
$cache->save($item->set(new NotUnserializable()));
62+
63+
$item = $cache->getItem('foo');
64+
$this->assertFalse($item->isHit());
65+
66+
foreach ($cache->getItems(array('foo')) as $item) {
67+
}
68+
$cache->save($item->set(new NotUnserializable()));
69+
70+
foreach ($cache->getItems(array('foo')) as $item) {
71+
}
72+
$this->assertFalse($item->isHit());
73+
}
74+
}
75+
76+
class NotUnserializable implements \Serializable
77+
{
78+
public function serialize()
79+
{
80+
return serialize(123);
81+
}
82+
83+
public function unserialize($ser)
84+
{
85+
throw new \Exception(__CLASS__);
86+
}
4987
}

‎src/Symfony/Component/Cache/Tests/Adapter/DoctrineAdapterTest.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ class DoctrineAdapterTest extends AdapterTestCase
2222
protected $skippedTests = array(
2323
'testDeferredSaveWithoutCommit' => 'Assumes a shared cache which ArrayCache is not.',
2424
'testSaveWithoutExpire' => 'Assumes a shared cache which ArrayCache is not.',
25+
'testNotUnserializable' => 'ArrayCache does not use serialize/unserialize',
2526
);
2627

2728
public function createCachePool($defaultLifetime = 0)

0 commit comments

Comments
 (0)
0