Skip to content

fix(cache/redis): use connectionAwareSerialize in RedisStore::putMany() #55814

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

Open
wants to merge 1 commit into
base: 12.x
Choose a base branch
from

Conversation

superbiche
Copy link

Cache RedisStore::putMany() should not serialize values if the connection already uses serialization.

This prevents a bug with the following setup:

  • Using Redis cache driver
  • Using phpredis client
  • Configuring phpredis to use serialization - eg "serializer" => Redis::SERIALIZER_IGBINARY

When using Cache::flexible(), the value returned is correct only the first time. Subsequent calls return a PHP serialized value:

> Cache::flexible('test:flex', [now()->addHours(12), now()->addHours(24)], fn () => ["test" => "ok"] )
= [
    "test" => "ok",
  ]

> Cache::flexible('test:flex', [now()->addHours(12), now()->addHours(24)], fn () => ["test" => "ok"])
= "a:1:{s:4:"test";s:2:"ok";}"

This is caused by RedisStore::putMany() method, which - contrarily to other methods in this class - doesn't use RedisStore::connectionAwareSerialize.
In the meantime, RedisStore::many() method uses RedisStore::connectionAwareUnserialize(), so it doesn't unserialize the value that is returned.

This PR fixes this inconsistent behavior by changing RedisStore::putMany() so it uses RedisStore::connectionAwareSerialize.

The result is what one should expect:

> Cache::flexible('test:flex', [now()->addHours(12), now()->addHours(24)], fn () => ["test" => "ok"] )
= [
    "test" => "ok",
  ]

> Cache::flexible('test:flex', [now()->addHours(12), now()->addHours(24)], fn () => ["test" => "ok"] )
= [
    "test" => "ok",
  ]

Cache RedisStore::putMany() should not serialize values if the connection already uses serialization.
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

Successfully merging this pull request may close these issues.

1 participant