bug #25220 [HttpFoundation] Add Session::isEmpty(), fix MockFileSessi…

…onStorage to behave like the native one (nicolas-grekas)

This PR was merged into the 3.4 branch.

Discussion
----------

[HttpFoundation] Add Session::isEmpty(), fix MockFileSessionStorage to behave like the native one

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

MockFileSessionStorage should not create any file when the session is empty. Like the native session storage, it should ignore the metadataBag to decide if the session is empty.

And to prevent AbstractTestSessionListener from registered a wrong cookie, it must have access to this empty state, which is now possible thanks to a new `Session::isEmpty()` method. Implementing is requires access to the internal storage of bags, which is possible via an internal proxy.

Commits
-------

56846ac [HttpFoundation] Add Session::isEmpty(), fix MockFileSessionStorage to behave like the native one

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(

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;
     }
 
     /**

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();
     }
 }

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();
+    }
+}

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

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());
+    }
 }

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']));
+            }
         }
     }
 

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')