bug #25220 [HttpFoundation] Add Session::isEmpty(), fix MockFileSessionStorage 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
This commit is contained in:
Nicolas Grekas 2017-11-30 16:06:52 +01:00
commit 679eebb256
8 changed files with 164 additions and 9 deletions

View File

@ -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(

View File

@ -28,7 +28,6 @@ class JsonLoginFactory extends AbstractFactory
$this->addOption('password_path', 'password');
$this->defaultFailureHandlerOptions = array();
$this->defaultSuccessHandlerOptions = array();
$this->options['require_previous_session'] = false;
}
/**

View File

@ -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 @@ class Session implements SessionInterface, \IteratorAggregate, \Countable
*/
public function clear()
{
$this->storage->getBag($this->attributeName)->clear();
$this->getAttributeBag()->clear();
}
/**
@ -139,6 +140,22 @@ class Session implements SessionInterface, \IteratorAggregate, \Countable
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,7 +227,7 @@ class Session implements SessionInterface, \IteratorAggregate, \Countable
*/
public function registerBag(SessionBagInterface $bag)
{
$this->storage->registerBag($bag);
$this->storage->registerBag(new SessionBagProxy($bag, $this->data));
}
/**
@ -218,7 +235,7 @@ class Session implements SessionInterface, \IteratorAggregate, \Countable
*/
public function getBag($name)
{
return $this->storage->getBag($name);
return $this->storage->getBag($name)->getBag();
}
/**
@ -240,6 +257,6 @@ class Session implements SessionInterface, \IteratorAggregate, \Countable
*/
private function getAttributeBag()
{
return $this->storage->getBag($this->attributeName);
return $this->storage->getBag($this->attributeName)->getBag();
}
}

View File

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

View File

@ -91,7 +91,26 @@ class MockFileSessionStorage extends MockArraySessionStorage
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

View File

@ -221,4 +221,22 @@ class SessionTest extends TestCase
{
$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());
}
}

View File

@ -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 @@ abstract class AbstractTestSessionListener implements EventSubscriberInterface
$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']));
}
}
}

View File

@ -73,6 +73,19 @@ class TestSessionListenerTest extends TestCase
$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 @@ class TestSessionListenerTest extends TestCase
->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')