From daed2b1740d1f32905ed3026598c761d87f703bb Mon Sep 17 00:00:00 2001 From: MD Ali Kadar Date: Mon, 31 Aug 2026 09:52:53 +0600 Subject: [PATCH 1/3] fix: MemcachedHandler::decrement() wrong sign on missing key MemcachedHandler::decrement() passed $offset as Memcached::decrement()'s initial_value, which is used as-is (not decremented from) when a key doesn't exist yet. That made a fresh key end up at +$offset instead of the -$offset every other cache handler (File, Redis, Predis) produces. Memcached counters are unsigned, so they can't hold a negative initial value the way the other handlers effectively can. Use 0 instead (also Memcached::decrement()'s own default), so a fresh key at least stops going the wrong direction. Updates the existing MemcachedHandlerTest::testDecrement() expectation and adds a changelog entry. --- system/Cache/Handlers/MemcachedHandler.php | 8 +++++--- tests/system/Cache/Handlers/MemcachedHandlerTest.php | 4 +++- user_guide_src/source/changelogs/v4.7.5.rst | 1 + 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/system/Cache/Handlers/MemcachedHandler.php b/system/Cache/Handlers/MemcachedHandler.php index 6d0c8aec4d97..8253b9606000 100644 --- a/system/Cache/Handlers/MemcachedHandler.php +++ b/system/Cache/Handlers/MemcachedHandler.php @@ -180,9 +180,11 @@ public function decrement(string $key, int $offset = 1): false|int $key = static::validateKey($key, $this->prefix); - // FIXME: third parameter isn't other handler actions. - - return $this->memcached->decrement($key, $offset, $offset, 60); + // Memcached counters are unsigned, so a missing key can't be + // initialized to a negative value like the other handlers do. + // Fall back to Memcached::decrement()'s own default of 0 instead + // of $offset, so a new key no longer starts positive. + return $this->memcached->decrement($key, $offset, 0, 60); } public function clean(): bool diff --git a/tests/system/Cache/Handlers/MemcachedHandlerTest.php b/tests/system/Cache/Handlers/MemcachedHandlerTest.php index 78412616e5bf..56e3bff5376e 100644 --- a/tests/system/Cache/Handlers/MemcachedHandlerTest.php +++ b/tests/system/Cache/Handlers/MemcachedHandlerTest.php @@ -151,7 +151,9 @@ public function testDecrement(): void $this->assertSame(9, $memcachedHandler->decrement(self::$key1, 1)); $this->assertFalse($memcachedHandler->decrement(self::$key2, 1)); - $this->assertSame(1, $memcachedHandler->decrement(self::$key3, 1)); + // A key that doesn't exist yet starts at 0, not at the offset + // (Memcached counters are unsigned, so it can't start negative). + $this->assertSame(0, $memcachedHandler->decrement(self::$key3, 1)); } public function testClean(): void diff --git a/user_guide_src/source/changelogs/v4.7.5.rst b/user_guide_src/source/changelogs/v4.7.5.rst index 47d447dfbbc3..bd9d152c0a77 100644 --- a/user_guide_src/source/changelogs/v4.7.5.rst +++ b/user_guide_src/source/changelogs/v4.7.5.rst @@ -39,6 +39,7 @@ Bugs Fixed - **Helpers:** Fixed a bug where ``get_dir_file_info()`` returned incomplete entries for subdirectories and missing files instead of omitting them. - **Honeypot:** Fixed a bug where bot detection returned an HTTP 500 response instead of 403 (Forbidden). - **Logger:** Fixed a bug where interpolating a log message with array or non-stringable context values could raise PHP warnings or errors. +- **Cache:** Fixed a bug where ``MemcachedHandler::decrement()`` on a non-existent key returned a positive value instead of ``0``, the opposite sign of what ``FileHandler``, ``RedisHandler``, and ``PredisHandler`` return. See the repo's `CHANGELOG.md `_ From ea342163a30bb394de82b6b79d3ca43c23e3a257 Mon Sep 17 00:00:00 2001 From: MD Ali Kadar Date: Tue, 1 Sep 2026 12:51:13 +0600 Subject: [PATCH 2/3] fix: address review feedback on Memcached decrement() fix --- system/Cache/Handlers/MemcachedHandler.php | 8 ++++---- tests/system/Cache/Handlers/MemcachedHandlerTest.php | 3 ++- user_guide_src/source/changelogs/v4.7.5.rst | 2 +- user_guide_src/source/libraries/caching.rst | 5 +++++ 4 files changed, 12 insertions(+), 6 deletions(-) diff --git a/system/Cache/Handlers/MemcachedHandler.php b/system/Cache/Handlers/MemcachedHandler.php index 8253b9606000..3633dc8e40da 100644 --- a/system/Cache/Handlers/MemcachedHandler.php +++ b/system/Cache/Handlers/MemcachedHandler.php @@ -180,10 +180,10 @@ public function decrement(string $key, int $offset = 1): false|int $key = static::validateKey($key, $this->prefix); - // Memcached counters are unsigned, so a missing key can't be - // initialized to a negative value like the other handlers do. - // Fall back to Memcached::decrement()'s own default of 0 instead - // of $offset, so a new key no longer starts positive. + // Memcached counters are unsigned and saturate at 0, so a missing + // key can only ever be initialized to 0, never a negative value. + // Use Memcached::decrement()'s own default of 0 as the initial + // value instead of $offset, so a new key no longer starts positive. return $this->memcached->decrement($key, $offset, 0, 60); } diff --git a/tests/system/Cache/Handlers/MemcachedHandlerTest.php b/tests/system/Cache/Handlers/MemcachedHandlerTest.php index 56e3bff5376e..642fb116c549 100644 --- a/tests/system/Cache/Handlers/MemcachedHandlerTest.php +++ b/tests/system/Cache/Handlers/MemcachedHandlerTest.php @@ -153,7 +153,8 @@ public function testDecrement(): void $this->assertFalse($memcachedHandler->decrement(self::$key2, 1)); // A key that doesn't exist yet starts at 0, not at the offset // (Memcached counters are unsigned, so it can't start negative). - $this->assertSame(0, $memcachedHandler->decrement(self::$key3, 1)); + $this->assertSame(0, $memcachedHandler->decrement(self::$key3, 5)); + $this->assertSame(0, $memcachedHandler->get(self::$key3)); } public function testClean(): void diff --git a/user_guide_src/source/changelogs/v4.7.5.rst b/user_guide_src/source/changelogs/v4.7.5.rst index bd9d152c0a77..f1d58e64be99 100644 --- a/user_guide_src/source/changelogs/v4.7.5.rst +++ b/user_guide_src/source/changelogs/v4.7.5.rst @@ -39,7 +39,7 @@ Bugs Fixed - **Helpers:** Fixed a bug where ``get_dir_file_info()`` returned incomplete entries for subdirectories and missing files instead of omitting them. - **Honeypot:** Fixed a bug where bot detection returned an HTTP 500 response instead of 403 (Forbidden). - **Logger:** Fixed a bug where interpolating a log message with array or non-stringable context values could raise PHP warnings or errors. -- **Cache:** Fixed a bug where ``MemcachedHandler::decrement()`` on a non-existent key returned a positive value instead of ``0``, the opposite sign of what ``FileHandler``, ``RedisHandler``, and ``PredisHandler`` return. +- **Cache:** Fixed ``MemcachedHandler::decrement()`` initializing a non-existent counter to the positive offset. Missing counters are now initialized to ``0``, reflecting Memcached's unsigned, saturating counter semantics. See the repo's `CHANGELOG.md `_ diff --git a/user_guide_src/source/libraries/caching.rst b/user_guide_src/source/libraries/caching.rst index b208d63499df..76a696f570ee 100644 --- a/user_guide_src/source/libraries/caching.rst +++ b/user_guide_src/source/libraries/caching.rst @@ -212,6 +212,11 @@ Class Reference .. literalinclude:: caching/008.php + .. important:: Memcached counters are unsigned and saturate at ``0`` instead of + going negative. Decrementing a key that does not yet exist initializes it + to ``0`` (rather than ``-$offset``, as the File, Redis, and Predis handlers + do), and decrementing an existing counter below ``0`` clamps it at ``0``. + .. php:method:: clean() :returns: ``true`` on success, ``false`` on failure From 4f400a31cef807cf4e8e48a0da2fc994de7d4dd6 Mon Sep 17 00:00:00 2001 From: MD Ali Kadar Date: Tue, 1 Sep 2026 13:04:52 +0600 Subject: [PATCH 3/3] fix: correct expected type in MemcachedHandlerTest::testDecrement --- tests/system/Cache/Handlers/MemcachedHandlerTest.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/system/Cache/Handlers/MemcachedHandlerTest.php b/tests/system/Cache/Handlers/MemcachedHandlerTest.php index 642fb116c549..e81c8c9056fe 100644 --- a/tests/system/Cache/Handlers/MemcachedHandlerTest.php +++ b/tests/system/Cache/Handlers/MemcachedHandlerTest.php @@ -154,7 +154,9 @@ public function testDecrement(): void // A key that doesn't exist yet starts at 0, not at the offset // (Memcached counters are unsigned, so it can't start negative). $this->assertSame(0, $memcachedHandler->decrement(self::$key3, 5)); - $this->assertSame(0, $memcachedHandler->get(self::$key3)); + // Memcached stores counter values as decimal strings on the wire, so + // a plain get() on a counter key returns a string, not an int. + $this->assertSame('0', $memcachedHandler->get(self::$key3)); } public function testClean(): void