Redis & RedisCluster eval with serializer & compression

[TheLevti]

• Laravel Version: 8.28.1
• PHP Version: 8.0.2
• Database Driver & Version: phpredis 5.3.3

Description:

The eval call is missing proper argument serialization and compression if php-redis extension is configured to do so.

https://github.com/laravel/framework/blob/8.x/src/Illuminate/Cache/RedisLock.php#L51

Steps To Reproduce:

Install igbinary, install phpredis, configure laravel to use redis as cache driver. Use the lock functionality as described here: https://laravel.com/docs/8.x/cache#atomic-locks

In the following section you can see the code example, the console and the redis monitor output.

No serializer enabled

Igbinary serializer enabled

As you can see in the second example the final lock release is not happening and the lock stays in redis until it times out or if no timeout set, will stay there forever. The reason for this is that the phpredis extension can not know which arguments are keys and which are values that need to be serialized and thus the responsibility to do so is in the hands of the extension user to serialize the values in the same way as the extension is currently configured. I have fixed another lock library already, but unfortunately I am not aware how to contribute in the laravel framework.

The solution would be in https://github.com/laravel/framework/blob/8.x/src/Illuminate/Cache/RedisLock.php#L51

/**
 * Release the lock.
 *
 * @return bool
 */
public function release()
{
    /* If a serialization mode such as "php" or "igbinary" is enabled, the arguments must be
     * serialized by us, because phpredis does not do this for the eval command.
     *
     * The keys must not be serialized.
     */
    $owner = $this->owner;
    $client = $this->redis->client();
    $clientClass = get_class($client);
    if ($clientClass === 'Redis' || $clientClass === 'RedisCluster') {
        $owner = $client->_serialize($owner);
    }

    return (bool) $this->redis->eval(LuaScripts::releaseLock(), 1, $this->name, $owner);
}

I have tested this code and it works.

IMPORTANT

A similar change needs to be done for the compression options. There, we have no access to an internal function like with the serializer (_serialize($x)), but rather have to check the options that are set ($redis->getOption(Redis::OPT_COMPRESSION);).

This fix would be also a pre condition to introduce those phpredis options to the general redis config.

[driesvints]

Thanks. I don't have experience with Redis clusters myself but your fix seems straight forward to me. Can you send in a PR for that?

[TheLevti]

Sure, will prepare a pull request these days.

[TheLevti]

Thanks. I don't have experience with Redis clusters myself but your fix seems straight forward to me. Can you send in a PR for that?

Pull reques submitted (#36412). As a result I also submitted two phpredis issues where one needs investigation/fix (phpredis/phpredis#1939) and one would be a good to have (phpredis/phpredis#1938).

[driesvints]

Thanks @TheLevti!