From b307950aed86f25923a1377a3ef4b5bfb4f8ec73 Mon Sep 17 00:00:00 2001 From: banghamid <62824084+banghamid@users.noreply.github.com> Date: Wed, 11 Nov 2026 16:49:52 +0700 Subject: [PATCH 1/5] fix(Session): release lock state on close and check session ID in MemcachedHandler --- system/Session/Handlers/MemcachedHandler.php | 12 +++-- .../Session/Handlers/MemcachedHandlerTest.php | 44 +++++++++++++++++++ 2 files changed, 52 insertions(+), 4 deletions(-) diff --git a/system/Session/Handlers/MemcachedHandler.php b/system/Session/Handlers/MemcachedHandler.php index c64ba65f6454..460bcc21947c 100644 --- a/system/Session/Handlers/MemcachedHandler.php +++ b/system/Session/Handlers/MemcachedHandler.php @@ -221,8 +221,8 @@ public function write($id, $data): bool public function close(): bool { if (isset($this->memcached)) { - if (isset($this->lockKey)) { - $this->memcached->delete($this->lockKey); + if (isset($this->lockKey) && ! $this->releaseLock()) { + return false; } return true; @@ -265,11 +265,15 @@ public function gc($max_lifetime): int */ protected function lockSession(string $sessionID): bool { - if (isset($this->lockKey)) { + $lockKey = $this->keyPrefix . $sessionID . ':lock'; + + // PHP 7 reuses the SessionHandler object on regeneration, + // so we need to check here if the lock key is for the + // correct session ID. + if ($this->lockKey === $lockKey) { return $this->memcached->replace($this->lockKey, Time::now()->getTimestamp(), 300); } - $lockKey = $this->keyPrefix . $sessionID . ':lock'; $attempt = 0; do { diff --git a/tests/system/Session/Handlers/MemcachedHandlerTest.php b/tests/system/Session/Handlers/MemcachedHandlerTest.php index 8a24a7377722..b7a90c2f768f 100644 --- a/tests/system/Session/Handlers/MemcachedHandlerTest.php +++ b/tests/system/Session/Handlers/MemcachedHandlerTest.php @@ -128,4 +128,48 @@ public function testGC(): void $handler = $this->getInstance(); $this->assertSame(1, $handler->gc(3600)); } + + public function testReopenAndReadWithSameHandlerInstance(): void + { + $handler = $this->getInstance(); + $handler->open($this->sessionSavePath, $this->sessionName); + + $sessionId = '666666b43phsnnf8if6bo33b635e4448'; + + $this->assertSame('', $handler->read($sessionId)); + + $data = '__ci_last_regenerate|i:1664607454;key|s:5:"value";'; + $this->assertTrue($handler->write($sessionId, $data)); + $this->assertTrue($handler->close()); + + // Reopen with the same handler instance (e.g. session_write_close() + session_start()) + $this->assertTrue($handler->open($this->sessionSavePath, $this->sessionName)); + $this->assertSame($data, $handler->read($sessionId)); + $this->assertTrue($handler->close()); + } + + public function testRegenerateSessionIdWithSameHandlerInstance(): void + { + $handler = $this->getInstance(); + $handler->open($this->sessionSavePath, $this->sessionName); + + $oldSessionId = '777777b43phsnnf8if6bo33b635e4449'; + $newSessionId = '888888b43phsnnf8if6bo33b635e4450'; + + $this->assertSame('', $handler->read($oldSessionId)); + + $data = '__ci_last_regenerate|i:1664607454;role|s:5:"admin";'; + $this->assertTrue($handler->write($oldSessionId, $data)); + $this->assertTrue($handler->close()); + + // PHP reuses the same SessionHandler object on regeneration + $this->assertTrue($handler->open($this->sessionSavePath, $this->sessionName)); + $this->assertSame('', $handler->read($newSessionId)); + $this->assertTrue($handler->write($newSessionId, $data)); + $this->assertTrue($handler->close()); + + $this->assertTrue($handler->open($this->sessionSavePath, $this->sessionName)); + $this->assertSame($data, $handler->read($newSessionId)); + $this->assertTrue($handler->close()); + } } From 8b150598a0f7ba8e8f9d95f76ea3b12631ab397c Mon Sep 17 00:00:00 2001 From: banghamid <62824084+banghamid@users.noreply.github.com> Date: Thu, 12 Nov 2026 01:52:57 +0700 Subject: [PATCH 2/5] test(Session): improve MemcachedHandler lock regeneration tests --- system/Session/Handlers/MemcachedHandler.php | 2 +- .../Session/Handlers/MemcachedHandlerTest.php | 65 ++++++++++++++++++- 2 files changed, 63 insertions(+), 4 deletions(-) diff --git a/system/Session/Handlers/MemcachedHandler.php b/system/Session/Handlers/MemcachedHandler.php index 460bcc21947c..5f71800db014 100644 --- a/system/Session/Handlers/MemcachedHandler.php +++ b/system/Session/Handlers/MemcachedHandler.php @@ -267,7 +267,7 @@ protected function lockSession(string $sessionID): bool { $lockKey = $this->keyPrefix . $sessionID . ':lock'; - // PHP 7 reuses the SessionHandler object on regeneration, + // PHP reuses the SessionHandler object on regeneration, // so we need to check here if the lock key is for the // correct session ID. if ($this->lockKey === $lockKey) { diff --git a/tests/system/Session/Handlers/MemcachedHandlerTest.php b/tests/system/Session/Handlers/MemcachedHandlerTest.php index b7a90c2f768f..d00489d61ce5 100644 --- a/tests/system/Session/Handlers/MemcachedHandlerTest.php +++ b/tests/system/Session/Handlers/MemcachedHandlerTest.php @@ -18,6 +18,7 @@ use CodeIgniter\Test\TestLogger; use Config\Logger as LoggerConfig; use Config\Session as SessionConfig; +use Memcached; use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\Attributes\RequiresPhpExtension; @@ -33,6 +34,11 @@ final class MemcachedHandlerTest extends CIUnitTestCase private string $sessionSavePath = '127.0.0.1:11211'; private string $userIpAddress = '127.0.0.1'; + /** + * @var list + */ + private array $sessionIds = []; + /** * @param array $options Replace values for `Config\Session`. */ @@ -62,11 +68,34 @@ protected function getInstance($options = []): MemcachedHandler protected function tearDown(): void { + if ($this->sessionIds !== []) { + $memcached = new Memcached(); + $memcached->setOption(Memcached::OPT_BINARY_PROTOCOL, true); + $memcached->addServer('127.0.0.1', 11211); + + $prefix = 'ci_session:' . $this->sessionName . ':'; + + foreach ($this->sessionIds as $sessionId) { + $memcached->delete($prefix . $sessionId); + $memcached->delete($prefix . $sessionId . ':lock'); + } + + $this->sessionIds = []; + } + parent::tearDown(); MemcachedHandler::resetPersistentConnections(); } + private function generateSessionId(string $prefix): string + { + $sessionId = $prefix . bin2hex(random_bytes(13)); + $this->sessionIds[] = $sessionId; + + return $sessionId; + } + public function testConstructorThrowsWithEmptySavePath(): void { $this->expectException(SessionException::class); @@ -134,7 +163,7 @@ public function testReopenAndReadWithSameHandlerInstance(): void $handler = $this->getInstance(); $handler->open($this->sessionSavePath, $this->sessionName); - $sessionId = '666666b43phsnnf8if6bo33b635e4448'; + $sessionId = $this->generateSessionId('666666'); $this->assertSame('', $handler->read($sessionId)); @@ -153,8 +182,8 @@ public function testRegenerateSessionIdWithSameHandlerInstance(): void $handler = $this->getInstance(); $handler->open($this->sessionSavePath, $this->sessionName); - $oldSessionId = '777777b43phsnnf8if6bo33b635e4449'; - $newSessionId = '888888b43phsnnf8if6bo33b635e4450'; + $oldSessionId = $this->generateSessionId('777777'); + $newSessionId = $this->generateSessionId('888888'); $this->assertSame('', $handler->read($oldSessionId)); @@ -172,4 +201,34 @@ public function testRegenerateSessionIdWithSameHandlerInstance(): void $this->assertSame($data, $handler->read($newSessionId)); $this->assertTrue($handler->close()); } + + public function testRegenerateSessionIdWriteWithoutClose(): void + { + $handler = $this->getInstance(); + $handler->open($this->sessionSavePath, $this->sessionName); + + $oldSessionId = $this->generateSessionId('999999'); + $newSessionId = $this->generateSessionId('aaaaaa'); + + $this->assertSame('', $handler->read($oldSessionId)); + + $oldData = '__ci_last_regenerate|i:1664607454;role|s:5:"guest";'; + $this->assertTrue($handler->write($oldSessionId, $oldData)); + + // During PHP session_regenerate_id(), write() is called with the new ID + // while the lock for the old session ID is still held. + $newData = '__ci_last_regenerate|i:1664607500;role|s:5:"admin";'; + $this->assertTrue($handler->write($newSessionId, $newData)); + $this->assertTrue($handler->close()); + + // Verify the old lock was released and the new session data is readable + $handler2 = $this->getInstance(); + $this->assertTrue($handler2->open($this->sessionSavePath, $this->sessionName)); + $this->assertSame($oldData, $handler2->read($oldSessionId)); + $this->assertTrue($handler2->close()); + + $this->assertTrue($handler2->open($this->sessionSavePath, $this->sessionName)); + $this->assertSame($newData, $handler2->read($newSessionId)); + $this->assertTrue($handler2->close()); + } } From ed78e22eadedc376202b7d0c99b9aa3bd0512cbd Mon Sep 17 00:00:00 2001 From: banghamid <62824084+banghamid@users.noreply.github.com> Date: Thu, 12 Nov 2026 05:26:07 +0700 Subject: [PATCH 3/5] style(Session): simplify close() return in MemcachedHandler --- system/Session/Handlers/MemcachedHandler.php | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/system/Session/Handlers/MemcachedHandler.php b/system/Session/Handlers/MemcachedHandler.php index 5f71800db014..7b43bc31bda4 100644 --- a/system/Session/Handlers/MemcachedHandler.php +++ b/system/Session/Handlers/MemcachedHandler.php @@ -221,11 +221,7 @@ public function write($id, $data): bool public function close(): bool { if (isset($this->memcached)) { - if (isset($this->lockKey) && ! $this->releaseLock()) { - return false; - } - - return true; + return ! isset($this->lockKey) || $this->releaseLock(); } return false; From 951b15a7ecf4f2ed0d27ad33221e4664e9a58112 Mon Sep 17 00:00:00 2001 From: banghamid <62824084+banghamid@users.noreply.github.com> Date: Thu, 12 Nov 2026 07:15:11 +0700 Subject: [PATCH 4/5] trigger ci From e9e202e71d0d7b275d90cbc24b633cdd81b1c708 Mon Sep 17 00:00:00 2001 From: banghamid <62824084+banghamid@users.noreply.github.com> Date: Thu, 12 Nov 2026 07:16:31 +0700 Subject: [PATCH 5/5] trigger ci