symfony · fabpot · Dec 15, 2012 · Dec 14, 2012 · Dec 14, 2012 · ghost
diff --git a/src/Symfony/Bundle/FrameworkBundle/EventListener/TestSessionListener.php b/src/Symfony/Bundle/FrameworkBundle/EventListener/TestSessionListener.php
@@ -68,7 +68,9 @@ public function onKernelResponse(FilterResponseEvent $event)
         }
 
         if ($session = $event->getRequest()->getSession()) {
-            $session->save();
+            if ($session->isStarted()) {
+                $session->save();
+            }
 
             $params = session_get_cookie_params();
 

diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/EventListener/TestSessionListenerTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/EventListener/TestSessionListenerTest.php
@@ -43,6 +43,7 @@ protected function tearDown()
 
     public function testShouldSaveMasterRequestSession()
     {
+        $this->sessionHasBeenStarted();
         $this->sessionMustBeSaved();
 
         $this->filterResponse(new Request());
@@ -66,6 +67,14 @@ public function testDoesNotDeleteCookieIfUsingSessionLifetime()
         $this->assertEquals(0, reset($cookies)->getExpiresTime());
     }
 
+    public function testUnstartedSessionIsNotSave()
+    {
+        $this->sessionHasNotBeenStarted();
+        $this->sessionMustNotBeSaved();
+
+        $this->filterResponse(new Request());
+    }
+
     private function filterResponse(Request $request, $type = HttpKernelInterface::MASTER_REQUEST)
     {
         $request->setSession($this->session);
@@ -92,6 +101,20 @@ private function sessionMustBeSaved()
             ->method('save');
     }
 
+    private function sessionHasBeenStarted()
+    {
+        $this->session->expects($this->once())
+            ->method('isStarted')
+            ->will($this->returnValue(true));
+    }
+
+    private function sessionHasNotBeenStarted()
+    {
+        $this->session->expects($this->once())
+            ->method('isStarted')
+            ->will($this->returnValue(false));
+    }
+
     private function getSession()
     {
         $mock = $this->getMockBuilder('Symfony\Component\HttpFoundation\Session\Session')

diff --git a/src/Symfony/Component/HttpFoundation/Session/Storage/MockArraySessionStorage.php b/src/Symfony/Component/HttpFoundation/Session/Storage/MockArraySessionStorage.php
@@ -159,6 +159,9 @@ public function setName($name)
      */
     public function save()
     {
+        if (!$this->started || $this->closed) {
+            throw new \RuntimeException("Trying to save a session that was not started yet or was already closed");
+        }
         // nothing to do since we don't persist the session data
         $this->closed = false;
     }

diff --git a/src/Symfony/Component/HttpFoundation/Session/Storage/MockFileSessionStorage.php b/src/Symfony/Component/HttpFoundation/Session/Storage/MockFileSessionStorage.php
@@ -97,6 +97,10 @@ public function regenerate($destroy = false, $lifetime = null)
      */
     public function save()
     {
+        if (!$this->started) {
+            throw new \RuntimeException("Trying to save a session that was not started yet or was already closed");
+        }
+
         file_put_contents($this->getFilePath(), serialize($this->data));
 
         // this is needed for Silex, where the session object is re-used across requests

diff --git a/src/Symfony/Component/HttpFoundation/Session/Storage/SessionStorageInterface.php b/src/Symfony/Component/HttpFoundation/Session/Storage/SessionStorageInterface.php
@@ -110,6 +110,9 @@ public function regenerate($destroy = false, $lifetime = null);
      * used for a storage object design for unit or functional testing where
      * a real PHP session would interfere with testing, in which case it
      * it should actually persist the session data if required.
+     *
+     * @throws \RuntimeException If the session is saved without being started, or if the session
+     *                           is already closed.
      */
     public function save();
 

diff --git a/src/Symfony/Component/HttpFoundation/Tests/Session/SessionTest.php b/src/Symfony/Component/HttpFoundation/Tests/Session/SessionTest.php
@@ -151,6 +151,7 @@ public function testMigrateDestroy()
 
     public function testSave()
     {
+        $this->session->start();
         $this->session->save();
     }
 

diff --git a/src/Symfony/Component/HttpFoundation/Tests/Session/Storage/MockArraySessionStorageTest.php b/src/Symfony/Component/HttpFoundation/Tests/Session/Storage/MockArraySessionStorageTest.php
@@ -95,4 +95,12 @@ public function testGetId()
         $this->storage->start();
         $this->assertNotEquals('', $this->storage->getId());
     }
+
+    /**
+     * @expectedException RuntimeException
+     */
+    public function testUnstartedSave()
+    {
+        $this->storage->save();
+    }
 }
diff --git a/src/Symfony/Component/HttpFoundation/Tests/Session/Storage/MockFileSessionStorageTest.php b/src/Symfony/Component/HttpFoundation/Tests/Session/Storage/MockFileSessionStorageTest.php
@@ -106,6 +106,15 @@ public function testMultipleInstances()
         $this->assertEquals('bar', $storage2->getBag('attributes')->get('foo'), 'values persist between instances');
     }
 
+    /**
+     * @expectedException RuntimeException
+     */
+    public function testSaveWithoutStart()
+    {
+        $storage1 = $this->getStorage();
+        $storage1->save();
+    }
+
     private function getStorage()
     {
         $storage = new MockFileSessionStorage($this->sessionDir);
-Original file line number
+Diff line change
@@ Expand Up / @@ -151,6 +151,7 @@ public function testMigrateDestroy() @@
         public function testSave()
         {
+            $this->session->start();
             $this->session->save();
         }
@@ Expand Down @@