diff --git a/src/Storage/BuiltinBackend.php b/src/Storage/BuiltinBackend.php index 7fb29a0..adeade1 100644 --- a/src/Storage/BuiltinBackend.php +++ b/src/Storage/BuiltinBackend.php @@ -14,9 +14,14 @@ namespace Horde\SessionHandler\Storage; use DateTimeImmutable; +use DirectoryIterator; +use Generator; +use Horde\SessionHandler\Exception\SessionException; +use Horde\SessionHandler\IterableSessionBackend; use Horde\SessionHandler\SerializedSessionPayload; use Horde\SessionHandler\SessionId; use Horde\SessionHandler\SessionStorageBackend; +use InvalidArgumentException; use RuntimeException; /** @@ -67,18 +72,19 @@ * {@see RuntimeException} with the exact path so the misconfiguration is * obvious. * - * # Garbage collection + * # Iteration and garbage collection * - * Not implemented. This backend does not declare - * {@see \Horde\SessionHandler\IterableSessionBackend} or - * {@see \Horde\SessionHandler\SessionMetadataBackend}, so - * {@see \Horde\SessionHandler\SessionHandler::gc()} returns false and PHP's - * probabilistic GC is a no-op. Operators relying on `Builtin` must run + * Implements {@see \Horde\SessionHandler\IterableSessionBackend} by + * walking the configured save-path tree and yielding a {@see SessionId} + * per `sess_*` file found. Traversal is lazy (a Generator); admin pages + * can process large session pools without materialising the full list. + * Does not declare {@see \Horde\SessionHandler\SessionMetadataBackend}, + * so {@see \Horde\SessionHandler\SessionHandler::gc()} still relies on * external cleanup (cron, `systemd-tmpfiles`, the OS `/tmp` cleaner, or - * similar). Adding GC that walks the hashed tree is a future capability - * tracked separately. + * similar). GC that walks the tree during iteration is a future + * capability tracked separately. */ -final class BuiltinBackend implements SessionStorageBackend +final class BuiltinBackend implements SessionStorageBackend, IterableSessionBackend { private const FILE_PREFIX = 'sess_'; private const FILE_MODE = 0o600; @@ -106,9 +112,35 @@ public function load(SessionId $id): ?SerializedSessionPayload return null; } + if (!is_readable($file)) { + throw new SessionException(sprintf( + 'Session file "%s" exists but is not readable by the PHP ' + . 'user (check filesystem permissions and umask on the ' + . 'save_path).', + $file, + )); + } + $contents = @file_get_contents($file); - if ($contents === false || $contents === '') { + if ($contents === false) { + /* file_get_contents on a file that passed is_file() and + * is_readable() moments ago most often means a concurrent + * unlink (session GC) or a filesystem-level I/O error. + * Surface the path so the caller can act; do not silently + * skip. */ + $err = error_get_last(); + throw new SessionException(sprintf( + 'Failed to read session file "%s"%s', + $file, + isset($err['message']) ? ': ' . $err['message'] : '.', + )); + } + + if ($contents === '') { + /* Freshly-created but never-written session file. Treated + * as "no data" rather than an error; the session will be + * populated on first write. */ return null; } @@ -173,6 +205,123 @@ public function delete(SessionId $id): void @unlink($this->filePath($id)); } + /** + * Enumerate all session ids currently present on disk. + * + * Walks the configured save-path tree; for depth 0 that is a flat + * scan of {@see PhpFilesSavePathSpec::basePath}, for depth N a + * recursive walk of the N-level single-hex-char subdirectory tree. + * + * Errors are surfaced with the exact path involved so that an + * admin who sees the failure can act on it directly: + * + * - Base path missing or not a directory: throw immediately, + * before yielding anything. + * - Base or subdirectory unreadable (permissions): throw naming + * that specific directory. + * - Subdirectory expected under the hashed layout but absent on + * disk: skipped silently. PHP's mod_files also tolerates this + * because it lazily creates directories on write. + * - Filename that does not parse as a SessionId (e.g., a stray + * `sess_` prefix on something unrelated): skipped silently. + * Matches FileBackend's tolerance. + * + * @return Generator + * @throws SessionException on unreadable directories. + */ + public function listSessions(): Generator + { + $base = $this->spec->basePath; + + if (!is_dir($base)) { + throw new SessionException(sprintf( + 'Session save_path "%s" is not a directory. Cannot list ' + . 'sessions.', + $base, + )); + } + if (!is_readable($base)) { + throw new SessionException(sprintf( + 'Session save_path "%s" is not readable by the PHP user ' + . '(check filesystem permissions). Cannot list sessions.', + $base, + )); + } + + yield from $this->listSessionsIn($base, $this->spec->depth); + } + + /** + * Recursive walk helper for {@see listSessions}. At depth 0 yields + * SessionId per `sess_*` file in $dir; otherwise recurses into each + * of the 16 single-hex-char subdirectories that PHP's mod_files + * would use for this level. + * + * @return Generator + */ + private function listSessionsIn(string $dir, int $remainingDepth): Generator + { + if ($remainingDepth > 0) { + for ($i = 0; $i < 16; $i++) { + $sub = $dir . '/' . dechex($i); + if (!file_exists($sub)) { + /* PHP mod_files does not pre-create these; a missing + * subdirectory just means no sessions have hashed + * into that bucket yet. */ + continue; + } + if (!is_dir($sub)) { + throw new SessionException(sprintf( + 'Session save_path subdirectory "%s" exists but ' + . 'is not a directory; the save_path tree is ' + . 'inconsistent.', + $sub, + )); + } + if (!is_readable($sub)) { + throw new SessionException(sprintf( + 'Session save_path subdirectory "%s" is not ' + . 'readable by the PHP user (check filesystem ' + . 'permissions).', + $sub, + )); + } + yield from $this->listSessionsIn($sub, $remainingDepth - 1); + } + return; + } + + try { + $it = new DirectoryIterator($dir); + } catch (RuntimeException $e) { + throw new SessionException(sprintf( + 'Failed to open session save_path directory "%s": %s', + $dir, + $e->getMessage(), + ), 0, $e); + } + + $prefixLength = strlen(self::FILE_PREFIX); + + foreach ($it as $entry) { + if ($entry->isDot() || !$entry->isFile()) { + continue; + } + $name = $entry->getFilename(); + if (!str_starts_with($name, self::FILE_PREFIX)) { + continue; + } + $idString = substr($name, $prefixLength); + try { + yield new SessionId($idString); + } catch (InvalidArgumentException) { + /* Stray file matching sess_* whose suffix is not a + * valid session id. Skip; matches FileBackend. */ + continue; + } + } + } + private function filePath(SessionId $id): string { return $this->spec->directoryFor($id->id) diff --git a/test/unit/SessionHandlerTest.php b/test/unit/SessionHandlerTest.php index d6e9046..5d99901 100644 --- a/test/unit/SessionHandlerTest.php +++ b/test/unit/SessionHandlerTest.php @@ -16,7 +16,7 @@ use Horde\SessionHandler\PhpSessionSerializer; use Horde\SessionHandler\SessionHandler; use Horde\SessionHandler\SessionId; -use Horde\SessionHandler\Storage\BuiltinBackend; +use Horde\SessionHandler\Storage\ExternalBackend; use Horde\SessionHandler\Storage\FileBackend; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\Test; @@ -178,9 +178,18 @@ public function testExpire(): void #[Test] public function testCapabilityExceptionForListSessions(): void { - $builtinBackend = new BuiltinBackend($this->tempDir); + /* ExternalBackend wraps arbitrary PHP SessionHandlerInterface + * callbacks and cannot enumerate; a good fit for exercising + * the CapabilityException path. BuiltinBackend used to be + * non-iterable and was the fixture here, but now implements + * IterableSessionBackend by walking the save-path tree. */ + $nonIterable = new ExternalBackend( + \Closure::fromCallable(function (): ?string { return null; }), + \Closure::fromCallable(function (): void {}), + \Closure::fromCallable(function (): void {}), + ); $handler = new SessionHandler( - backend: $builtinBackend, + backend: $nonIterable, serializer: new PhpSessionSerializer(), sessionFactory: new DefaultSessionFactory(), ); diff --git a/test/unit/Storage/BuiltinBackendTest.php b/test/unit/Storage/BuiltinBackendTest.php index b5bf5b3..ee07315 100644 --- a/test/unit/Storage/BuiltinBackendTest.php +++ b/test/unit/Storage/BuiltinBackendTest.php @@ -12,6 +12,8 @@ namespace Horde\SessionHandler\Test\unit\Storage; use DateTimeImmutable; +use Horde\SessionHandler\Exception\SessionException; +use Horde\SessionHandler\IterableSessionBackend; use Horde\SessionHandler\SerializedSessionPayload; use Horde\SessionHandler\SessionId; use Horde\SessionHandler\Storage\BuiltinBackend; @@ -226,4 +228,179 @@ public function testDepthModeAndPathFormParses(): void self::assertFileExists($this->tempDir . '/a/sess_abc'); } + + // --------------------------------------------------------------- + // Iteration + // --------------------------------------------------------------- + + #[Test] + public function testImplementsIterableSessionBackend(): void + { + $backend = new BuiltinBackend($this->tempDir); + self::assertInstanceOf(IterableSessionBackend::class, $backend); + } + + #[Test] + public function testListSessionsFlatYieldsIds(): void + { + $backend = new BuiltinBackend($this->tempDir); + $backend->save(new SessionId('aaa'), $this->payload('1'), $this->expiresAt); + $backend->save(new SessionId('bbb'), $this->payload('2'), $this->expiresAt); + + $ids = []; + foreach ($backend->listSessions() as $id) { + $ids[] = $id->id; + } + sort($ids); + + self::assertSame(['aaa', 'bbb'], $ids); + } + + #[Test] + public function testListSessionsSkipsNonSessFiles(): void + { + $backend = new BuiltinBackend($this->tempDir); + $backend->save(new SessionId('good'), $this->payload('x'), $this->expiresAt); + // Foreign files that must not surface as session ids. + file_put_contents($this->tempDir . '/README', 'not a session'); + file_put_contents($this->tempDir . '/.hidden', 'not a session'); + + $ids = []; + foreach ($backend->listSessions() as $id) { + $ids[] = $id->id; + } + + self::assertSame(['good'], $ids); + } + + #[Test] + public function testListSessionsRecursesIntoHashedTree(): void + { + // Depth 2, ids "0ab..." and "1cd..." — mkdir the two subdirs + // they hash into, then save. + $backend = new BuiltinBackend('2;' . $this->tempDir); + mkdir($this->tempDir . '/0/a', 0o755, true); + mkdir($this->tempDir . '/1/c', 0o755, true); + $backend->save(new SessionId('0abc'), $this->payload('x'), $this->expiresAt); + $backend->save(new SessionId('1cde'), $this->payload('y'), $this->expiresAt); + + $ids = []; + foreach ($backend->listSessions() as $id) { + $ids[] = $id->id; + } + sort($ids); + + self::assertSame(['0abc', '1cde'], $ids); + } + + #[Test] + public function testListSessionsSkipsMissingHashSubdirectories(): void + { + // Depth 2 but no subdirectories yet — mod_files-compatible: + // never-created buckets mean no sessions, not an error. + $backend = new BuiltinBackend('2;' . $this->tempDir); + + $ids = iterator_to_array($backend->listSessions()); + + self::assertSame([], $ids); + } + + #[Test] + public function testListSessionsThrowsWhenBaseIsNotADirectory(): void + { + $file = $this->tempDir . '/not_a_dir'; + file_put_contents($file, 'x'); + + $backend = new BuiltinBackend($file); + + $this->expectException(SessionException::class); + $this->expectExceptionMessageMatches('/is not a directory/'); + $this->expectExceptionMessageMatches('/' . preg_quote($file, '/') . '/'); + + iterator_to_array($backend->listSessions()); + } + + #[Test] + public function testListSessionsThrowsWhenBaseIsUnreadable(): void + { + if (function_exists('posix_geteuid') && posix_geteuid() === 0) { + self::markTestSkipped('permission tests do not apply to root'); + } + + chmod($this->tempDir, 0o000); + // Verify chmod actually stripped the read bit (some filesystems + // ignore Unix mode bits). + if (is_readable($this->tempDir)) { + chmod($this->tempDir, 0o755); + self::markTestSkipped('filesystem does not honour chmod(0)'); + } + + try { + $backend = new BuiltinBackend($this->tempDir); + $this->expectException(SessionException::class); + $this->expectExceptionMessageMatches('/not readable/'); + $this->expectExceptionMessageMatches('/' . preg_quote($this->tempDir, '/') . '/'); + iterator_to_array($backend->listSessions()); + } finally { + /* Restore mode so tearDown()'s recursive remove works. */ + chmod($this->tempDir, 0o755); + } + } + + #[Test] + public function testListSessionsThrowsWhenHashSubdirectoryIsUnreadable(): void + { + if (function_exists('posix_geteuid') && posix_geteuid() === 0) { + self::markTestSkipped('permission tests do not apply to root'); + } + + $backend = new BuiltinBackend('1;' . $this->tempDir); + mkdir($this->tempDir . '/a', 0o755, true); + $backend->save(new SessionId('abc'), $this->payload('x'), $this->expiresAt); + chmod($this->tempDir . '/a', 0o000); + if (is_readable($this->tempDir . '/a')) { + chmod($this->tempDir . '/a', 0o755); + self::markTestSkipped('filesystem does not honour chmod(0)'); + } + + try { + $this->expectException(SessionException::class); + $this->expectExceptionMessageMatches('/subdirectory/'); + $this->expectExceptionMessageMatches('/not readable/'); + iterator_to_array($backend->listSessions()); + } finally { + chmod($this->tempDir . '/a', 0o755); + } + } + + // --------------------------------------------------------------- + // load() error surfacing + // --------------------------------------------------------------- + + #[Test] + public function testLoadThrowsWhenSessionFileIsUnreadable(): void + { + if (function_exists('posix_geteuid') && posix_geteuid() === 0) { + self::markTestSkipped('permission tests do not apply to root'); + } + + $backend = new BuiltinBackend($this->tempDir); + $id = new SessionId('locked'); + $backend->save($id, $this->payload('x'), $this->expiresAt); + $file = $this->tempDir . '/sess_locked'; + chmod($file, 0o000); + if (is_readable($file)) { + chmod($file, 0o600); + self::markTestSkipped('filesystem does not honour chmod(0)'); + } + + try { + $this->expectException(SessionException::class); + $this->expectExceptionMessageMatches('/not readable/'); + $this->expectExceptionMessageMatches('/' . preg_quote($file, '/') . '/'); + $backend->load($id); + } finally { + chmod($file, 0o600); + } + } }