[11.x] Proper rate limiter fix with phpredis serialization/compression enabled

[TheLevti]

As a follow up from #54307:

As it turns out it is not safe to globally skip phpredis's serialization/compression for numeric values in EVAL script parameters. The result of my investigation (with the help of the phpredis team, see: phpredis/phpredis#2616) concludes the following: when those options are enabled, the framework must not interfere at all. The application code must disable serialization/compression on per key basis where we know that this key is used for increment/decrement or similar operations.

I have found several cases where skipping the serialization/compression for numeric values, causes the RateLimiter to still fail.

One example is when used with MSGPACK. Its a binary format and for example the rate limit counter 1 is stored as 1, but when this counter value is read, it is interpreted as 49.

Proof

For the following test I expect a fresh counter with a rate limit of 1

/**
 * @param  string  $driver
 */
#[DataProvider('redisDriverProvider')]
public function testRedisCacheRateLimiter($driver)
{
    $store = new RedisStore($this->redis[$driver]);
    $repository = new Repository($store);
    $rateLimiter = new RateLimiter($repository);

    $this->assertFalse($rateLimiter->tooManyAttempts('key', 1));
    $this->assertEquals(1, $rateLimiter->hit('key', 60));
    $this->assertTrue($rateLimiter->tooManyAttempts('key', 1));

    // Fails here, because the stored value is 49 when read back instead of 1.
    $this->assertFalse($rateLimiter->tooManyAttempts('key', 2));
}

Redis output looks fine here

1737982882.439262 [5 172.18.0.1:46038] "GET" "test_key"
1737982882.453418 [5 172.18.0.1:46038] "EVAL" "return redis.call('exists',KEYS[1])<1 and redis.call('setex',KEYS[1],ARGV[2],ARGV[1])" "1" "test_key:timer" "1737982942" "60"
1737982882.453455 [5 lua] "exists" "test_key:timer"
1737982882.453461 [5 lua] "setex" "test_key:timer" "60" "1737982942"
1737982882.453601 [5 172.18.0.1:46038] "EVAL" "return redis.call('exists',KEYS[1])<1 and redis.call('setex',KEYS[1],ARGV[2],ARGV[1])" "1" "test_key" "0" "60"
1737982882.453612 [5 lua] "exists" "test_key"
1737982882.453615 [5 lua] "setex" "test_key" "60" "0"
1737982882.453670 [5 172.18.0.1:46038] "INCRBY" "test_key" "1"
1737982882.453841 [5 172.18.0.1:46038] "GET" "test_key"
1737982882.453992 [5 172.18.0.1:46038] "GET" "test_key:timer"
1737982882.454303 [5 172.18.0.1:46038] "GET" "test_key"
1737982882.454383 [5 172.18.0.1:46038] "GET" "test_key:timer"
1737982882.457179 [5 172.18.0.1:46038] "FLUSHDB"

Phpunit output reveals the error when we read back the value and phpredis tries to deserialize

vendor/bin/phpunit tests/Cache/RedisCacheIntegrationTest.php --filter=testRedisCacheRateLimiter
PHPUnit 11.5.3 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.3.16
Configuration: /home/xxx/Documents/workspace/framework/phpunit.xml

F                                                                   1 / 1 (100%)

Time: 00:00.029, Memory: 14.00 MB

There was 1 failure:

1) Illuminate\Tests\Cache\RedisCacheIntegrationTest::testRedisCacheRateLimiter with data set #0 ('phpredis')
Failed asserting that true is false.

/home/xxx/Documents/workspace/framework/tests/Cache/RedisCacheIntegrationTest.php:54

--

1 test triggered 1 PHP warning:

1) /home/xxx/Documents/workspace/framework/src/Illuminate/Redis/Connections/Connection.php:116
[msgpack] (php_msgpack_unserialize) Extra bytes

Triggered by:

* Illuminate\Tests\Cache\RedisCacheIntegrationTest::testRedisCacheRateLimiter#0 (2 times)
  /home/xxx/Documents/workspace/framework/tests/Cache/RedisCacheIntegrationTest.php:45

FAILURES!
Tests: 1, Assertions: 4, Failures: 1, Warnings: 1.

Solution

1. Do not interfere or decide what needs to be serialized/deserialized or compressed/uncompressed on application side while those settings are enabled. Let phpredis deal with packing/unpacking exclusively.
2. For concrete features like RateLimiter, disable serialization/compression for the duration of the operation. Reading/writing those options is just a flag flip, so basically 0 extra overhead (as confirmed by phpredis maintainer).

Because the RateLimiter is built on top of the Cache component, I have added this logic in all 3 places where it sets and reads the counter value. As a result the test pass with all combinations of compression/serialization settings locally.

In addition a new helper method is provided in the PacksPhpRedisValues trait that users can use to skip phpredis serialization/compression on per key basis. This allows full control over this behavior. (fyi, we should mention in the docs that connections with those settings should ideally by only used for caching).