diff --git a/src/Infrastructure/Adapter/In/Web/Init.php b/src/Infrastructure/Adapter/In/Web/Init.php index 8c99fc949..41ed53c47 100644 --- a/src/Infrastructure/Adapter/In/Web/Init.php +++ b/src/Infrastructure/Adapter/In/Web/Init.php @@ -190,6 +190,12 @@ public function initialize(string $controller): void $this->isIndex = $controller === IndexController::class; + // PHP refuses to change a session ini setting once a session is active, so this has to + // happen before the session is started rather than after — where it used to be, it always + // failed silently and the configured timeout never reached PHP's own collector, which was + // then free to delete a session file the application still considered valid. + ini_set('session.gc_maxlifetime', (string)$this->configData->getSessionTimeout()); + $this->context->initialize(); $isReload = $this->request->checkReload(); @@ -331,8 +337,12 @@ private function initUserSession(): void // Regenerate session's ID frequently to avoid fixation if ($sidStartTime === 0) { - // Try to set PHP's session lifetime - ini_set('session.gc_maxlifetime', $this->getSessionLifeTime()); + // Resolve this session's timeout and store it: getSessionLifeTime() writes it into + // the session, and for a session this new nothing else has called it yet — the + // check above short-circuits while there is no recorded activity. PHP's own + // collector was told about the configured lifetime in initialize(), before the + // session existed, which is the only moment PHP accepts it. + $this->getSessionLifeTime(); } elseif (!$inMaintenance && SessionLifecycleHandler::needsRegenerate($sidStartTime) && $this->context->isLoggedIn() diff --git a/tests/Unit/Infrastructure/Adapter/In/Web/InitSessionTest.php b/tests/Unit/Infrastructure/Adapter/In/Web/InitSessionTest.php index a976c2d12..c00b6e00b 100644 --- a/tests/Unit/Infrastructure/Adapter/In/Web/InitSessionTest.php +++ b/tests/Unit/Infrastructure/Adapter/In/Web/InitSessionTest.php @@ -212,11 +212,60 @@ public function testInitializeReachesSessionBookkeepingForAnOrdinaryController() $this->configData->setDatabaseVersion($currentVersion); $this->configData->setSessionTimeout(3600); - // A fresh, NOT-yet-initialized session: initialize() itself calls Context::initialize() - // as its first step (Session::initialize() refuses to bind twice), unlike every other test - // in this class, which bypasses that call by invoking the private methods directly against - // the already-initialized $this->session built in buildContext(). $freshSession = new Session(); + + $init = $this->buildInitForAFreshSession($freshSession); + + $init->initialize(IndexController::class); + + self::assertGreaterThan(0, $freshSession->getLastActivity()); + } + + /** + * PHP's own session collector is free to delete a session file older than + * session.gc_maxlifetime, whatever the application thinks. Telling it the configured timeout + * therefore has to happen before the session is started: PHP refuses the change once one is + * active, and the attempt that used to be made afterwards silently did nothing at all — so an + * installation configured for a long session could still have its files collected at whatever + * the platform default happened to be. + * + * @throws Exception + */ + public function testTheConfiguredSessionTimeoutReachesPhpsOwnCollector(): void + { + $currentVersion = Version::getVersionStringNormalized(); + $this->configData->setInstalled(true); + $this->configData->setMaintenance(false); + $this->configData->setAppVersion($currentVersion); + $this->configData->setDatabaseVersion($currentVersion); + $this->configData->setSessionTimeout(4321); + + // Built first: it closes the session the harness started, which is also what lets the + // baseline below be set at all. + $init = $this->buildInitForAFreshSession(new Session()); + + ini_set('session.gc_maxlifetime', '1440'); + + $init->initialize(IndexController::class); + + self::assertSame('4321', ini_get('session.gc_maxlifetime')); + } + + + /** + * Everything Init needs to reach session bookkeeping for an ordinary controller, against a + * session that has not been initialized yet — initialize() calls Context::initialize() itself + * as its first step, and Session::initialize() refuses to bind twice. + * + * @throws Exception + */ + private function buildInitForAFreshSession(Session $freshSession): Init + { + // The harness builds its context — and so starts a session — in setUp, whereas a real + // request reaches initialize() with none active. Close it, so these tests ask what + // production asks, including of anything that has to happen before a session exists. + session_write_close(); + $freshApplication = new Application( $this->configMock, $this->createStub(EventDispatcherInterface::class), @@ -264,9 +313,8 @@ public function testInitializeReachesSessionBookkeepingForAnOrdinaryController() $this->sessionKeyService ); - $init->initialize(IndexController::class); - self::assertGreaterThan(0, $freshSession->getLastActivity()); + return $init; } /**