diff --git a/system/Cache/Handlers/MemcachedHandler.php b/system/Cache/Handlers/MemcachedHandler.php index 6d0c8aec4d97..3633dc8e40da 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 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); } public function clean(): bool diff --git a/tests/system/Cache/Handlers/MemcachedHandlerTest.php b/tests/system/Cache/Handlers/MemcachedHandlerTest.php index 78412616e5bf..e81c8c9056fe 100644 --- a/tests/system/Cache/Handlers/MemcachedHandlerTest.php +++ b/tests/system/Cache/Handlers/MemcachedHandlerTest.php @@ -151,7 +151,12 @@ 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, 5)); + // 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 diff --git a/user_guide_src/source/changelogs/v4.7.5.rst b/user_guide_src/source/changelogs/v4.7.5.rst index 47d447dfbbc3..f1d58e64be99 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 ``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