diff --git a/system/Cache/Handlers/PredisHandler.php b/system/Cache/Handlers/PredisHandler.php index 79e7ae98f916..2eae6dda20a6 100644 --- a/system/Cache/Handlers/PredisHandler.php +++ b/system/Cache/Handlers/PredisHandler.php @@ -20,12 +20,26 @@ use Predis\Client; use Predis\Collection\Iterator\Keyspace; use Predis\Response\Status; +use Throwable; /** * Predis cache handler */ class PredisHandler extends BaseHandler { + /** + * Lua script to conditionally execute HMSET and EXPIREAT in a single round trip. + * Prevents EXPIREAT from mutating TTL or expiring keys when HMSET fails (e.g. WRONGTYPE). + */ + private const SAVE_SCRIPT = <<<'LUA' + local res = redis.pcall('HMSET', KEYS[1], '__ci_type', ARGV[2], '__ci_value', ARGV[3]) + if type(res) == 'table' and res.err then + return 0 + end + redis.call('EXPIREAT', KEYS[1], ARGV[1]) + return 1 + LUA; + /** * Default config * @@ -118,15 +132,33 @@ public function save(string $key, mixed $value, int $ttl = 60): bool return false; } - if (! $this->redis->hmset($key, ['__ci_type' => $dataType, '__ci_value' => $value]) instanceof Status) { - return false; - } + $fields = ['__ci_type' => $dataType, '__ci_value' => $value]; if ($ttl !== 0) { - $this->redis->expireat($key, Time::now()->getTimestamp() + $ttl); + $expiresAt = Time::now()->getTimestamp() + $ttl; + + try { + return (bool) $this->redis->eval( + self::SAVE_SCRIPT, + 1, + $key, + (string) $expiresAt, + $dataType, + (string) $value, + ); + } catch (Throwable) { + // If EVAL is unsupported or blocked, fallback to sequential execution. + if (! $this->redis->hmset($key, $fields) instanceof Status) { + return false; + } + + $this->redis->expireat($key, $expiresAt); + + return true; + } } - return true; + return $this->redis->hmset($key, $fields) instanceof Status; } public function delete(string $key): bool diff --git a/system/Cache/Handlers/RedisHandler.php b/system/Cache/Handlers/RedisHandler.php index ccbb29bcd032..e008a35c59c4 100644 --- a/system/Cache/Handlers/RedisHandler.php +++ b/system/Cache/Handlers/RedisHandler.php @@ -18,12 +18,26 @@ use Config\Cache; use Redis; use RedisException; +use Throwable; /** * Redis cache handler */ class RedisHandler extends BaseHandler { + /** + * Lua script to conditionally execute HMSET and EXPIREAT in a single round trip. + * Prevents EXPIREAT from mutating TTL or expiring keys when HMSET fails (e.g. WRONGTYPE). + */ + private const SAVE_SCRIPT = <<<'LUA' + local res = redis.pcall('HMSET', KEYS[1], '__ci_type', ARGV[2], '__ci_value', ARGV[3]) + if type(res) == 'table' and res.err then + return 0 + end + redis.call('EXPIREAT', KEYS[1], ARGV[1]) + return 1 + LUA; + /** * Default config * @@ -134,15 +148,28 @@ public function save(string $key, mixed $value, int $ttl = 60): bool return false; } - if (! $this->redis->hMset($key, ['__ci_type' => $dataType, '__ci_value' => $value])) { - return false; - } + $fields = ['__ci_type' => $dataType, '__ci_value' => $value]; if ($ttl !== 0) { - $this->redis->expireAt($key, Time::now()->getTimestamp() + $ttl); + $expiresAt = Time::now()->getTimestamp() + $ttl; + + try { + return (bool) $this->redis->eval( + self::SAVE_SCRIPT, + [$key, $expiresAt, $dataType, (string) $value], + 1, + ); + } catch (Throwable) { + // If EVAL is unsupported or blocked, fallback to sequential execution. + if (! $this->redis->hMset($key, $fields)) { + return false; + } + + return $this->redis->expireAt($key, $expiresAt); + } } - return true; + return (bool) $this->redis->hMset($key, $fields); } public function delete(string $key): bool diff --git a/tests/system/Cache/Handlers/PredisHandlerTest.php b/tests/system/Cache/Handlers/PredisHandlerTest.php index 91949f53238f..dbe70d112bd7 100644 --- a/tests/system/Cache/Handlers/PredisHandlerTest.php +++ b/tests/system/Cache/Handlers/PredisHandlerTest.php @@ -18,6 +18,7 @@ use CodeIgniter\I18n\Time; use Config\Cache; use PHPUnit\Framework\Attributes\Group; +use Predis\Client; /** * @internal @@ -196,4 +197,25 @@ public function testReconnect(): void $this->assertSame('value', $this->handler->get(self::$key1)); } + + public function testSaveFailsOnNonHashKeyPreservingValueAndTtl(): void + { + /** @var Client $redis */ + $redis = $this->getPrivateProperty($this->handler, 'redis'); + + $prefixedKey = $this->config->prefix . 'non_hash_key'; + $redis->set($prefixedKey, 'raw_string_value'); + $redis->expire($prefixedKey, 300); + + $this->assertFalse($this->handler->save('non_hash_key', 'new_value', 60)); + + $this->assertSame('raw_string_value', $redis->get($prefixedKey)); + $this->assertGreaterThan(250, $redis->ttl($prefixedKey)); + + // Handler remains usable for subsequent operations + $this->assertTrue($this->handler->save(self::$key1, 'usable_value')); + $this->assertSame('usable_value', $this->handler->get(self::$key1)); + + $redis->del([$prefixedKey]); + } } diff --git a/tests/system/Cache/Handlers/RedisHandlerTest.php b/tests/system/Cache/Handlers/RedisHandlerTest.php index 1f85ecc28349..4dbd06452169 100644 --- a/tests/system/Cache/Handlers/RedisHandlerTest.php +++ b/tests/system/Cache/Handlers/RedisHandlerTest.php @@ -19,6 +19,7 @@ use Config\Cache; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Group; +use Redis; /** * @internal @@ -238,4 +239,25 @@ public function testReconnect(): void $this->assertSame('value', $this->handler->get(self::$key1)); } + + public function testSaveFailsOnNonHashKeyPreservingValueAndTtl(): void + { + /** @var Redis $redis */ + $redis = $this->getPrivateProperty($this->handler, 'redis'); + + $prefixedKey = $this->config->prefix . 'non_hash_key'; + $redis->set($prefixedKey, 'raw_string_value'); + $redis->expire($prefixedKey, 300); + + $this->assertFalse($this->handler->save('non_hash_key', 'new_value', 60)); + + $this->assertSame('raw_string_value', $redis->get($prefixedKey)); + $this->assertGreaterThan(250, $redis->ttl($prefixedKey)); + + // Handler remains usable for subsequent operations + $this->assertTrue($this->handler->save(self::$key1, 'usable_value')); + $this->assertSame('usable_value', $this->handler->get(self::$key1)); + + $redis->del($prefixedKey); + } }