[HttpFoundation] Add Session::isEmpty(), fix MockFileSessionStorage t…

…o behave like the native one
symfony · Nov 30, 2017 · 56846ac · 56846ac
1 parent d90a3b5
commit 56846ac
Show file tree

Hide file tree

Showing 8 changed files with 164 additions and 9 deletions.
diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/AbstractFactory.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/AbstractFactory.php
@@ -29,7 +29,7 @@ abstract class AbstractFactory implements SecurityFactoryInterface
     protected $options = array(
         'check_path' => '/login_check',
         'use_forward' => false,
-        'require_previous_session' => true,
+        'require_previous_session' => false,
     );
 
     protected $defaultSuccessHandlerOptions = array(

diff --git a/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/JsonLoginFactory.php b/src/Symfony/Bundle/SecurityBundle/DependencyInjection/Security/Factory/JsonLoginFactory.php
@@ -28,7 +28,6 @@ public function __construct()
         $this->addOption('password_path', 'password');
         $this->defaultFailureHandlerOptions = array();
         $this->defaultSuccessHandlerOptions = array();
-        $this->options['require_previous_session'] = false;
     }
 
     /**

diff --git a/src/Symfony/Component/HttpFoundation/Session/Session.php b/src/Symfony/Component/HttpFoundation/Session/Session.php
@@ -28,6 +28,7 @@ class Session implements SessionInterface, \IteratorAggregate, \Countable
 
     private $flashName;
     private $attributeName;
+    private $data = array();
 
     /**
      * @param SessionStorageInterface $storage    A SessionStorageInterface instance
@@ -108,7 +109,7 @@ public function remove($name)
      */
     public function clear()
     {
-        $this->storage->getBag($this->attributeName)->clear();
+        $this->getAttributeBag()->clear();
     }
 
     /**
@@ -139,6 +140,22 @@ public function count()
         return count($this->getAttributeBag()->all());
     }
 
+    /**
+     * @return bool
+     *
+     * @internal
+     */
+    public function isEmpty()
+    {
+        foreach ($this->data as &$data) {
+            if (!empty($data)) {
+                return false;
+            }
+        }
+
+        return true;
+    }
+
     /**
      * {@inheritdoc}
      */
@@ -210,15 +227,15 @@ public function getMetadataBag()
      */
     public function registerBag(SessionBagInterface $bag)
     {
-        $this->storage->registerBag($bag);
+        $this->storage->registerBag(new SessionBagProxy($bag, $this->data));
     }
 
     /**
      * {@inheritdoc}
      */
     public function getBag($name)
     {
-        return $this->storage->getBag($name);
+        return $this->storage->getBag($name)->getBag();
     }
 
     /**
@@ -240,6 +257,6 @@ public function getFlashBag()
      */
     private function getAttributeBag()
     {
-        return $this->storage->getBag($this->attributeName);
+        return $this->storage->getBag($this->attributeName)->getBag();
     }
 }
diff --git a/src/Symfony/Component/HttpFoundation/Session/SessionBagProxy.php b/src/Symfony/Component/HttpFoundation/Session/SessionBagProxy.php
@@ -0,0 +1,79 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\HttpFoundation\Session;
+
+/**
+ * @author Nicolas Grekas <p@tchwork.com>
+ *
+ * @internal
+ */
+final class SessionBagProxy implements SessionBagInterface
+{
+    private $bag;
+    private $data;
+
+    public function __construct(SessionBagInterface $bag, array &$data)
+    {
+        $this->bag = $bag;
+        $this->data = &$data;
+    }
+
+    /**
+     * @return SessionBagInterface
+     */
+    public function getBag()
+    {
+        return $this->bag;
+    }
+
+    /**
+     * @return bool
+     */
+    public function isEmpty()
+    {
+        return empty($this->data[$this->bag->getStorageKey()]);
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function getName()
+    {
+        return $this->bag->getName();
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function initialize(array &$array)
+    {
+        $this->data[$this->bag->getStorageKey()] = &$array;
+
+        $this->bag->initialize($array);
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function getStorageKey()
+    {
+        return $this->bag->getStorageKey();
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function clear()
+    {
+        return $this->bag->clear();
+    }
+}
diff --git a/src/Symfony/Component/HttpFoundation/Session/Storage/MockFileSessionStorage.php b/src/Symfony/Component/HttpFoundation/Session/Storage/MockFileSessionStorage.php
@@ -91,7 +91,26 @@ public function save()
             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));
+        $data = $this->data;
+
+        foreach ($this->bags as $bag) {
+            if (empty($data[$key = $bag->getStorageKey()])) {
+                unset($data[$key]);
+            }
+        }
+        if (array($key = $this->metadataBag->getStorageKey()) === array_keys($data)) {
+            unset($data[$key]);
+        }
+
+        try {
+            if ($data) {
+                file_put_contents($this->getFilePath(), serialize($data));
+            } else {
+                $this->destroy();
+            }
+        } finally {
+            $this->data = $data;
+        }
 
         // this is needed for Silex, where the session object is re-used across requests
         // in functional tests. In Symfony, the container is rebooted, so we don't have

diff --git a/src/Symfony/Component/HttpFoundation/Tests/Session/SessionTest.php b/src/Symfony/Component/HttpFoundation/Tests/Session/SessionTest.php
@@ -221,4 +221,22 @@ public function testGetMeta()
     {
         $this->assertInstanceOf('Symfony\Component\HttpFoundation\Session\Storage\MetadataBag', $this->session->getMetadataBag());
     }
+
+    public function testIsEmpty()
+    {
+        $this->assertTrue($this->session->isEmpty());
+
+        $this->session->set('hello', 'world');
+        $this->assertFalse($this->session->isEmpty());
+
+        $this->session->remove('hello');
+        $this->assertTrue($this->session->isEmpty());
+
+        $flash = $this->session->getFlashBag();
+        $flash->set('hello', 'world');
+        $this->assertFalse($this->session->isEmpty());
+
+        $flash->get('hello');
+        $this->assertTrue($this->session->isEmpty());
+    }
 }
diff --git a/src/Symfony/Component/HttpKernel/EventListener/AbstractTestSessionListener.php b/src/Symfony/Component/HttpKernel/EventListener/AbstractTestSessionListener.php
@@ -12,6 +12,7 @@
 namespace Symfony\Component\HttpKernel\EventListener;
 
 use Symfony\Component\HttpFoundation\Cookie;
+use Symfony\Component\HttpFoundation\Session\Session;
 use Symfony\Component\HttpFoundation\Session\SessionInterface;
 use Symfony\Component\HttpKernel\KernelEvents;
 use Symfony\Component\HttpKernel\Event\FilterResponseEvent;
@@ -60,8 +61,10 @@ public function onKernelResponse(FilterResponseEvent $event)
         $session = $event->getRequest()->getSession();
         if ($session && $session->isStarted()) {
             $session->save();
-            $params = session_get_cookie_params();
-            $event->getResponse()->headers->setCookie(new Cookie($session->getName(), $session->getId(), 0 === $params['lifetime'] ? 0 : time() + $params['lifetime'], $params['path'], $params['domain'], $params['secure'], $params['httponly']));
+            if (!$session instanceof Session || !\method_exists($session, 'isEmpty') || !$session->isEmpty()) {
+                $params = session_get_cookie_params();
+                $event->getResponse()->headers->setCookie(new Cookie($session->getName(), $session->getId(), 0 === $params['lifetime'] ? 0 : time() + $params['lifetime'], $params['path'], $params['domain'], $params['secure'], $params['httponly']));
+            }
         }
     }
 

diff --git a/src/Symfony/Component/HttpKernel/Tests/EventListener/TestSessionListenerTest.php b/src/Symfony/Component/HttpKernel/Tests/EventListener/TestSessionListenerTest.php
@@ -73,6 +73,19 @@ public function testDoesNotDeleteCookieIfUsingSessionLifetime()
         $this->assertEquals(0, reset($cookies)->getExpiresTime());
     }
 
+    /**
+     * @requires function \Symfony\Component\HttpFoundation\Session\Session::isEmpty
+     */
+    public function testEmptySessionDoesNotSendCookie()
+    {
+        $this->sessionHasBeenStarted();
+        $this->sessionIsEmpty();
+
+        $response = $this->filterResponse(new Request(), HttpKernelInterface::MASTER_REQUEST);
+
+        $this->assertSame(array(), $response->headers->getCookies());
+    }
+
     public function testUnstartedSessionIsNotSave()
     {
         $this->sessionHasNotBeenStarted();
@@ -130,6 +143,13 @@ private function sessionHasNotBeenStarted()
             ->will($this->returnValue(false));
     }
 
+    private function sessionIsEmpty()
+    {
+        $this->session->expects($this->once())
+            ->method('isEmpty')
+            ->will($this->returnValue(true));
+    }
+
     private function getSession()
     {
         $mock = $this->getMockBuilder('Symfony\Component\HttpFoundation\Session\Session')