guzzle-cache-middleware icon indicating copy to clipboard operation
guzzle-cache-middleware copied to clipboard

Storing PHP serialized data in caches MUST be avoided

Open bburnichon opened this issue 2 years ago • 2 comments

I was bitten by this several times already.

You have a running application in production heavily relying on cached data.

You upgrade a PHP dependency like going from 3.5.0 to 4.0.1, everything is working as expected during tests. You confidently push code to production.

Everything breaks in production, and you have a hard time figuring why. You revert the changes and the issue does not stop until you somehow manage to either clear all caches (if your infrastructure can support it) or wait for the cache TTL.

What happened: stored data is incompatible between the 2 versions. Some properties have changed in CacheEntry https://github.com/Kevinrob/guzzle-cache-middleware/compare/v3.5.0...v4.0.1#diff-cc5ac6f8871a586dc5f99a175c4e7d9a9f9093c11638546b5b7d176daa28a271

Already cached data contains a messageBody property which is then unused by new code. PHP unserialization does not tell you anything but created objects are in invalid state.

For this reason, I encourage to avoid storing anything other than string in caches and handle serialization elsewhere.

At the moment, I have a custom PSR16 cache implementation to avoid the issue. I think having something handling serialization of CacheEntry as raw_string or at least, normalized data (stdClass, array or scalar values) which is immune to this issue

bburnichon avatar Jul 15 '22 08:07 bburnichon

I encountered this issue as well when doing the dependency upgrade. I did some digging and it seems like https://github.com/Kevinrob/guzzle-cache-middleware/pull/156 is the culprit. The release notes should probably be updated to note this breaking change more prominently (and how the fix is to clear the cache).

EricTendian avatar Jul 17 '22 03:07 EricTendian

I also encountered this issue, took ages to find the root cause. Clearing out the cache solved the issue for me too.

seanhamlin avatar Dec 20 '22 20:12 seanhamlin