feature #26681 Allow to easily ask Symfony not to set a response to private automatically (Toflar)

fabpot · fabpot · commit 0d35d0f90c9f · 2018-03-30T10:02:28.000+02:00
This PR was squashed before being merged into the 4.1-dev branch (closes #26681). Discussion ---------- Allow to easily ask Symfony not to set a response to private automatically | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | This PR is related to the many discussions going on about the latest (Symfony 3.4+) changes regarding the way Symfony handles the session. I think we're almost there now, Symfony 4.1 automatically turns responses into `private` responses once the session is started and it's all done in the `AbstractSessionListener` where the session is also saved. In other issues/PRs (symfony/symfony#25583, symfony/symfony#25699, symfony/symfony#24988) it was agreed that setting the response to `private` if the session is started is a good default for Symfony. It was also agreed that setting it to `private` does not always make sense because you **can share a response** across sessions, it just requires a more complex caching setup with shared user context etc. So there must be an easy way to disable this behaviour. Right now it's very hard to do so because what you end up doing is basically decorating the `session_listener` which is very hard because you have to keep track on that over different Symfony versions as the base listener might get additional features etc. The [FOSCacheBundle](FriendsOfSymfony/FOSHttpCacheBundle#438) is already having this problem, [Contao](contao/core-bundle#1388) has the same issue and there will be probably more. Basically everyone that wants to share a response cache across the session will have to decorate the default listener. That's just too hard, so I came up with this solution. The header is easy. Every project can add that easily. It does not require any extension, configuration or adjustment of any service. It's clean, transparent and has absolutely no impact on "default" Symfony setups. Would be happy to have some feedback before 4.1 freeze. Commits ------- 0f36710 Allow to easily ask Symfony not to set a response to private automatically
diff --git a/EventListener/AbstractSessionListener.php b/EventListener/AbstractSessionListener.php
@@ -20,13 +20,22 @@
 use Symfony\Component\EventDispatcher\EventSubscriberInterface;
 
 /**
- * Sets the session in the request.
+ * Sets the session onto the request on the "kernel.request" event and saves
+ * it on the "kernel.response" event.
+ *
+ * In addition, if the session has been started it overrides the Cache-Control
+ * header in such a way that all caching is disabled in that case.
+ * If you have a scenario where caching responses with session information in
+ * them makes sense, you can disable this behaviour by setting the header
+ * AbstractSessionListener::NO_AUTO_CACHE_CONTROL_HEADER on the response.
  *
  * @author Johannes M. Schmitt <schmittjoh@gmail.com>
  * @author Tobias Schultze <http://tobion.de>
  */
 abstract class AbstractSessionListener implements EventSubscriberInterface
 {
+    const NO_AUTO_CACHE_CONTROL_HEADER = 'Symfony-Session-NoAutoCacheControl';
+
     protected $container;
 
     public function __construct(ContainerInterface $container = null)
@@ -60,13 +69,20 @@ public function onKernelResponse(FilterResponseEvent $event)
             return;
         }
 
+        $response = $event->getResponse();
+
         if ($session->isStarted() || ($session instanceof Session && $session->hasBeenStarted())) {
-            $event->getResponse()
-                ->setPrivate()
-                ->setMaxAge(0)
-                ->headers->addCacheControlDirective('must-revalidate');
+            if (!$response->headers->has(self::NO_AUTO_CACHE_CONTROL_HEADER)) {
+                $response
+                    ->setPrivate()
+                    ->setMaxAge(0)
+                    ->headers->addCacheControlDirective('must-revalidate');
+            }
         }
 
+        // Always remove the internal header if present
+        $response->headers->remove(self::NO_AUTO_CACHE_CONTROL_HEADER);
+
         if ($session->isStarted()) {
             /*
              * Saves the session, in case it is still open, before sending the response/headers.
diff --git a/Tests/EventListener/SessionListenerTest.php b/Tests/EventListener/SessionListenerTest.php
@@ -56,7 +56,7 @@ public function testSessionIsSet()
         $this->assertSame($session, $request->getSession());
     }
 
-    public function testResponseIsPrivate()
+    public function testResponseIsPrivateIfSessionStarted()
     {
         $session = $this->getMockBuilder(Session::class)->disableOriginalConstructor()->getMock();
         $session->expects($this->exactly(2))->method('isStarted')->willReturn(false);
@@ -74,6 +74,31 @@ public function testResponseIsPrivate()
         $this->assertTrue($response->headers->hasCacheControlDirective('private'));
         $this->assertTrue($response->headers->hasCacheControlDirective('must-revalidate'));
         $this->assertSame('0', $response->headers->getCacheControlDirective('max-age'));
+        $this->assertFalse($response->headers->has(AbstractSessionListener::NO_AUTO_CACHE_CONTROL_HEADER));
+    }
+
+    public function testResponseIsStillPublicIfSessionStartedAndHeaderPresent()
+    {
+        $session = $this->getMockBuilder(Session::class)->disableOriginalConstructor()->getMock();
+        $session->expects($this->exactly(2))->method('isStarted')->willReturn(false);
+        $session->expects($this->once())->method('hasBeenStarted')->willReturn(true);
+
+        $container = new Container();
+        $container->set('initialized_session', $session);
+
+        $listener = new SessionListener($container);
+        $kernel = $this->getMockBuilder(HttpKernelInterface::class)->disableOriginalConstructor()->getMock();
+
+        $response = new Response();
+        $response->setSharedMaxAge(60);
+        $response->headers->set(AbstractSessionListener::NO_AUTO_CACHE_CONTROL_HEADER, 'true');
+        $listener->onKernelResponse(new FilterResponseEvent($kernel, new Request(), HttpKernelInterface::MASTER_REQUEST, $response));
+
+        $this->assertTrue($response->headers->hasCacheControlDirective('public'));
+        $this->assertFalse($response->headers->hasCacheControlDirective('private'));
+        $this->assertFalse($response->headers->hasCacheControlDirective('must-revalidate'));
+        $this->assertSame('60', $response->headers->getCacheControlDirective('s-maxage'));
+        $this->assertFalse($response->headers->has(AbstractSessionListener::NO_AUTO_CACHE_CONTROL_HEADER));
     }
 
     public function testUninitilizedSession()
