Skip to content

Commit 28f77b4

Browse files
author
Ibrahim BinAlshikh
committed
fix(session): reuse cookie ID when session storage is empty
When a browser sends a session cookie but storage has been cleared, the framework now reuses the existing cookie ID for the new session instead of generating a new one. This prevents the infinite new-session loop where the browser perpetually sends the old cookie. - Session::start() only regenerates ID when status is KILLED - SessionManager::start() passes cookie ID to new session Closes #389 Closes #388
1 parent f61f378 commit 28f77b4

3 files changed

Lines changed: 43 additions & 1 deletion

File tree

WebFiori/Framework/Session/Session.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -668,9 +668,11 @@ public function start() {
668668
if (!$this->isRunning()) {
669669
$sessionStr = SessionsManager::getStorage()->read($this->getId());
670670

671-
if ($this->getStatus() == SessionStatus::KILLED || $sessionStr === null || !$this->deserialize($sessionStr)) {
671+
if ($this->getStatus() == SessionStatus::KILLED) {
672672
$this->reGenerateID();
673673
$this->initNewSessionVars();
674+
} else if ($sessionStr === null || !$this->deserialize($sessionStr)) {
675+
$this->initNewSessionVars();
674676
} else {
675677
$this->checkIfExpired();
676678
}

WebFiori/Framework/Session/SessionManager.php

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -385,6 +385,13 @@ public function start(string $sessionName, array $options = []): void {
385385

386386
if (!$this->hasSession($sessionName)) {
387387
$options[SessionOption::NAME] = $sessionName;
388+
389+
$cookieId = $this->getSessionIDFromRequest($sessionName);
390+
391+
if ($cookieId !== false && !isset($options[SessionOption::SESSION_ID])) {
392+
$options[SessionOption::SESSION_ID] = $cookieId;
393+
}
394+
388395
$s = new Session($options);
389396
$s->start();
390397
$this->sessionsArr[] = $s;

tests/WebFiori/Framework/Tests/Session/SessionsManagerTest.php

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -567,4 +567,37 @@ public function testFacadePauseAll() {
567567
public function testGetSessionIDFromCookieNotSet() {
568568
$this->assertFalse(SessionsManager::getSessionIDFromCookie('nonexistent'));
569569
}
570+
571+
/**
572+
* @test
573+
* Verifies bug #389: When a session cookie exists but storage is empty,
574+
* the framework should reuse the cookie ID instead of generating a new one.
575+
*/
576+
public function testReuseCookieIdWhenStorageEmpty() {
577+
SessionsManager::reset();
578+
App::getRequest()->setRequestMethod('GET');
579+
580+
// Simulate: browser has a session cookie with ID 'old-session-id-abc'
581+
$sessionName = 'wf-test-session';
582+
$_GET[$sessionName] = 'old-session-id-abc';
583+
584+
// Storage is empty (no session data for this ID — simulates cleared storage)
585+
// Start the session
586+
SessionsManager::start($sessionName);
587+
588+
$active = SessionsManager::getActiveSession();
589+
$this->assertNotNull($active);
590+
591+
// BUG: The session should reuse 'old-session-id-abc' from the cookie,
592+
// not generate a brand new ID
593+
$this->assertEquals(
594+
'old-session-id-abc',
595+
$active->getId(),
596+
'Session ID should match the cookie value when storage is empty (bug #389)'
597+
);
598+
599+
// Cleanup
600+
SessionsManager::reset();
601+
unset($_GET[$sessionName]);
602+
}
570603
}

0 commit comments

Comments
 (0)