diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/session.xml b/src/Symfony/Bundle/FrameworkBundle/Resources/config/session.xml index f7903de64790f..111c75b4fd9c4 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/session.xml +++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/session.xml @@ -63,13 +63,14 @@ + - + The "%service_id%" service is deprecated since Symfony 4.1 and will be removed in 5.0. Use the "session_listener" service instead. diff --git a/src/Symfony/Component/HttpFoundation/Request.php b/src/Symfony/Component/HttpFoundation/Request.php index c78354b42dcb7..f40943e16e317 100644 --- a/src/Symfony/Component/HttpFoundation/Request.php +++ b/src/Symfony/Component/HttpFoundation/Request.php @@ -720,7 +720,12 @@ public function get($key, $default = null) */ public function getSession() { - return $this->session; + $session = $this->session; + if (!$session instanceof SessionInterface && null !== $session) { + $this->setSession($session = $session()); + } + + return $session; } /** @@ -732,7 +737,7 @@ public function getSession() public function hasPreviousSession() { // the check for $this->session avoids malicious users trying to fake a session cookie with proper name - return $this->hasSession() && $this->cookies->has($this->session->getName()); + return $this->hasSession() && $this->cookies->has($this->getSession()->getName()); } /** @@ -759,6 +764,14 @@ public function setSession(SessionInterface $session) $this->session = $session; } + /** + * @internal + */ + public function setSessionFactory(callable $factory) + { + $this->session = $factory; + } + /** * Returns the client IP addresses. * diff --git a/src/Symfony/Component/HttpKernel/EventListener/AbstractSessionListener.php b/src/Symfony/Component/HttpKernel/EventListener/AbstractSessionListener.php index dff29ee80b418..9865d6a79aba3 100644 --- a/src/Symfony/Component/HttpKernel/EventListener/AbstractSessionListener.php +++ b/src/Symfony/Component/HttpKernel/EventListener/AbstractSessionListener.php @@ -11,6 +11,7 @@ namespace Symfony\Component\HttpKernel\EventListener; +use Psr\Container\ContainerInterface; use Symfony\Component\HttpFoundation\Session\Session; use Symfony\Component\HttpFoundation\Session\SessionInterface; use Symfony\Component\HttpKernel\Event\FilterResponseEvent; @@ -22,9 +23,17 @@ * Sets the session in the request. * * @author Johannes M. Schmitt + * @author Tobias Schultze */ abstract class AbstractSessionListener implements EventSubscriberInterface { + protected $container; + + public function __construct(ContainerInterface $container = null) + { + $this->container = $container; + } + public function onKernelRequest(GetResponseEvent $event) { if (!$event->isMasterRequest()) { @@ -32,12 +41,13 @@ public function onKernelRequest(GetResponseEvent $event) } $request = $event->getRequest(); - $session = $this->getSession(); - if (null === $session || $request->hasSession()) { - return; + if ($request->hasSession()) { + // no-op + } elseif (method_exists($request, 'setSessionFactory')) { + $request->setSessionFactory(function () { return $this->getSession(); }); + } elseif ($session = $this->getSession()) { + $request->setSession($session); } - - $request->setSession($session); } public function onKernelResponse(FilterResponseEvent $event) @@ -46,7 +56,7 @@ public function onKernelResponse(FilterResponseEvent $event) return; } - if (!$session = $event->getRequest()->getSession()) { + if (!$session = $this->container && $this->container->has('initialized_session') ? $this->container->get('initialized_session') : $event->getRequest()->getSession()) { return; } @@ -56,13 +66,42 @@ public function onKernelResponse(FilterResponseEvent $event) ->setMaxAge(0) ->headers->addCacheControlDirective('must-revalidate'); } + + if ($session->isStarted()) { + /* + * Saves the session, in case it is still open, before sending the response/headers. + * + * This ensures several things in case the developer did not save the session explicitly: + * + * * If a session save handler without locking is used, it ensures the data is available + * on the next request, e.g. after a redirect. PHPs auto-save at script end via + * session_register_shutdown is executed after fastcgi_finish_request. So in this case + * the data could be missing the next request because it might not be saved the moment + * the new request is processed. + * * A locking save handler (e.g. the native 'files') circumvents concurrency problems like + * the one above. But by saving the session before long-running things in the terminate event, + * we ensure the session is not blocked longer than needed. + * * When regenerating the session ID no locking is involved in PHPs session design. See + * https://bugs.php.net/bug.php?id=61470 for a discussion. So in this case, the session must + * be saved anyway before sending the headers with the new session ID. Otherwise session + * data could get lost again for concurrent requests with the new ID. One result could be + * that you get logged out after just logging in. + * + * This listener should be executed as one of the last listeners, so that previous listeners + * can still operate on the open session. This prevents the overhead of restarting it. + * Listeners after closing the session can still work with the session as usual because + * Symfonys session implementation starts the session on demand. So writing to it after + * it is saved will just restart it. + */ + $session->save(); + } } public static function getSubscribedEvents() { return array( KernelEvents::REQUEST => array('onKernelRequest', 128), - // low priority to come after regular response listeners, same as SaveSessionListener + // low priority to come after regular response listeners, but higher than StreamedResponseListener KernelEvents::RESPONSE => array('onKernelResponse', -1000), ); } diff --git a/src/Symfony/Component/HttpKernel/EventListener/SaveSessionListener.php b/src/Symfony/Component/HttpKernel/EventListener/SaveSessionListener.php index 36809b59af914..a29121a3c66aa 100644 --- a/src/Symfony/Component/HttpKernel/EventListener/SaveSessionListener.php +++ b/src/Symfony/Component/HttpKernel/EventListener/SaveSessionListener.php @@ -11,36 +11,16 @@ namespace Symfony\Component\HttpKernel\EventListener; +@trigger_error(sprintf('The "%s" class is deprecated since Symfony 4.1 and will be removed in 5.0. Use AbstractSessionListener instead.', SaveSessionListener::class), E_USER_DEPRECATED); + use Symfony\Component\EventDispatcher\EventSubscriberInterface; use Symfony\Component\HttpKernel\Event\FilterResponseEvent; use Symfony\Component\HttpKernel\KernelEvents; /** - * Saves the session, in case it is still open, before sending the response/headers. - * - * This ensures several things in case the developer did not save the session explicitly: - * - * * If a session save handler without locking is used, it ensures the data is available - * on the next request, e.g. after a redirect. PHPs auto-save at script end via - * session_register_shutdown is executed after fastcgi_finish_request. So in this case - * the data could be missing the next request because it might not be saved the moment - * the new request is processed. - * * A locking save handler (e.g. the native 'files') circumvents concurrency problems like - * the one above. But by saving the session before long-running things in the terminate event, - * we ensure the session is not blocked longer than needed. - * * When regenerating the session ID no locking is involved in PHPs session design. See - * https://bugs.php.net/bug.php?id=61470 for a discussion. So in this case, the session must - * be saved anyway before sending the headers with the new session ID. Otherwise session - * data could get lost again for concurrent requests with the new ID. One result could be - * that you get logged out after just logging in. - * - * This listener should be executed as one of the last listeners, so that previous listeners - * can still operate on the open session. This prevents the overhead of restarting it. - * Listeners after closing the session can still work with the session as usual because - * Symfonys session implementation starts the session on demand. So writing to it after - * it is saved will just restart it. - * * @author Tobias Schultze + * + * @deprecated since Symfony 4.1, to be removed in 5.0. Use AbstractSessionListener instead. */ class SaveSessionListener implements EventSubscriberInterface { diff --git a/src/Symfony/Component/HttpKernel/EventListener/SessionListener.php b/src/Symfony/Component/HttpKernel/EventListener/SessionListener.php index 39ebfd922fac6..9f30eea38b42e 100644 --- a/src/Symfony/Component/HttpKernel/EventListener/SessionListener.php +++ b/src/Symfony/Component/HttpKernel/EventListener/SessionListener.php @@ -22,8 +22,6 @@ */ class SessionListener extends AbstractSessionListener { - private $container; - public function __construct(ContainerInterface $container) { $this->container = $container; diff --git a/src/Symfony/Component/HttpKernel/Fragment/InlineFragmentRenderer.php b/src/Symfony/Component/HttpKernel/Fragment/InlineFragmentRenderer.php index 5fa5b0ca8d340..e742845858dcc 100644 --- a/src/Symfony/Component/HttpKernel/Fragment/InlineFragmentRenderer.php +++ b/src/Symfony/Component/HttpKernel/Fragment/InlineFragmentRenderer.php @@ -124,9 +124,12 @@ protected function createSubRequest($uri, Request $request) $subRequest->headers->set('Surrogate-Capability', $request->headers->get('Surrogate-Capability')); } - if ($session = $request->getSession()) { - $subRequest->setSession($session); + static $setSession; + + if (null === $setSession) { + $setSession = \Closure::bind(function ($subRequest, $request) { $subRequest->session = $request->session; }, null, Request::class); } + $setSession($subRequest, $request); return $subRequest; } diff --git a/src/Symfony/Component/HttpKernel/Tests/EventListener/SaveSessionListenerTest.php b/src/Symfony/Component/HttpKernel/Tests/EventListener/SaveSessionListenerTest.php index 5492c3d784805..a903fd5891693 100644 --- a/src/Symfony/Component/HttpKernel/Tests/EventListener/SaveSessionListenerTest.php +++ b/src/Symfony/Component/HttpKernel/Tests/EventListener/SaveSessionListenerTest.php @@ -19,6 +19,9 @@ use Symfony\Component\HttpKernel\EventListener\SaveSessionListener; use Symfony\Component\HttpKernel\HttpKernelInterface; +/** + * @group legacy + */ class SaveSessionListenerTest extends TestCase { public function testOnlyTriggeredOnMasterRequest() diff --git a/src/Symfony/Component/HttpKernel/Tests/EventListener/SessionListenerTest.php b/src/Symfony/Component/HttpKernel/Tests/EventListener/SessionListenerTest.php index 34598363c8914..79fd88e4ab1cd 100644 --- a/src/Symfony/Component/HttpKernel/Tests/EventListener/SessionListenerTest.php +++ b/src/Symfony/Component/HttpKernel/Tests/EventListener/SessionListenerTest.php @@ -13,6 +13,7 @@ use PHPUnit\Framework\TestCase; use Symfony\Component\DependencyInjection\Container; +use Symfony\Component\DependencyInjection\ServiceLocator; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpFoundation\Session\Session; @@ -58,22 +59,33 @@ public function testSessionIsSet() public function testResponseIsPrivate() { $session = $this->getMockBuilder(Session::class)->disableOriginalConstructor()->getMock(); - $session->expects($this->once())->method('isStarted')->willReturn(false); + $session->expects($this->exactly(2))->method('isStarted')->willReturn(false); $session->expects($this->once())->method('hasBeenStarted')->willReturn(true); $container = new Container(); - $container->set('session', $session); + $container->set('initialized_session', $session); $listener = new SessionListener($container); $kernel = $this->getMockBuilder(HttpKernelInterface::class)->disableOriginalConstructor()->getMock(); - $request = new Request(); $response = new Response(); - $listener->onKernelRequest(new GetResponseEvent($kernel, $request, HttpKernelInterface::MASTER_REQUEST)); - $listener->onKernelResponse(new FilterResponseEvent($kernel, $request, HttpKernelInterface::MASTER_REQUEST, $response)); + $listener->onKernelResponse(new FilterResponseEvent($kernel, new Request(), HttpKernelInterface::MASTER_REQUEST, $response)); $this->assertTrue($response->headers->hasCacheControlDirective('private')); $this->assertTrue($response->headers->hasCacheControlDirective('must-revalidate')); $this->assertSame('0', $response->headers->getCacheControlDirective('max-age')); } + + public function testUninitilizedSession() + { + $event = $this->getMockBuilder(FilterResponseEvent::class)->disableOriginalConstructor()->getMock(); + $event->expects($this->once())->method('isMasterRequest')->willReturn(true); + + $container = new ServiceLocator(array( + 'initialized_session' => function () {}, + )); + + $listener = new SessionListener($container); + $listener->onKernelResponse($event); + } } diff --git a/src/Symfony/Component/Security/Http/HttpUtils.php b/src/Symfony/Component/Security/Http/HttpUtils.php index a55421340d495..b5762c464d4c4 100644 --- a/src/Symfony/Component/Security/Http/HttpUtils.php +++ b/src/Symfony/Component/Security/Http/HttpUtils.php @@ -77,9 +77,13 @@ public function createRedirectResponse(Request $request, $path, $status = 302) public function createRequest(Request $request, $path) { $newRequest = Request::create($this->generateUri($request, $path), 'get', array(), $request->cookies->all(), array(), $request->server->all()); - if ($request->hasSession()) { - $newRequest->setSession($request->getSession()); + + static $setSession; + + if (null === $setSession) { + $setSession = \Closure::bind(function ($newRequest, $request) { $newRequest->session = $request->session; }, null, Request::class); } + $setSession($newRequest, $request); if ($request->attributes->has(Security::AUTHENTICATION_ERROR)) { $newRequest->attributes->set(Security::AUTHENTICATION_ERROR, $request->attributes->get(Security::AUTHENTICATION_ERROR));