Igbinary serializer in Redis cache driver not working properly

[andreyzp87]

• Laravel Version: 8.83.25
• PHP Version: 8.1.1
• Database Driver & Version:
  Redis 6.0.6 on Ubuntu 22.04
  Redis 6.2.7 on Fedora 36
  PhpRedis 5.3.7

Description:

When you enable igbinary serializer for redis in database.php, the actual value stored in redis is serialized with php serializer and some igbinary bytes are added at beginning.

Example:
\Cache::put('test', ['a', 'b'])

Actual value saved in Redis:
\x00\x00\x00\x02\x11\x1ea:2:{i:0;s:1:\"a\";i:1;s:1:\"b\";}

As for me, it looks like value is serialized with php serializer and afterwards is serialized with igbinary

Expected value in Redis (from igbinary_serialize()):
\0\0\0\x02\x14\x02\x06\0\x11\x01a\x06\x01\x11\x01b

Value from Redis if default serializer is used:
a:2:{i:0;s:1:"a";i:1;s:1:"b";}

As you see, default serializer value is even shorter than igbinary one, which should never be the case

Checked with more complex stuff, for example putting the model into cache, issue is the same - some bytes from igbinary in beginning and default serializer value aftrewards.

Steps To Reproduce:

1. Enable Redis::SERIALIZER_IGBINARY serializer in config/database.php
2. Save some array or object to cache
3. Go to redis-cli and compare the value inside redis with igbinary_serialize() result

[it-can]

does \Cache::get('test') also show incorrect data?

If I test it:

Redis stores this

\Cache::put('test', ['a', 'b']);
dd(\Cache::get('test'));

array:2 [▼ // app/Http/Controllers/HomeController.php:27
  0 => "a"
  1 => "b"
]

For me it seems to work...

[andreyzp87]

does \Cache::get('test') also show incorrect data?

If I test it:

Redis stores this

\Cache::put('test', ['a', 'b']);
dd(\Cache::get('test'));

array:2 [▼ // app/Http/Controllers/HomeController.php:27
  0 => "a"
  1 => "b"
]

For me it seems to work...

Actually you confirmed the bug :)

The cache put-get works, no problem with it. Problem is in the actual data stored in Redis.
If you encode it with igbinary igbinary_encode(['a','b']) you will see that there is no such stuff as a:2:{i:0 - it's php serializer format, igbinary has much more compact representation for arrays.

What it means in real life for the project where I work - when igbinary serializer worked fine, all our redis cache took around 1.2-1.5 gigabytes of RAM. When this bug appeared - redis RAM usage stared hitting 5+ gigabytes.
And when I switched to regular php serializer - these 5 gigabytes stayed the same, so igbinary now does not give any benefits right now.

The problem appeared when we migrated from ubuntu 18.04 to 22.04 and php 8.0 to php 8.1 and updated all project dependencies.

[it-can]

ok but is this a laravel or phpredis bug then?

[andreyzp87]

Not sure if I can confirm that. Will try our previous setup with latest laravel and get back here.

[andreyzp87]

It's not phpredis bug, try this code:

$client = new \Redis();

$client->connect('127.0.0.1', 6379);

$client->select(0);
$client->setOption(Redis::OPT_PREFIX, 'r:');
$client->setOption(Redis::OPT_SERIALIZER, Redis::SERIALIZER_IGBINARY);

$client->set('test', ['a', 'b']);
var_dump($client->get('test'));

When I check it in redis-cli:

127.0.0.1:6379> get r:test
"\x00\x00\x00\x02\x14\x02\x06\x00\x11\x01a\x06\x01\x11\x01b"

So the actual value in redis is the expected igbinary encoded array. But it's not the case with laravel Cache
Need some more time to check our old setup

[it-can]

can you also post some version numbers?

[it-can]

Laravel seems to set the serializer config option correct on the connector though...

[andreyzp87]

Same as in my initial message:
Laravel Version: 8.83.25
PHP Version: 8.1.1
Database Driver & Version:
Redis 6.0.6 on Ubuntu 22.04
Redis 6.2.7 on Fedora 36
PhpRedis 5.3.7

Just checked our old setup, it has the same problem, not sure why memory consumption increased.

Please, check this one, there is a hardcoded php serializer there:
https://github.com/laravel/framework/blob/8.x/src/Illuminate/Cache/RedisStore.php
line 334

So how it works now in laravel - we always encode everything with php serializer, and afterwards redis encodes serialized string with igbinary.
As I understand it - there is no point on using the igbinary serializer in redis config, it just adds some overhead to redis. We already pass serialized string as value.

It would be a nice feature to have in config - change the serializer/unserializer function. So use igbinary_serialize instead of default serialize. Or simply remove serialize function if redis does the job properly.

For now for my project I'll just extend the RedisStore overriding the serialize/unserialize functions in this class.

Sorry for wasting your time, you can convert this ticket to a feature request if you think it's nesessary.

[webard]

I think serialize method should be used by framework only when 'serializer' option in config is set as Redis::SERIALIZER_NONE. In other serializers, even Redis::SERIALIZER_PHP, the extension is reponsible for serializing data.

[it-can]

I think serialize method should be used by framework only when 'serializer' option in config is set as Redis::SERIALIZER_NONE. In other serializers, even Redis::SERIALIZER_PHP, the extension is reponsible for serializing data.

this library seems to do this

https://github.com/akalongman/laravel-lodash#redis-using-igbinary

[webard]

Yeah and look what they do with serialize/unserialize: https://github.com/akalongman/laravel-lodash/blob/67b4d5206d3118cff9bce3d4f718ebd9b2ee6491/src/Lodash/Cache/RedisStore.php#L11-L19

[andreyzp87]

Just tested such setup:

<?php

namespace App\Extensions;

use Illuminate\Cache\RedisStore;

class RedisStoreFixed extends RedisStore
{
    public function serialize($value)
    {
        return $value;
    }

    public function unserialize($value)
    {
        return $value;
    }
}

And it's loaded like that from extended Illuminate\Cache\CacheManager:

    protected function createRedisDriver(array $config)
    {
        $redisConfig = config('database.redis', []);

        if (class_exists('Redis')
            && defined('Redis::SERIALIZER_NONE')
            && isset($redisConfig['options']['serializer'])
            && $redisConfig['options']['serializer'] !== \Redis::SERIALIZER_NONE
        ) {
            $redis = $this->app['redis'];

            $connection = $config['connection'] ?? 'default';

            $store = new RedisStoreFixed($redis, $this->getPrefix($config), $connection);

            return $this->repository(
                $store->setLockConnection($config['lock_connection'] ?? $connection)
            );
        }

        return parent::createRedisDriver($config);
    }

Everything works flawlessly with any serializer.

I'm not sure how to do it properly in a form of a pull-request, it would be nice if someone makes it.

[webard]

Did you check Redis::SERIALIZER_NONE?

[andreyzp87]

$redisConfig['options']['serializer'] !== \Redis::SERIALIZER_NONE

I don't load modified RedisStore if serializer is none:
$redisConfig['options']['serializer'] !== \Redis::SERIALIZER_NONE

And I did not check if it works with predis

[it-can]

Why is the serialize() function there in the first place? Is that needed for predis?