Remove initialized_session

symfony · Jul 6, 2021 · 1ce3641 · 1ce3641
1 parent 352f839
commit 1ce3641
Show file tree

Hide file tree

Showing 5 changed files with 42 additions and 85 deletions.
diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
@@ -78,7 +78,6 @@
 use Symfony\Component\HttpKernel\Controller\ArgumentValueResolverInterface;
 use Symfony\Component\HttpKernel\DataCollector\DataCollectorInterface;
 use Symfony\Component\HttpKernel\DependencyInjection\Extension;
-use Symfony\Component\HttpKernel\EventListener\TestSessionListener;
 use Symfony\Component\Lock\Lock;
 use Symfony\Component\Lock\LockFactory;
 use Symfony\Component\Lock\PersistingStoreInterface;

diff --git a/src/Symfony/Component/HttpKernel/EventListener/AbstractSessionListener.php b/src/Symfony/Component/HttpKernel/EventListener/AbstractSessionListener.php
@@ -41,7 +41,6 @@ abstract class AbstractSessionListener implements EventSubscriberInterface
     public const NO_AUTO_CACHE_CONTROL_HEADER = 'Symfony-Session-NoAutoCacheControl';
 
     protected $container;
-    private $sessionUsageStack = [];
     private $debug;
 
     public function __construct(ContainerInterface $container = null, bool $debug = false)
@@ -62,10 +61,6 @@ public function onKernelRequest(RequestEvent $event)
             $sess = null;
             $request->setSessionFactory(function () use (&$sess) { return $sess ?? $sess = $this->getSession(); });
         }
-
-        // Code related to `initialized_session` can be removed when symfony/http-kernel will stop being compatible with symfony/framework-bundle<6.0
-        $session = $this->container && $this->container->has('initialized_session') ? $this->container->get('initialized_session') : null;
-        $this->sessionUsageStack[] = $session instanceof Session ? $session->getUsageIndex() : 0;
     }
 
     public function onKernelResponse(ResponseEvent $event)
@@ -79,9 +74,10 @@ public function onKernelResponse(ResponseEvent $event)
         // Always remove the internal header if present
         $response->headers->remove(self::NO_AUTO_CACHE_CONTROL_HEADER);
 
-        if (!$session = $this->container && $this->container->has('initialized_session') ? $this->container->get('initialized_session') : $event->getRequest()->getSession()) {
+        if (!$event->getRequest()->hasSession()) {
             return;
         }
+        $session = $event->getRequest()->getSession();
 
         if ($session->isStarted()) {
             /*
@@ -112,7 +108,7 @@ public function onKernelResponse(ResponseEvent $event)
             $session->save();
         }
 
-        if ($session instanceof Session ? $session->getUsageIndex() === end($this->sessionUsageStack) : !$session->isStarted()) {
+        if ($session instanceof Session ? $session->getUsageIndex() === 0 : !$session->isStarted()) {
             return;
         }
 
@@ -137,13 +133,6 @@ public function onKernelResponse(ResponseEvent $event)
         }
     }
 
-    public function onFinishRequest(FinishRequestEvent $event)
-    {
-        if ($event->isMainRequest()) {
-            array_pop($this->sessionUsageStack);
-        }
-    }
-
     public function onSessionUsage(): void
     {
         if (!$this->debug) {
@@ -168,7 +157,7 @@ public function onSessionUsage(): void
             return;
         }
 
-        if (!$session = $this->container && $this->container->has('initialized_session') ? $this->container->get('initialized_session') : $requestStack->getCurrentRequest()->getSession()) {
+        if (!$session = $requestStack->getCurrentRequest()->getSession()) {
             return;
         }
 
@@ -185,7 +174,6 @@ public static function getSubscribedEvents(): array
             KernelEvents::REQUEST => ['onKernelRequest', 128],
             // low priority to come after regular response listeners, but higher than StreamedResponseListener
             KernelEvents::RESPONSE => ['onKernelResponse', -1000],
-            KernelEvents::FINISH_REQUEST => ['onFinishRequest'],
         ];
     }
 

diff --git a/src/Symfony/Component/HttpKernel/EventListener/AbstractTestSessionListener.php b/src/Symfony/Component/HttpKernel/EventListener/AbstractTestSessionListener.php
@@ -100,7 +100,7 @@ public function onKernelResponse(ResponseEvent $event)
     public static function getSubscribedEvents(): array
     {
         return [
-            KernelEvents::REQUEST => ['onKernelRequest', 127], // After SessionListener
+            KernelEvents::REQUEST => ['onKernelRequest', 127], // AFTER SessionListener
             KernelEvents::RESPONSE => ['onKernelResponse', -128],
         ];
     }

diff --git a/src/Symfony/Component/HttpKernel/EventListener/SessionListener.php b/src/Symfony/Component/HttpKernel/EventListener/SessionListener.php
@@ -19,10 +19,6 @@
 /**
  * Sets the session in the request.
  *
- * When the passed container contains a "session_storage" entry which
- * holds a NativeSessionStorage instance, the "cookie_secure" option
- * will be set to true whenever the current main request is secure.
- *
  * @author Fabien Potencier <fabien@symfony.com>
  *
  * @final
@@ -34,29 +30,8 @@ public function __construct(ContainerInterface $container, bool $debug = false)
         parent::__construct($container, $debug);
     }
 
-    public function onKernelRequest(RequestEvent $event)
-    {
-        parent::onKernelRequest($event);
-
-        if (!$event->isMainRequest() || (!$this->container->has('session') && !$this->container->has('session_factory'))) {
-            return;
-        }
-
-        if ($this->container->has('session_storage')
-            && ($storage = $this->container->get('session_storage')) instanceof NativeSessionStorage
-            && ($mainRequest = $this->container->get('request_stack')->getMainRequest())
-            && $mainRequest->isSecure()
-        ) {
-            $storage->setOptions(['cookie_secure' => true]);
-        }
-    }
-
     protected function getSession(): ?SessionInterface
     {
-        if ($this->container->has('session')) {
-            return $this->container->get('session');
-        }
-
         if ($this->container->has('session_factory')) {
             return $this->container->get('session_factory')->createSession();
         }

diff --git a/src/Symfony/Component/HttpKernel/Tests/EventListener/SessionListenerTest.php b/src/Symfony/Component/HttpKernel/Tests/EventListener/SessionListenerTest.php
@@ -31,10 +31,6 @@
 use Symfony\Component\HttpKernel\HttpKernelInterface;
 use Symfony\Component\HttpKernel\KernelInterface;
 
-/**
- * Tests related to `initialized_session` and `session` can be updated when symfony/http-kernel will stop being
- * compatible with symfony/framework-bundle<6.0
- */
 class SessionListenerTest extends TestCase
 {
     public function testOnlyTriggeredOnMainRequest()
@@ -51,23 +47,23 @@ public function testOnlyTriggeredOnMainRequest()
     public function testSessionIsSet()
     {
         $session = $this->createMock(Session::class);
+        $sessionFactory = $this->createMock(SessionFactory::class);
+        $sessionFactory->expects($this->once())->method('createSession')->willReturn($session);
 
         $requestStack = $this->createMock(RequestStack::class);
-        $requestStack->expects($this->once())->method('getMainRequest')->willReturn(null);
 
         $sessionStorage = $this->createMock(NativeSessionStorage::class);
         $sessionStorage->expects($this->never())->method('setOptions')->with(['cookie_secure' => true]);
 
         $container = new Container();
-        $container->set('session', $session);
+        $container->set('session_factory', $sessionFactory);
         $container->set('request_stack', $requestStack);
-        $container->set('session_storage', $sessionStorage);
 
         $request = new Request();
         $listener = new SessionListener($container);
 
         $event = $this->createMock(RequestEvent::class);
-        $event->expects($this->exactly(2))->method('isMainRequest')->willReturn(true);
+        $event->expects($this->exactly(1))->method('isMainRequest')->willReturn(true);
         $event->expects($this->once())->method('getRequest')->willReturn($request);
 
         $listener->onKernelRequest($event);
@@ -89,7 +85,7 @@ public function testSessionUsesFactory()
         $listener = new SessionListener($container);
 
         $event = $this->createMock(RequestEvent::class);
-        $event->expects($this->exactly(2))->method('isMainRequest')->willReturn(true);
+        $event->expects($this->exactly(1))->method('isMainRequest')->willReturn(true);
         $event->expects($this->once())->method('getRequest')->willReturn($request);
 
         $listener->onKernelRequest($event);
@@ -101,10 +97,12 @@ public function testSessionUsesFactory()
     public function testResponseIsPrivateIfSessionStarted()
     {
         $session = $this->createMock(Session::class);
-        $session->expects($this->exactly(2))->method('getUsageIndex')->will($this->onConsecutiveCalls(0, 1));
+        $session->expects($this->once())->method('getUsageIndex')->willReturn(1);
+        $sessionFactory = $this->createMock(SessionFactory::class);
+        $sessionFactory->expects($this->once())->method('createSession')->willReturn($session);
 
         $container = new Container();
-        $container->set('initialized_session', $session);
+        $container->set('session_factory', $sessionFactory);
 
         $listener = new SessionListener($container);
         $kernel = $this->createMock(HttpKernelInterface::class);
@@ -113,7 +111,7 @@ public function testResponseIsPrivateIfSessionStarted()
         $listener->onKernelRequest(new RequestEvent($kernel, $request, HttpKernelInterface::MAIN_REQUEST));
 
         $response = new Response();
-        $listener->onKernelResponse(new ResponseEvent($kernel, new Request(), HttpKernelInterface::MAIN_REQUEST, $response));
+        $listener->onKernelResponse(new ResponseEvent($kernel, $request, HttpKernelInterface::MAIN_REQUEST, $response));
 
         $this->assertTrue($response->headers->has('Expires'));
         $this->assertTrue($response->headers->hasCacheControlDirective('private'));
@@ -126,10 +124,9 @@ public function testResponseIsPrivateIfSessionStarted()
     public function testResponseIsStillPublicIfSessionStartedAndHeaderPresent()
     {
         $session = $this->createMock(Session::class);
-        $session->expects($this->exactly(2))->method('getUsageIndex')->will($this->onConsecutiveCalls(0, 1));
+        $session->expects($this->once())->method('getUsageIndex')->willReturn(1);
 
         $container = new Container();
-        $container->set('initialized_session', $session);
 
         $listener = new SessionListener($container);
         $kernel = $this->createMock(HttpKernelInterface::class);
@@ -140,7 +137,10 @@ public function testResponseIsStillPublicIfSessionStartedAndHeaderPresent()
         $response = new Response();
         $response->setSharedMaxAge(60);
         $response->headers->set(AbstractSessionListener::NO_AUTO_CACHE_CONTROL_HEADER, 'true');
-        $listener->onKernelResponse(new ResponseEvent($kernel, new Request(), HttpKernelInterface::MAIN_REQUEST, $response));
+
+        $request = new Request();
+        $request->setSession($session);
+        $listener->onKernelResponse(new ResponseEvent($kernel, $request, HttpKernelInterface::MAIN_REQUEST, $response));
 
         $this->assertTrue($response->headers->hasCacheControlDirective('public'));
         $this->assertFalse($response->headers->has('Expires'));
@@ -157,9 +157,7 @@ public function testUninitializedSession()
         $response->setSharedMaxAge(60);
         $response->headers->set(AbstractSessionListener::NO_AUTO_CACHE_CONTROL_HEADER, 'true');
 
-        $container = new ServiceLocator([
-            'initialized_session' => function () {},
-        ]);
+        $container = new Container();
 
         $listener = new SessionListener($container);
         $listener->onKernelResponse(new ResponseEvent($kernel, new Request(), HttpKernelInterface::MAIN_REQUEST, $response));
@@ -174,11 +172,12 @@ public function testUninitializedSession()
     public function testSurrogateMainRequestIsPublic()
     {
         $session = $this->createMock(Session::class);
-        $session->expects($this->exactly(4))->method('getUsageIndex')->will($this->onConsecutiveCalls(0, 1, 1, 1));
+        $session->expects($this->exactly(2))->method('getUsageIndex')->will($this->onConsecutiveCalls(0, 1));
+        $sessionFactory = $this->createMock(SessionFactory::class);
+        $sessionFactory->expects($this->once())->method('createSession')->willReturn($session);
 
         $container = new Container();
-        $container->set('initialized_session', $session);
-        $container->set('session', $session);
+        $container->set('session_factory', $sessionFactory);
 
         $listener = new SessionListener($container);
         $kernel = $this->createMock(HttpKernelInterface::class);
@@ -193,7 +192,6 @@ public function testSurrogateMainRequestIsPublic()
         $this->assertSame($request->getSession(), $subRequest->getSession());
         $listener->onKernelRequest(new RequestEvent($kernel, $subRequest, HttpKernelInterface::MAIN_REQUEST));
         $listener->onKernelResponse(new ResponseEvent($kernel, $subRequest, HttpKernelInterface::MAIN_REQUEST, $response));
-        $listener->onFinishRequest(new FinishRequestEvent($kernel, $subRequest, HttpKernelInterface::MAIN_REQUEST));
 
         $this->assertFalse($response->headers->has('Expires'));
         $this->assertFalse($response->headers->hasCacheControlDirective('private'));
@@ -213,19 +211,15 @@ public function testSurrogateMainRequestIsPublic()
     public function testGetSessionIsCalledOnce()
     {
         $session = $this->createMock(Session::class);
-        $sessionStorage = $this->createMock(NativeSessionStorage::class);
+        $sessionFactory = $this->createMock(SessionFactory::class);
+        $sessionFactory->expects($this->once())->method('createSession')->willReturn($session);
         $kernel = $this->createMock(KernelInterface::class);
 
-        $sessionStorage->expects($this->once())
-            ->method('setOptions')
-            ->with(['cookie_secure' => true]);
-
         $requestStack = new RequestStack();
         $requestStack->push($mainRequest = new Request([], [], [], [], [], ['HTTPS' => 'on']));
 
         $container = new Container();
-        $container->set('session_storage', $sessionStorage);
-        $container->set('session', $session);
+        $container->set('session_factory', $sessionFactory);
         $container->set('request_stack', $requestStack);
 
         $event = new RequestEvent($kernel, $mainRequest, HttpKernelInterface::MAIN_REQUEST);
@@ -234,9 +228,6 @@ public function testGetSessionIsCalledOnce()
         $listener->onKernelRequest($event);
 
         // storage->setOptions() should have been called already
-        $container->set('session_storage', null);
-        $sessionStorage = null;
-
         $subRequest = $mainRequest->duplicate();
         // at this point both main and subrequest have a closure to build the session
 
@@ -249,10 +240,12 @@ public function testGetSessionIsCalledOnce()
     public function testSessionUsageExceptionIfStatelessAndSessionUsed()
     {
         $session = $this->createMock(Session::class);
-        $session->expects($this->exactly(2))->method('getUsageIndex')->will($this->onConsecutiveCalls(0, 1));
+        $session->expects($this->once())->method('getUsageIndex')->willReturn(1);
+        $sessionFactory = $this->createMock(SessionFactory::class);
+        $sessionFactory->expects($this->once())->method('createSession')->willReturn($session);
 
         $container = new Container();
-        $container->set('initialized_session', $session);
+        $container->set('session_factory', $sessionFactory);
 
         $listener = new SessionListener($container, true);
         $kernel = $this->createMock(HttpKernelInterface::class);
@@ -268,13 +261,15 @@ public function testSessionUsageExceptionIfStatelessAndSessionUsed()
     public function testSessionUsageLogIfStatelessAndSessionUsed()
     {
         $session = $this->createMock(Session::class);
-        $session->expects($this->exactly(2))->method('getUsageIndex')->will($this->onConsecutiveCalls(0, 1));
+        $session->expects($this->once())->method('getUsageIndex')->willReturn(1);
+        $sessionFactory = $this->createMock(SessionFactory::class);
+        $sessionFactory->expects($this->once())->method('createSession')->willReturn($session);
 
         $logger = $this->createMock(LoggerInterface::class);
         $logger->expects($this->exactly(1))->method('warning');
 
         $container = new Container();
-        $container->set('initialized_session', $session);
+        $container->set('session_factory', $sessionFactory);
         $container->set('logger', $logger);
 
         $listener = new SessionListener($container, false);
@@ -291,11 +286,13 @@ public function testSessionIsSavedWhenUnexpectedSessionExceptionThrown()
     {
         $session = $this->createMock(Session::class);
         $session->method('isStarted')->willReturn(true);
-        $session->expects($this->exactly(2))->method('getUsageIndex')->will($this->onConsecutiveCalls(0, 1));
+        $session->expects($this->once())->method('getUsageIndex')->willReturn(1);
         $session->expects($this->exactly(1))->method('save');
+        $sessionFactory = $this->createMock(SessionFactory::class);
+        $sessionFactory->expects($this->once())->method('createSession')->willReturn($session);
 
         $container = new Container();
-        $container->set('initialized_session', $session);
+        $container->set('session_factory', $sessionFactory);
 
         $listener = new SessionListener($container, true);
         $kernel = $this->createMock(HttpKernelInterface::class);
@@ -323,13 +320,13 @@ public function testSessionUsageCallbackWhenDebugAndStateless()
 
         $requestStack->push(new Request());
         $requestStack->push($request);
-        $requestStack->push(new Request());
+        $requestStack->push($subRequest = new Request());
+        $subRequest->setSession($session);
 
         $collector = $this->createMock(RequestDataCollector::class);
         $collector->expects($this->once())->method('collectSessionUsage');
 
         $container = new Container();
-        $container->set('initialized_session', $session);
         $container->set('request_stack', $requestStack);
         $container->set('session_collector', \Closure::fromCallable([$collector, 'collectSessionUsage']));
 
@@ -353,7 +350,6 @@ public function testSessionUsageCallbackWhenNoDebug()
         $collector->expects($this->never())->method('collectSessionUsage');
 
         $container = new Container();
-        $container->set('initialized_session', $session);
         $container->set('request_stack', $requestStack);
         $container->set('session_collector', $collector);
 
@@ -371,7 +367,6 @@ public function testSessionUsageCallbackWhenNoStateless()
         $requestStack->push(new Request());
 
         $container = new Container();
-        $container->set('initialized_session', $session);
         $container->set('request_stack', $requestStack);
 
         (new SessionListener($container, true))->onSessionUsage();