Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions system/Session/Handlers/MemcachedHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -221,11 +221,7 @@ public function write($id, $data): bool
public function close(): bool
{
if (isset($this->memcached)) {
if (isset($this->lockKey)) {
$this->memcached->delete($this->lockKey);
}

return true;
return ! isset($this->lockKey) || $this->releaseLock();
}

return false;
Expand Down Expand Up @@ -265,11 +261,15 @@ public function gc($max_lifetime): int
*/
protected function lockSession(string $sessionID): bool
{
if (isset($this->lockKey)) {
$lockKey = $this->keyPrefix . $sessionID . ':lock';

// 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) {
return $this->memcached->replace($this->lockKey, Time::now()->getTimestamp(), 300);
}

$lockKey = $this->keyPrefix . $sessionID . ':lock';
$attempt = 0;

do {
Expand Down
103 changes: 103 additions & 0 deletions tests/system/Session/Handlers/MemcachedHandlerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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<string>
*/
private array $sessionIds = [];

/**
* @param array<string, bool|int|string|null> $options Replace values for `Config\Session`.
*/
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -128,4 +157,78 @@ 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 = $this->generateSessionId('666666');

$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 = $this->generateSessionId('777777');
$newSessionId = $this->generateSessionId('888888');

$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());
}

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());
}
}
Loading