Fix missing NoAutoCacheControl when vary on contex hash is set from Response (#485)

This moves logic to set NoAutoCacheControl so it is always executed if
vary header exists, and not only we we add it in the listener.

Author: andrerom

src/EventListener/UserContextListener.php

@@ -198,10 +198,12 @@ public function onKernelResponse(FilterResponseEvent $event)
                 && !in_array($this->options['user_hash_header'], $vary)
             ) {
                 $vary[] = $this->options['user_hash_header'];
-                if (4 <= Kernel::MAJOR_VERSION && 1 <= Kernel::MINOR_VERSION) {
-                    // header to avoid Symfony SessionListener overwriting the response to private
-                    $response->headers->set(AbstractSessionListener::NO_AUTO_CACHE_CONTROL_HEADER, 1);
-                }
+            }
+
+            // For Symfony 4.1+ if user hash header was in vary or just added here by "add_vary_on_hash"
+            if (4 <= Kernel::MAJOR_VERSION && 1 <= Kernel::MINOR_VERSION && in_array($this->options['user_hash_header'], $vary)) {
+                // header to avoid Symfony SessionListener overwriting the response to private
+                $response->headers->set(AbstractSessionListener::NO_AUTO_CACHE_CONTROL_HEADER, 1);
             }
         } elseif ($this->options['add_vary_on_hash']) {
             /*

tests/Unit/EventListener/UserContextListenerTest.php

@@ -20,7 +20,9 @@
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\HttpKernel\Event\FilterResponseEvent;
 use Symfony\Component\HttpKernel\Event\GetResponseEvent;
+use Symfony\Component\HttpKernel\EventListener\AbstractSessionListener;
 use Symfony\Component\HttpKernel\HttpKernelInterface;
+use Symfony\Component\HttpKernel\Kernel;
 
 class UserContextListenerTest extends TestCase
 {
@@ -182,6 +184,107 @@ public function testOnKernelResponse()
         $this->assertContains('X-Hash', $event->getResponse()->headers->get('Vary'));
     }
 
+    public function testOnKernelResponseSetsNoAutoCacheHeader()
+    {
+        if (4 > Kernel::MAJOR_VERSION || 1 > Kernel::MINOR_VERSION) {
+            $this->markTestSkipped('Test only relevant for Symfony 4.1 and up');
+        }
+
+        $request = new Request();
+        $request->setMethod('HEAD');
+        $request->headers->set('X-User-Context-Hash', 'hash');
+
+        $hashGenerator = \Mockery::mock(HashGenerator::class);
+
+        $userContextListener = new UserContextListener(
+            $this->getRequestMatcher($request, false),
+            $hashGenerator
+        );
+        $event = $this->getKernelResponseEvent($request);
+
+        $userContextListener->onKernelResponse($event);
+
+        $this->assertContains('X-User-Context-Hash', $event->getResponse()->headers->get('Vary'));
+        $this->assertEquals(1, $event->getResponse()->headers->get(AbstractSessionListener::NO_AUTO_CACHE_CONTROL_HEADER));
+    }
+
+    public function testOnKernelResponseSetsNoAutoCacheHeaderWhenCustomHeader()
+    {
+        if (4 > Kernel::MAJOR_VERSION || 1 > Kernel::MINOR_VERSION) {
+            $this->markTestSkipped('Test only relevant for Symfony 4.1 and up');
+        }
+
+        $request = new Request();
+        $request->setMethod('HEAD');
+        $request->headers->set('X-User-Context-Hash', 'hash');
+
+        $hashGenerator = \Mockery::mock(HashGenerator::class);
+
+        $userContextListener = new UserContextListener(
+            $this->getRequestMatcher($request, false),
+            $hashGenerator
+        );
+        $event = $this->getKernelResponseEvent($request, new Response('', 200, ['Vary' => 'X-User-Context-Hash']));
+
+        $userContextListener->onKernelResponse($event);
+
+        $this->assertEquals(1, $event->getResponse()->headers->get(AbstractSessionListener::NO_AUTO_CACHE_CONTROL_HEADER));
+    }
+
+    public function testOnKernelResponseSetsNoAutoCacheHeaderWhenCustomHeaderAndNoAddVaryOnHash()
+    {
+        if (4 > Kernel::MAJOR_VERSION || 1 > Kernel::MINOR_VERSION) {
+            $this->markTestSkipped('Test only relevant for Symfony 4.1 and up');
+        }
+
+        $request = new Request();
+        $request->setMethod('HEAD');
+        $request->headers->set('X-User-Context-Hash', 'hash');
+
+        $hashGenerator = \Mockery::mock(HashGenerator::class);
+
+        $userContextListener = new UserContextListener(
+            $this->getRequestMatcher($request, false),
+            $hashGenerator,
+            null,
+            [
+                'add_vary_on_hash' => false,
+            ]
+        );
+        $event = $this->getKernelResponseEvent($request, new Response('', 200, ['Vary' => 'X-User-Context-Hash']));
+
+        $userContextListener->onKernelResponse($event);
+
+        $this->assertEquals(1, $event->getResponse()->headers->get(AbstractSessionListener::NO_AUTO_CACHE_CONTROL_HEADER));
+    }
+
+    public function testOnKernelResponseDoesNotSetNoAutoCacheHeaderWhenNoCustomHeaderAndNoAddVaryOnHash()
+    {
+        if (4 > Kernel::MAJOR_VERSION || 1 > Kernel::MINOR_VERSION) {
+            $this->markTestSkipped('Test only relevant for Symfony 4.1 and up');
+        }
+
+        $request = new Request();
+        $request->setMethod('HEAD');
+        $request->headers->set('X-User-Context-Hash', 'hash');
+
+        $hashGenerator = \Mockery::mock(HashGenerator::class);
+
+        $userContextListener = new UserContextListener(
+            $this->getRequestMatcher($request, false),
+            $hashGenerator,
+            null,
+            [
+                'add_vary_on_hash' => false,
+            ]
+        );
+        $event = $this->getKernelResponseEvent($request);
+
+        $userContextListener->onKernelResponse($event);
+
+        $this->assertFalse($event->getResponse()->headers->has(AbstractSessionListener::NO_AUTO_CACHE_CONTROL_HEADER));
+    }
+
     public function testOnKernelResponseNotMaster()
     {
         $request = new Request();