[Cache] Handle unserialize() failures gracefully #19567
Conversation
2915a08 to
1e151a6
Compare
b18a56f to
448de93
Compare
| $isHit = false; | ||
| } | ||
| } catch (\Exception $e) { | ||
| CacheItem::log($this->logger, 'Failed to unserialize key "{key}"', array('key' => $key, 'exception' => $e)); |
There was a problem hiding this comment.
It should not happen as this adapter is not cross requests (so something not unserializable won't be cached as it is not serializable as well).
If you want to keep this anyway then i guess you should use parent::unserialize below ?
There was a problem hiding this comment.
Ok got it it might break in case of a custom unserializer.
At least using parent::unserialize would allow you to remove the case managing false.
There was a problem hiding this comment.
There is no parent here, and calling the same implementation would call ini_set twice for something that can't happen because as you said, this is not cross reqs
There was a problem hiding this comment.
You're right sorry, i should definitely stop reviewing on my phone...
448de93 to
5a8dbbe
Compare
| } | ||
| } catch (\Exception $e) { |
There was a problem hiding this comment.
Shouldn't this be moved inside the foreach to fetch as many values as possible ?
There was a problem hiding this comment.
right, done: try/catch moved inside the foreach
|
LGTM 👍 |
20184d0 to
69cfa83
Compare
69cfa83 to
47db638
Compare
|
Thank you @nicolas-grekas. |
…grekas) This PR was merged into the 3.1 bran 7E67 ch. 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
This makes fetching cached items more resilient:
__PHP_Incomplete_Class"objects" and other errors triggered by unserialize should be turned to cache misses.