Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Storing PHP serialized data in caches MUST be avoided #165

Open
bburnichon opened this issue Jul 15, 2022 · 2 comments
Open

Storing PHP serialized data in caches MUST be avoided #165

bburnichon opened this issue Jul 15, 2022 · 2 comments

Comments

@bburnichon
Copy link

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 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

@EricTendian
Copy link

I encountered this issue as well when doing the dependency upgrade. I did some digging and it seems like #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).

@seanhamlin
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants