diff --git a/src/Symfony/Bundle/FrameworkBundle/EventListener/SessionListener.php b/src/Symfony/Bundle/FrameworkBundle/EventListener/SessionListener.php index 83fdec4b74..ad396347d5 100644 --- a/src/Symfony/Bundle/FrameworkBundle/EventListener/SessionListener.php +++ b/src/Symfony/Bundle/FrameworkBundle/EventListener/SessionListener.php @@ -28,7 +28,14 @@ use Symfony\Component\EventDispatcher\EventSubscriberInterface; */ class SessionListener implements EventSubscriberInterface { + /** + * @var ContainerInterface + */ private $container; + + /** + * @var boolean + */ private $autoStart; public function __construct(ContainerInterface $container, $autoStart = false) diff --git a/src/Symfony/Bundle/FrameworkBundle/EventListener/TestSessionListener.php b/src/Symfony/Bundle/FrameworkBundle/EventListener/TestSessionListener.php index 9010342d94..8f4536de8e 100644 --- a/src/Symfony/Bundle/FrameworkBundle/EventListener/TestSessionListener.php +++ b/src/Symfony/Bundle/FrameworkBundle/EventListener/TestSessionListener.php @@ -43,15 +43,16 @@ class TestSessionListener implements EventSubscriberInterface } // bootstrap the session - if ($this->container->has('session')) { - $this->container->get('session'); + if (!$this->container->has('session')) { + return; } + $session = $this->container->get('session'); $cookies = $event->getRequest()->cookies; - if ($cookies->has(session_name())) { - session_id($cookies->get(session_name())); + if ($cookies->has($session->getName())) { + $session->setId($cookies->get($session->getName())); } else { - session_id(''); + $session->setId(''); } } @@ -72,7 +73,7 @@ class TestSessionListener implements EventSubscriberInterface $params = session_get_cookie_params(); - $event->getResponse()->headers->setCookie(new Cookie(session_name(), $session->getId(), 0 === $params['lifetime'] ? 0 : time() + $params['lifetime'], $params['path'], $params['domain'], $params['secure'], $params['httponly'])); + $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/Bundle/FrameworkBundle/Tests/EventListener/TestSessionListenerTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/EventListener/TestSessionListenerTest.php index fe551ba360..4c577f292f 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/EventListener/TestSessionListenerTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/EventListener/TestSessionListenerTest.php @@ -94,8 +94,13 @@ class TestSessionListenerTest extends \PHPUnit_Framework_TestCase private function getSession() { - return $this->getMockBuilder('Symfony\Component\HttpFoundation\Session\Session') + $mock = $this->getMockBuilder('Symfony\Component\HttpFoundation\Session\Session') ->disableOriginalConstructor() ->getMock(); + + // set return value for getName() + $mock->expects($this->any())->method('getName')->will($this->returnValue('MOCKSESSID')); + + return $mock; } } diff --git a/src/Symfony/Component/HttpFoundation/Request.php b/src/Symfony/Component/HttpFoundation/Request.php index f38b1e7d0f..eaa453e1b4 100644 --- a/src/Symfony/Component/HttpFoundation/Request.php +++ b/src/Symfony/Component/HttpFoundation/Request.php @@ -496,7 +496,8 @@ class Request public function hasPreviousSession() { // the check for $this->session avoids malicious users trying to fake a session cookie with proper name - return $this->cookies->has(session_name()) && null !== $this->session; + $sessionName = $this->hasSession() ? $this->session->getName() : null; + return $this->cookies->has($sessionName) && $this->hasSession(); } /** diff --git a/src/Symfony/Component/HttpFoundation/Session/Session.php b/src/Symfony/Component/HttpFoundation/Session/Session.php index 6f2c6a4262..9f2e3326e8 100644 --- a/src/Symfony/Component/HttpFoundation/Session/Session.php +++ b/src/Symfony/Component/HttpFoundation/Session/Session.php @@ -17,6 +17,7 @@ use Symfony\Component\HttpFoundation\Session\Attribute\AttributeBagInterface; use Symfony\Component\HttpFoundation\Session\Flash\FlashBag; use Symfony\Component\HttpFoundation\Session\Flash\FlashBagInterface; use Symfony\Component\HttpFoundation\Session\SessionBagInterface; +use Symfony\Component\HttpFoundation\Session\Storage\SessionStorage; /** * Session. @@ -140,11 +141,7 @@ class Session implements SessionInterface } /** - * Returns the session ID - * - * @return mixed The session ID - * - * @api + * {@inheritdoc} */ public function getId() { @@ -152,7 +149,31 @@ class Session implements SessionInterface } /** - * Registers a SessionBagInterface with the sessio. + * {@inheritdoc} + */ + public function setId($id) + { + $this->storage->setId($id); + } + + /** + * {@inheritdoc} + */ + public function getName() + { + return $this->storage->getName(); + } + + /** + * {@inheritdoc} + */ + public function setName($name) + { + $this->storage->setName($name); + } + + /** + * Registers a SessionBagInterface with the session. * * @param SessionBagInterface $bag */ diff --git a/src/Symfony/Component/HttpFoundation/Session/SessionInterface.php b/src/Symfony/Component/HttpFoundation/Session/SessionInterface.php index 741969b10e..4e4962de4e 100644 --- a/src/Symfony/Component/HttpFoundation/Session/SessionInterface.php +++ b/src/Symfony/Component/HttpFoundation/Session/SessionInterface.php @@ -29,6 +29,42 @@ interface SessionInterface */ function start(); + /** + * Returns the session ID. + * + * @return string The session ID. + * + * @api + */ + function getId(); + + /** + * Sets the session ID + * + * @param string $id + * + * @api + */ + function setId($id); + + /** + * Returns the session name. + * + * @return mixed The session name. + * + * @api + */ + function getName(); + + /** + * Sets the session name. + * + * @param string $name + * + * @api + */ + function setName($name); + /** * Invalidates the current session. * diff --git a/src/Symfony/Component/HttpFoundation/Session/Storage/Handler/NativeFileSessionHandler.php b/src/Symfony/Component/HttpFoundation/Session/Storage/Handler/NativeFileSessionHandler.php index 5acf004d15..202d8d0f90 100644 --- a/src/Symfony/Component/HttpFoundation/Session/Storage/Handler/NativeFileSessionHandler.php +++ b/src/Symfony/Component/HttpFoundation/Session/Storage/Handler/NativeFileSessionHandler.php @@ -12,7 +12,7 @@ namespace Symfony\Component\HttpFoundation\Session\Storage\Handler; /** - * NativeFileSessionStorage. + * NativeFileSessionHandler. * * Native session handler using PHP's built in file storage. * diff --git a/src/Symfony/Component/HttpFoundation/Session/Storage/MockArraySessionStorage.php b/src/Symfony/Component/HttpFoundation/Session/Storage/MockArraySessionStorage.php index 482f3b6c20..6f1e279f41 100644 --- a/src/Symfony/Component/HttpFoundation/Session/Storage/MockArraySessionStorage.php +++ b/src/Symfony/Component/HttpFoundation/Session/Storage/MockArraySessionStorage.php @@ -11,7 +11,7 @@ namespace Symfony\Component\HttpFoundation\Session\Storage; -use Symfony\Component\HttpFoundation\Session\Storage\Handler\NullSessionHandler; +use Symfony\Component\HttpFoundation\Session\SessionBagInterface; /** * MockArraySessionStorage mocks the session for unit tests. @@ -25,21 +25,41 @@ use Symfony\Component\HttpFoundation\Session\Storage\Handler\NullSessionHandler; * @author Bulat Shakirzyanov * @author Drak */ -class MockArraySessionStorage extends SessionStorage +class MockArraySessionStorage implements SessionStorageInterface { /** * @var string */ - protected $sessionId; + protected $id = ''; + + /** + * @var string + */ + protected $name; + + /** + * @var boolean + */ + protected $started = false; + + /** + * @var boolean + */ + protected $closed = false; /** * @var array */ - protected $sessionData = array(); + protected $data = array(); - public function __construct(array $options = array()) + /** + * Constructor. + * + * @param string $name Session name + */ + public function __construct($name = 'MOCKSESSID') { - parent::__construct($options, new NullSessionHandler()); + $this->name = $name; } /** @@ -49,7 +69,7 @@ class MockArraySessionStorage extends SessionStorage */ public function setSessionData(array $array) { - $this->sessionData = $array; + $this->data = $array; } /** @@ -61,11 +81,11 @@ class MockArraySessionStorage extends SessionStorage return true; } - $this->started = true; - $this->loadSession($this->sessionData); + if (empty($this->id)) { + $this->id = $this->generateId(); + } - $this->sessionId = $this->generateSessionId(); - session_id($this->sessionId); + $this->loadSession(); return true; } @@ -80,8 +100,7 @@ class MockArraySessionStorage extends SessionStorage $this->start(); } - $this->sessionId = $this->generateSessionId(); - session_id($this->sessionId); + $this->id = $this->generateId(); return true; } @@ -91,11 +110,35 @@ class MockArraySessionStorage extends SessionStorage */ public function getId() { - if (!$this->started) { - return ''; + return $this->id; + } + + /** + * {@inheritdoc} + */ + public function setId($id) + { + if ($this->started) { + throw new \LogicException('Cannot set session ID after the session has started.'); } - return $this->sessionId; + $this->id = $id; + } + + /** + * {@inheritdoc} + */ + public function getName() + { + return $this->name; + } + + /** + * {@inheritdoc} + */ + public function setName($name) + { + $this->name = $name; } /** @@ -118,10 +161,18 @@ class MockArraySessionStorage extends SessionStorage } // clear out the session - $this->sessionData = array(); + $this->data = array(); // reconnect the bags to the session - $this->loadSession($this->sessionData); + $this->loadSession(); + } + + /** + * {@inheritdoc} + */ + public function registerBag(SessionBagInterface $bag) + { + $this->bags[$bag->getName()] = $bag; } /** @@ -143,10 +194,25 @@ class MockArraySessionStorage extends SessionStorage /** * Generates a session ID. * + * This doesn't need to be particularly cryptographically secure since this is just + * a mock. + * * @return string */ - protected function generateSessionId() + protected function generateId() { - return sha1(uniqid(mt_rand(), true)); + return sha1(uniqid(mt_rand())); + } + + protected function loadSession() + { + foreach ($this->bags as $bag) { + $key = $bag->getStorageKey(); + $this->data[$key] = isset($this->data[$key]) ? $this->data[$key] : array(); + $bag->initialize($this->data[$key]); + } + + $this->started = true; + $this->closed = false; } } diff --git a/src/Symfony/Component/HttpFoundation/Session/Storage/MockFileSessionStorage.php b/src/Symfony/Component/HttpFoundation/Session/Storage/MockFileSessionStorage.php index 184f76e747..24457319f9 100644 --- a/src/Symfony/Component/HttpFoundation/Session/Storage/MockFileSessionStorage.php +++ b/src/Symfony/Component/HttpFoundation/Session/Storage/MockFileSessionStorage.php @@ -16,7 +16,9 @@ namespace Symfony\Component\HttpFoundation\Session\Storage; * functional testing when done in a single PHP process. * * No PHP session is actually started since a session can be initialized - * and shutdown only once per PHP execution cycle. + * and shutdown only once per PHP execution cycle and this class does + * not pollute any session related globals, including session_*() functions + * or session.* PHP ini directives. * * @author Drak */ @@ -31,11 +33,9 @@ class MockFileSessionStorage extends MockArraySessionStorage * Constructor. * * @param string $savePath Path of directory to save session files. - * @param array $options Session options. - * - * @see SessionStorage::__construct() + * @param string $name Session name. */ - public function __construct($savePath = null, array $options = array()) + public function __construct($savePath = null, $name = 'MOCKSESSID') { if (null === $savePath) { $savePath = sys_get_temp_dir(); @@ -47,7 +47,7 @@ class MockFileSessionStorage extends MockArraySessionStorage $this->savePath = $savePath; - parent::__construct($options); + parent::__construct($name); } /** @@ -59,12 +59,10 @@ class MockFileSessionStorage extends MockArraySessionStorage return true; } - if (!session_id()) { - session_id($this->generateSessionId()); + if (!$this->id) { + $this->id = $this->generateId(); } - $this->sessionId = session_id(); - $this->read(); $this->started = true; @@ -81,10 +79,7 @@ class MockFileSessionStorage extends MockArraySessionStorage $this->destroy(); } - session_id($this->generateSessionId()); - $this->sessionId = session_id(); - - $this->save(); + $this->id = $this->generateId(); return true; } @@ -92,23 +87,15 @@ class MockFileSessionStorage extends MockArraySessionStorage /** * {@inheritdoc} */ - public function getId() + public function save() { - if (!$this->started) { - return ''; - } - - return $this->sessionId; + file_put_contents($this->getFilePath(), serialize($this->data)); } /** - * {@inheritdoc} + * Deletes a session from persistent storage. + * Deliberately leaves session data in memory intact. */ - public function save() - { - file_put_contents($this->getFilePath(), serialize($this->sessionData)); - } - private function destroy() { if (is_file($this->getFilePath())) { @@ -121,16 +108,19 @@ class MockFileSessionStorage extends MockArraySessionStorage * * @return string File path */ - public function getFilePath() + private function getFilePath() { - return $this->savePath.'/'.$this->sessionId.'.sess'; + return $this->savePath.'/'.$this->id.'.mocksess'; } + /** + * Reads session from storage and loads session. + */ private function read() { $filePath = $this->getFilePath(); - $this->sessionData = is_readable($filePath) && is_file($filePath) ? unserialize(file_get_contents($filePath)) : array(); + $this->data = is_readable($filePath) && is_file($filePath) ? unserialize(file_get_contents($filePath)) : array(); - $this->loadSession($this->sessionData); + $this->loadSession(); } } diff --git a/src/Symfony/Component/HttpFoundation/Session/Storage/Proxy/AbstractProxy.php b/src/Symfony/Component/HttpFoundation/Session/Storage/Proxy/AbstractProxy.php index 0cd4d73d96..09f9efa091 100644 --- a/src/Symfony/Component/HttpFoundation/Session/Storage/Proxy/AbstractProxy.php +++ b/src/Symfony/Component/HttpFoundation/Session/Storage/Proxy/AbstractProxy.php @@ -84,4 +84,52 @@ abstract class AbstractProxy { $this->active = (bool) $flag; } + + /** + * Gets the session ID. + * + * @return string + */ + public function getId() + { + return session_id(); + } + + /** + * Sets the session ID. + * + * @param string $id + */ + public function setId($id) + { + if ($this->isActive()) { + throw new \LogicException('Cannot change the ID of an active session'); + } + + session_id($id); + } + + /** + * Gets the session name. + * + * @return string + */ + public function getName() + { + return session_name(); + } + + /** + * Sets the session name. + * + * @param string $name + */ + public function setName($name) + { + if ($this->isActive()) { + throw new \LogicException('Cannot change the name of an active session'); + } + + session_name($name); + } } diff --git a/src/Symfony/Component/HttpFoundation/Session/Storage/SessionStorage.php b/src/Symfony/Component/HttpFoundation/Session/Storage/SessionStorage.php index 47a6044e87..194318aa2e 100644 --- a/src/Symfony/Component/HttpFoundation/Session/Storage/SessionStorage.php +++ b/src/Symfony/Component/HttpFoundation/Session/Storage/SessionStorage.php @@ -93,6 +93,12 @@ class SessionStorage implements SessionStorageInterface ini_set('session.cache_limiter', ''); // disable by default because it's managed by HeaderBag (if used) ini_set('session.use_cookies', 1); + if (version_compare(phpversion(), '5.4.0', '>=')) { + session_register_shutdown(); + } else { + register_shutdown_function('session_write_close'); + } + $this->setOptions($options); $this->setSaveHandler($handler); } @@ -151,7 +157,31 @@ class SessionStorage implements SessionStorageInterface return ''; // returning empty is consistent with session_id() behaviour } - return session_id(); + return $this->saveHandler->getId(); + } + + /** + * {@inheritdoc} + */ + public function setId($id) + { + return $this->saveHandler->setId($id); + } + + /** + * {@inheritdoc} + */ + public function getName() + { + return $this->saveHandler->getName(); + } + + /** + * {@inheritdoc} + */ + public function setName($name) + { + $this->saveHandler->setName($name); } /** @@ -275,7 +305,7 @@ class SessionStorage implements SessionStorageInterface if ($this->saveHandler instanceof \SessionHandlerInterface) { if (version_compare(phpversion(), '5.4.0', '>=')) { - session_set_save_handler($this->saveHandler, true); + session_set_save_handler($this->saveHandler, false); } else { session_set_save_handler( array($this->saveHandler, 'open'), @@ -285,8 +315,6 @@ class SessionStorage implements SessionStorageInterface array($this->saveHandler, 'destroy'), array($this->saveHandler, 'gc') ); - - register_shutdown_function('session_write_close'); } } } diff --git a/src/Symfony/Component/HttpFoundation/Session/Storage/SessionStorageInterface.php b/src/Symfony/Component/HttpFoundation/Session/Storage/SessionStorageInterface.php index 6065a550dc..8bf2e5d32a 100644 --- a/src/Symfony/Component/HttpFoundation/Session/Storage/SessionStorageInterface.php +++ b/src/Symfony/Component/HttpFoundation/Session/Storage/SessionStorageInterface.php @@ -37,12 +37,39 @@ interface SessionStorageInterface /** * Returns the session ID * - * @return mixed The session ID or false if the session has not started. + * @return string The session ID or empty. * * @api */ function getId(); + /** + * Sets the session ID + * + * @param string $id + * + * @api + */ + function setId($id); + + /** + * Returns the session name + * + * @return mixed The session name. + * + * @api + */ + function getName(); + + /** + * Sets the session name + * + * @param string $name + * + * @api + */ + function setName($name); + /** * Regenerates id that represents this storage. * @@ -51,6 +78,9 @@ interface SessionStorageInterface * or functional testing where a real PHP session would interfere * with testing. * + * Note regenerate+destroy should not clear the session data in memory + * only delete the session data from persistent storage. + * * @param Boolean $destroy Destroy session when regenerating? * * @return Boolean True if session regenerated, false if error diff --git a/tests/Symfony/Tests/Component/HttpFoundation/RequestTest.php b/tests/Symfony/Tests/Component/HttpFoundation/RequestTest.php index 3e672da273..136b8fc95b 100644 --- a/tests/Symfony/Tests/Component/HttpFoundation/RequestTest.php +++ b/tests/Symfony/Tests/Component/HttpFoundation/RequestTest.php @@ -866,7 +866,7 @@ class RequestTest extends \PHPUnit_Framework_TestCase public function testHasSession() { - $request = new Request; + $request = new Request(); $this->assertFalse($request->hasSession()); $request->setSession(new Session(new MockArraySessionStorage())); @@ -875,10 +875,10 @@ class RequestTest extends \PHPUnit_Framework_TestCase public function testHasPreviousSession() { - $request = new Request; + $request = new Request(); $this->assertFalse($request->hasPreviousSession()); - $request->cookies->set(session_name(), 'foo'); + $request->cookies->set('MOCKSESSID', 'foo'); $this->assertFalse($request->hasPreviousSession()); $request->setSession(new Session(new MockArraySessionStorage())); $this->assertTrue($request->hasPreviousSession()); diff --git a/tests/Symfony/Tests/Component/HttpFoundation/Session/Storage/Handler/NativeFileSessionHandlerTest.php b/tests/Symfony/Tests/Component/HttpFoundation/Session/Storage/Handler/NativeFileSessionHandlerTest.php index 08b8644ebf..37eb40cbe9 100644 --- a/tests/Symfony/Tests/Component/HttpFoundation/Session/Storage/Handler/NativeFileSessionHandlerTest.php +++ b/tests/Symfony/Tests/Component/HttpFoundation/Session/Storage/Handler/NativeFileSessionHandlerTest.php @@ -6,7 +6,7 @@ use Symfony\Component\HttpFoundation\Session\Storage\Handler\NativeFileSessionHa use Symfony\Component\HttpFoundation\Session\Storage\SessionStorage; /** - * Test class for NativeFileSessionStorage. + * Test class for NativeFileSessionHandler. * * @author Drak * diff --git a/tests/Symfony/Tests/Component/HttpFoundation/Session/Storage/MockFileSessionStorageTest.php b/tests/Symfony/Tests/Component/HttpFoundation/Session/Storage/MockFileSessionStorageTest.php index 770bcda8f0..c0453df977 100644 --- a/tests/Symfony/Tests/Component/HttpFoundation/Session/Storage/MockFileSessionStorageTest.php +++ b/tests/Symfony/Tests/Component/HttpFoundation/Session/Storage/MockFileSessionStorageTest.php @@ -69,6 +69,7 @@ class MockFileSessionStorageTest extends \PHPUnit_Framework_TestCase public function testSave() { $this->storage->start(); + $id = $this->storage->getId(); $this->assertNotEquals('108', $this->storage->getBag('attributes')->get('new')); $this->assertFalse($this->storage->getBag('flashes')->has('newkey')); $this->storage->getBag('attributes')->set('new', '108'); @@ -76,6 +77,7 @@ class MockFileSessionStorageTest extends \PHPUnit_Framework_TestCase $this->storage->save(); $storage = $this->getStorage(); + $storage->setId($id); $storage->start(); $this->assertEquals('108', $storage->getBag('attributes')->get('new')); $this->assertTrue($storage->getBag('flashes')->has('newkey')); @@ -90,13 +92,14 @@ class MockFileSessionStorageTest extends \PHPUnit_Framework_TestCase $storage1->save(); $storage2 = $this->getStorage(); + $storage2->setId($storage1->getId()); $storage2->start(); $this->assertEquals('bar', $storage2->getBag('attributes')->get('foo'), 'values persist between instances'); } - private function getStorage(array $options = array()) + private function getStorage() { - $storage = new MockFileSessionStorage($this->sessionDir, $options); + $storage = new MockFileSessionStorage($this->sessionDir); $storage->registerBag(new FlashBag); $storage->registerBag(new AttributeBag);