Skip to content

Commit daed2b1

Browse files
committed
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.
1 parent a826e69 commit daed2b1

3 files changed

Lines changed: 9 additions & 4 deletions

File tree

system/Cache/Handlers/MemcachedHandler.php

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -180,9 +180,11 @@ public function decrement(string $key, int $offset = 1): false|int
180180

181181
$key = static::validateKey($key, $this->prefix);
182182

183-
// FIXME: third parameter isn't other handler actions.
184-
185-
return $this->memcached->decrement($key, $offset, $offset, 60);
183+
// Memcached counters are unsigned, so a missing key can't be
184+
// initialized to a negative value like the other handlers do.
185+
// Fall back to Memcached::decrement()'s own default of 0 instead
186+
// of $offset, so a new key no longer starts positive.
187+
return $this->memcached->decrement($key, $offset, 0, 60);
186188
}
187189

188190
public function clean(): bool

tests/system/Cache/Handlers/MemcachedHandlerTest.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,9 @@ public function testDecrement(): void
151151

152152
$this->assertSame(9, $memcachedHandler->decrement(self::$key1, 1));
153153
$this->assertFalse($memcachedHandler->decrement(self::$key2, 1));
154-
$this->assertSame(1, $memcachedHandler->decrement(self::$key3, 1));
154+
// A key that doesn't exist yet starts at 0, not at the offset
155+
// (Memcached counters are unsigned, so it can't start negative).
156+
$this->assertSame(0, $memcachedHandler->decrement(self::$key3, 1));
155157
}
156158

157159
public function testClean(): void

user_guide_src/source/changelogs/v4.7.5.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ Bugs Fixed
3939
- **Helpers:** Fixed a bug where ``get_dir_file_info()`` returned incomplete entries for subdirectories and missing files instead of omitting them.
4040
- **Honeypot:** Fixed a bug where bot detection returned an HTTP 500 response instead of 403 (Forbidden).
4141
- **Logger:** Fixed a bug where interpolating a log message with array or non-stringable context values could raise PHP warnings or errors.
42+
- **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.
4243

4344
See the repo's
4445
`CHANGELOG.md <https://github.com/codeigniter4/CodeIgniter4/blob/develop/CHANGELOG.md>`_

0 commit comments

Comments
 (0)