From d16d70e51562ef4c7dacd103e13931d7f1111687 Mon Sep 17 00:00:00 2001 From: Franck DAKIA Date: Mon, 15 Jun 2026 01:31:26 +0000 Subject: [PATCH] fix(session): fix database adaptor --- src/Session/Adapters/DatabaseAdapter.php | 28 +++--- tests/Session/DatabaseAdapterTest.php | 106 +++++++++++++++++++++++ 2 files changed, 119 insertions(+), 15 deletions(-) create mode 100644 tests/Session/DatabaseAdapterTest.php diff --git a/src/Session/Adapters/DatabaseAdapter.php b/src/Session/Adapters/DatabaseAdapter.php index 8805c376..d2948b36 100644 --- a/src/Session/Adapters/DatabaseAdapter.php +++ b/src/Session/Adapters/DatabaseAdapter.php @@ -20,13 +20,6 @@ class DatabaseAdapter implements SessionHandlerInterface */ private string $table; - /** - * The current session session_id - * - * @var string - */ - private string $session_id; - /** * The current user ip * @@ -91,11 +84,11 @@ private function sessions(): QueryBuilder */ public function gc(int $max_lifetime): int|false { - $this->sessions() - ->where('time', '<', $this->createTimestamp()) + // The `time` column stores each session's expiry timestamp, so a + // session is collectable once that expiry is in the past. + return $this->sessions() + ->where('time', '<', date('Y-m-d H:i:s')) ->delete(); - - return 1; } /** @@ -117,10 +110,14 @@ public function open(string $path, string $name): bool * @return string * @throws QueryBuilderException */ - public function read(string $id): string + public function read(string $session_id): string { + // Only return live sessions: an expired row (expiry in the past) must + // be treated as absent, otherwise stale sessions stay usable until gc. $session = $this->sessions() - ->where('id', $id)->first(); + ->where('id', $session_id) + ->where('time', '>=', date('Y-m-d H:i:s')) + ->first(); if (is_null($session)) { return ''; @@ -147,11 +144,12 @@ public function write(string $id, string $data): bool return (bool)$insert; } - // Update the session information + // Update the session payload and slide the expiry forward so an active + // session does not expire a fixed window after its first request. $update = $this->sessions()->where('id', $id)->update( [ 'data' => $data, - 'id' => $id + 'time' => $this->createTimestamp() ] ); diff --git a/tests/Session/DatabaseAdapterTest.php b/tests/Session/DatabaseAdapterTest.php new file mode 100644 index 00000000..81234682 --- /dev/null +++ b/tests/Session/DatabaseAdapterTest.php @@ -0,0 +1,106 @@ +adapter = new DatabaseAdapter(['table' => 'sessions'], '127.0.0.1'); + } + + /** + * Seed a row directly with an explicit expiry offset so each test controls + * exactly whether a session is expired or still active. + */ + private function seed(string $id, string $data, int $expiresInSeconds): void + { + Database::table('sessions')->insert([ + 'id' => $id, + 'time' => date('Y-m-d H:i:s', time() + $expiresInSeconds), + 'data' => $data, + 'ip' => '127.0.0.1', + ]); + } + + private function exists(string $id): bool + { + return Database::table('sessions')->where('id', $id)->exists(); + } + + /** Bug #1 — gc() must drop expired rows but keep still-valid ones. */ + public function test_gc_removes_only_expired_sessions(): void + { + $this->seed('expired-session', 'old', -60); // expired a minute ago + $this->seed('active-session', 'fresh', 3600); // valid for another hour + + $this->adapter->gc(0); + + $this->assertFalse($this->exists('expired-session'), 'Expired session should be collected'); + $this->assertTrue($this->exists('active-session'), 'Active session must survive gc'); + } + + /** Bug #3 — read() must not return data for an expired session. */ + public function test_read_returns_empty_for_expired_session(): void + { + $this->seed('stale', 'secret-payload', -60); + + $this->assertSame('', $this->adapter->read('stale')); + } + + /** read() still returns the payload for a live session. */ + public function test_read_returns_data_for_active_session(): void + { + $this->seed('live', 'hello world', 3600); + + $this->assertSame('hello world', $this->adapter->read('live')); + } + + /** Bug #2 — write() on an existing session must refresh its expiry (sliding window). */ + public function test_write_refreshes_expiry_on_update(): void + { + $this->adapter->write('rolling', 'v1'); + + // Simulate time passing: push the stored expiry into the past. + Database::table('sessions') + ->where('id', 'rolling') + ->update(['time' => date('Y-m-d H:i:s', time() - 60)]); + + $this->adapter->write('rolling', 'v2'); + + $row = Database::table('sessions')->where('id', 'rolling')->first(); + + $this->assertSame('v2', $row->data, 'Payload should be updated'); + $this->assertGreaterThan( + date('Y-m-d H:i:s'), + $row->time, + 'Expiry must be pushed back into the future on each write' + ); + } +}