From 12828e7665e89b400c3f0a564b8ad4c0c0a344c7 Mon Sep 17 00:00:00 2001 From: Ismael Ambrosi Date: Mon, 14 Sep 2015 18:59:44 -0300 Subject: [PATCH 1/3] Test `regenerate` wrongly sets storage as started The Session Storage should not be flagged as started if `session_regenerate_id` fails. A common use case would be to attempt regeneration before actually starting the session. --- .../Tests/Session/Storage/NativeSessionStorageTest.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/Symfony/Component/HttpFoundation/Tests/Session/Storage/NativeSessionStorageTest.php b/src/Symfony/Component/HttpFoundation/Tests/Session/Storage/NativeSessionStorageTest.php index c8743aba943e..531b6a371382 100644 --- a/src/Symfony/Component/HttpFoundation/Tests/Session/Storage/NativeSessionStorageTest.php +++ b/src/Symfony/Component/HttpFoundation/Tests/Session/Storage/NativeSessionStorageTest.php @@ -130,6 +130,13 @@ public function testSessionGlobalIsUpToDateAfterIdRegeneration() $this->assertEquals(42, $_SESSION['_sf2_attributes']['lucky']); } + public function testRegenerationFailureDoesNotFlagStorageAsStarted() + { + $storage = $this->getStorage(); + $this->assertFalse($storage->regenerate()); + $this->assertFalse($storage->isStarted()); + } + public function testDefaultSessionCacheLimiter() { $this->iniSet('session.cache_limiter', 'nocache'); From a269a914695c5a93bf40380b5dfa9e606f004bb1 Mon Sep 17 00:00:00 2001 From: Ismael Ambrosi Date: Mon, 14 Sep 2015 19:43:08 -0300 Subject: [PATCH 2/3] Don't regenerate the id of non-active sessions This is to avoid flagging the storage as started(done by `loadSession()`) if the regeneration fails. Also, PHP 7 will throw an error if a regeneration is attempted for non-active sessions. --- .../HttpFoundation/Session/Storage/NativeSessionStorage.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/Symfony/Component/HttpFoundation/Session/Storage/NativeSessionStorage.php b/src/Symfony/Component/HttpFoundation/Session/Storage/NativeSessionStorage.php index db705db87c48..ea52ddaa91f6 100644 --- a/src/Symfony/Component/HttpFoundation/Session/Storage/NativeSessionStorage.php +++ b/src/Symfony/Component/HttpFoundation/Session/Storage/NativeSessionStorage.php @@ -195,6 +195,11 @@ public function setName($name) */ public function regenerate($destroy = false, $lifetime = null) { + // Cannot regenerate the session ID for non-active sessions. + if ('' === session_id()) { + return false; + } + if (null !== $lifetime) { ini_set('session.cookie_lifetime', $lifetime); } From a492a7a68f04a69b6809373ea659fe379465d993 Mon Sep 17 00:00:00 2001 From: Ismael Ambrosi Date: Tue, 22 Sep 2015 14:59:00 -0300 Subject: [PATCH 3/3] Fix check for active sessions in PHP >= 5.4 `session_status()` should be used in PHP >= 5.4 instead of `session_id()`. --- .../Session/Storage/NativeSessionStorage.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/Symfony/Component/HttpFoundation/Session/Storage/NativeSessionStorage.php b/src/Symfony/Component/HttpFoundation/Session/Storage/NativeSessionStorage.php index ea52ddaa91f6..078544cfcbf0 100644 --- a/src/Symfony/Component/HttpFoundation/Session/Storage/NativeSessionStorage.php +++ b/src/Symfony/Component/HttpFoundation/Session/Storage/NativeSessionStorage.php @@ -196,7 +196,12 @@ public function setName($name) public function regenerate($destroy = false, $lifetime = null) { // Cannot regenerate the session ID for non-active sessions. - if ('' === session_id()) { + if (PHP_VERSION_ID >= 50400 && \PHP_SESSION_ACTIVE !== session_status()) { + return false; + } + + // Check if session ID exists in PHP 5.3 + if (PHP_VERSION_ID < 50400 && '' === session_id()) { return false; }