bug #9625 [HttpFoundation] Do not return an empty session id if the session was closed (Taluu)

fabpot · fabpot · commit be0a3102cc06 · 2013-11-28T07:56:59.000+01:00
This PR was merged into the 2.2 branch. Discussion ---------- [HttpFoundation] Do not return an empty session id if the session was closed | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #7936 | License | MIT | Doc PR | ~ A regression from the bug fix #9246 was introduced due to an incomplete fix ; As the `started` flag on the NativeSessionStorage was not `true` anymore when saving the session, the session id was always empty when saving it, and thus when sending the `PHPSESSID` cookie. The previous PR I made (#9530) was another approach to solve this regression, but this one should keep the fix made by @tecbot on #9246, and finish it with another verification. I may miss another place where `started` is used, but I don't really see which other conditions depending on this property should be altered... **Note : this is #9611 rebased on 2.2** This should be mergeable. Commits ------- 5b9a727 When getting the session's id, check if the session is not closed
diff --git a/src/Symfony/Component/HttpFoundation/Session/Storage/NativeSessionStorage.php b/src/Symfony/Component/HttpFoundation/Session/Storage/NativeSessionStorage.php
@@ -161,7 +161,7 @@ public function start()
      */
     public function getId()
     {
-        if (!$this->started) {
+        if (!$this->started && !$this->closed) {
             return ''; // returning empty is consistent with session_id() behaviour
         }
 
diff --git a/src/Symfony/Component/HttpFoundation/Tests/Session/Storage/NativeSessionStorageTest.php b/src/Symfony/Component/HttpFoundation/Tests/Session/Storage/NativeSessionStorageTest.php
@@ -62,8 +62,12 @@ public function testGetId()
     {
         $storage = $this->getStorage();
         $this->assertEquals('', $storage->getId());
+
         $storage->start();
         $this->assertNotEquals('', $storage->getId());
+
+        $storage->save();
+        $this->assertNotEquals('', $storage->getId());
     }
 
     public function testRegenerate()

Original file line number	Diff line number	Diff line change
`@@ -161,7 +161,7 @@ public function start()`
`161`	`161`	`*/`
`162`	`162`	`public function getId()`
`163`	`163`	`{`
`164`		`- if (!$this->started) {`
	`164`	`+ if (!$this->started && !$this->closed) {`
`165`	`165`	`return ''; // returning empty is consistent with session_id() behaviour`
`166`	`166`	`}`
`167`	`167`
Original file line number	Diff line number	Diff line change
`@@ -62,8 +62,12 @@ public function testGetId()`
`62`	`62`	`{`
`63`	`63`	`$storage = $this->getStorage();`
`64`	`64`	`$this->assertEquals('', $storage->getId());`
	`65`	`+`
`65`	`66`	`$storage->start();`
`66`	`67`	`$this->assertNotEquals('', $storage->getId());`
	`68`	`+`
	`69`	`+ $storage->save();`
	`70`	`+ $this->assertNotEquals('', $storage->getId());`
`67`	`71`	`}`
`68`	`72`
`69`	`73`	`public function testRegenerate()`