Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Properly resolve RateLimiter not working while redis serialization or…
… compression is enabled
  • Loading branch information
TheLevti committed Jan 27, 2025
commit 62da8f7243a8ac1bd79a076c516aac4532d9c3b8
28 changes: 25 additions & 3 deletions src/Illuminate/Cache/RateLimiter.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

use Closure;
use Illuminate\Contracts\Cache\Repository as Cache;
use Illuminate\Redis\Connections\PhpRedisConnection;
use Illuminate\Support\Collection;
use Illuminate\Support\InteractsWithTime;

Expand Down Expand Up @@ -165,12 +166,12 @@ public function increment($key, $decaySeconds = 60, $amount = 1)
$key.':timer', $this->availableAt($decaySeconds), $decaySeconds
);

$added = $this->cache->add($key, 0, $decaySeconds);
$added = $this->runClean(fn (): mixed => $this->cache->add($key, 0, $decaySeconds));

$hits = (int) $this->cache->increment($key, $amount);

if (! $added && $hits == 1) {
$this->cache->put($key, 1, $decaySeconds);
$this->runClean(fn (): mixed => $this->cache->put($key, 1, $decaySeconds));
}

return $hits;
Expand Down Expand Up @@ -199,7 +200,7 @@ public function attempts($key)
{
$key = $this->cleanRateLimiterKey($key);

return $this->cache->get($key, 0);
return $this->runClean(fn (): mixed => $this->cache->get($key, 0));
}

/**
Expand Down Expand Up @@ -282,6 +283,27 @@ public function cleanRateLimiterKey($key)
return preg_replace('/&([a-z])[a-z]+;/i', '$1', htmlentities($key));
}

/**
* Allow the rate limiter to run a callback against a clean repository.
*
* For example when using RedisStore, the serializer and compressor can be
* disabled during the callback execution.
*/
protected function runClean(callable $callback): mixed
{
$store = $this->cache->getStore();
if (! $store instanceof RedisStore) {
return $callback();
}

$connection = $store->connection();
if (! $connection instanceof PhpRedisConnection) {
return $callback();
}

return $connection->runClean($callback);
}

/**
* Resolve the rate limiter name.
*
Expand Down
4 changes: 0 additions & 4 deletions src/Illuminate/Cache/RedisStore.php
Original file line number Diff line number Diff line change
Expand Up @@ -432,10 +432,6 @@ public function setPrefix($prefix)
protected function pack($value, $connection)
{
if ($connection instanceof PhpRedisConnection) {
if ($this->shouldBeStoredWithoutSerialization($value)) {
return $value;
}

if ($connection->serialized()) {
return $connection->pack([$value])[0];
}
Expand Down
35 changes: 35 additions & 0 deletions src/Illuminate/Redis/Connections/PacksPhpRedisValues.php
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,41 @@ public function lz4Compressed(): bool
$this->client->getOption(Redis::OPT_COMPRESSION) === Redis::COMPRESSION_LZ4;
}

/**
* Run a callback without serialization or compression enabled. Useful when
* performing operations like increment/decrement that should not be
* serialized or compressed on client side.
*/
public function runClean(callable $callback): mixed
{
/** @var \Redis|\RedisCluster $client */
$client = $this->client;

$oldSerializer = null;
if ($this->serialized()) {
$oldSerializer = $client->getOption($client::OPT_SERIALIZER);
$client->setOption($client::OPT_SERIALIZER, $client::SERIALIZER_NONE);
}

$oldCompressor = null;
if ($this->compressed()) {
$oldCompressor = $client->getOption($client::OPT_COMPRESSION);
$client->setOption($client::OPT_COMPRESSION, $client::COMPRESSION_NONE);
}

try {
return $callback();
} finally {
if ($oldSerializer !== null) {
$client->setOption($client::OPT_SERIALIZER, $oldSerializer);
}

if ($oldCompressor !== null) {
$client->setOption($client::OPT_COMPRESSION, $oldCompressor);
}
}
}

/**
* Determine if the current PhpRedis extension version supports packing.
*
Expand Down
17 changes: 17 additions & 0 deletions tests/Cache/RedisCacheIntegrationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace Illuminate\Tests\Cache;

use Illuminate\Cache\RateLimiter;
use Illuminate\Cache\RedisStore;
use Illuminate\Cache\Repository;
use Illuminate\Foundation\Testing\Concerns\InteractsWithRedis;
Expand Down Expand Up @@ -37,6 +38,22 @@ public function testRedisCacheAddTwice($driver)
$this->assertGreaterThan(3500, $this->redis[$driver]->connection()->ttl('k'));
}

/**
* @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));
$this->assertFalse($rateLimiter->tooManyAttempts('key', 2));
}

/**
* Breaking change.
*
Expand Down
1 change: 1 addition & 0 deletions tests/Integration/Queue/RateLimitedTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ public function testRateLimitedJobsAreNotExecutedOnLimitReached2()
$cache->shouldReceive('add')->andReturn(true, true);
$cache->shouldReceive('increment')->andReturn(1);
$cache->shouldReceive('has')->andReturn(true);
$cache->shouldReceive('getStore')->andReturn(new ArrayStore);

$rateLimiter = new RateLimiter($cache);
$this->app->instance(RateLimiter::class, $rateLimiter);
Expand Down
Loading