search

sybrew / The-SEO-Framework-Extension-Manager

A WordPress plugin that manages extensions for The SEO Framework.
https://tsf.fyi/em
GNU General Public License v3.0
76 stars 9 forks source link

Corrupted metadata when using the persistent object cache plugin #44

Closed vralle closed 3 years ago

vralle commented 3 years ago

Describe the bug Serialization error with Docket Cache plugin (a persistent WordPress Object Cache plugin)

To Reproduce

  1. Install and activate opcache zend extension
  2. Install and activate Docket Cache plugin
  3. See serialize/unserialize error log. https://github.com/sybrew/The-SEO-Framework-Extension-Manager/blob/1adbf22ccdc99465e9d658ddfd4cca67228cb4ed/inc/traits/extension/post-meta.trait.php#L62

Expected behavior The request can be retrieved from the cache as a PHP value.

Additional context fix: replace unserialize -> maybe_unserialize

sybrew commented 3 years ago

Hi Vitaliy,

I like how you paid homage to the commit's title of https://github.com/sybrew/The-SEO-Framework-Extension-Manager/commit/11150f5790ba279359c90051dccfa48a30054e73 😄

Anyway, this is not the first time I encountered an issue like this. https://wordpress.org/support/topic/can-destroy-serialized-data/

maybe_unserialize() mitigates double-unserialization issues; but introduces them too when not tightly controlled. This should not be an issue in our case since we use a Key-Value store; yet, using maybe_unserialize() would fix a symptom of an underlying problem.

I'm not sure what Docket Cache does to fault the metadata -- I believe they might try to unserialize the data and not return it as is intended, but I haven't read the code. I suggest reaching out to them about this issue. My inbox is open for communication.

vralle commented 3 years ago

The PHP optimizer returns computed values and does not know anything about the expected data type. Of the 26 active plugins, the serialization error only occurs in the seo extension manager. I'm amazed at the need to choose between site speed and some structured data.

sybrew commented 5 months ago

I just looked into this again, and Docket Cache deeply unserializes data in several passes. This behavior is different from WordPress's; thus, we couldn't anticipate that, and the data type changed from a string to an array.

Still, I'm going to add more safeguards to mitigate this issue and resolve it for Docket Cache.