framework icon indicating copy to clipboard operation
framework copied to clipboard

RedisStore throw an unserialization notice when a key used by an atomic lock is checked.

Open juanparati opened this issue 1 year ago • 3 comments

  • Laravel Version: v9.24.0 and v9.30.0
  • PHP Version: 8.0.17 and 8.1.9
  • Redis Driver & Version: Redis 5.3.7 using PHPRedis with php, json, igbinary serializers

Description:

I observed that Laravel raises a "PHP Notice" when a key used by an atomic lock is verified with the "has" method.

Steps To Reproduce:

  1. Remove the 'lock_connection' from the redis configuration into cache.php config file.
  2. Run artisan cache:clear
  3. In Laravel Tinker type:
\Cache::lock('testlock', 20)->get();
\Cache::has('testlock');

Received result

PHP Notice:  unserialize(): Error at offset 0 of 16 bytes in /home/vagrant/testproject/code/vendor/laravel/framework/src/Illuminate/Cache/RedisStore.php on line 345
true

Expected result

Do not raise notice.

Notes

The result is received correctly however with a notice. I am not sure if to use the "has" method in order to verify if a lock was acquire without to previously acquire the lock is good practice.

juanparati avatar Sep 20 '22 09:09 juanparati

Does this also happens on the latest Laravel release?

driesvints avatar Sep 20 '22 09:09 driesvints

Hi Dries, I can reproduce with 9.30, however I was doing some research and I think I found the reason.

It seems that my cache.php config file has missing the 'lock_connection' parameter (My cache.php config file is a legacy one from an old Laravel version). When this parameter is missing then I can reproduce this issue (I edited the original bug report).

I observed that when the parameter is present the behavior is also different, so when 'lock_connection' => 'default' is present the "has" method will not find the key used in the lock and the notice is not raised.

Example:

Psy Shell v0.11.8 (PHP 8.0.17 — cli) by Justin Hileman
>>> \Cache::lock('testlock1', 20)->get();
=> true

>>> \Cache::has('testlock1');
=> false

If I remove the "lock_connection" from the configuration the "has" method will be able to find the key used. Example:

Psy Shell v0.11.8 (PHP 8.0.17 — cli) by Justin Hileman
>>> \Cache::lock('testlock1', 20)->get();
=> true

>>> \Cache::has('testlock1');
PHP Notice:  unserialize(): Error at offset 0 of 16 bytes in /home/vagrant/test/laravel/vendor/laravel/framework/src/Illuminate/Cache/RedisStore.php on line 345
=> true

juanparati avatar Sep 20 '22 11:09 juanparati

It's logic that "has" method cannot find the key used in the atomic lock because the "lock_connection" indicates a different "connection" for the locks.

juanparati avatar Sep 20 '22 11:09 juanparati

Ow I see. Yeah just don't remove that parameter from your config please.

driesvints avatar Sep 23 '22 12:09 driesvints