[1.3] Backport Symfony session listener for Symfony 3.4+ to restore u…

…ser context functionality (#441)

* Decorate default Symfony session listener for Symfony 3.4+ to restore user context functionality

.travis.yml

@@ -50,6 +50,11 @@ matrix:
       env:
         - SYMFONY_VERSION='3.2.*'
         - FRAMEWORK_EXTRA_VERSION='~3.0'
+    - php: 5.6
+      env:
+        - SYMFONY_VERSION='3.4.*'
+        - PHPUNIT_FLAGS="--group sf34"
+        - FRAMEWORK_EXTRA_VERSION='~3.0'
     - php: hhvm
       dist: trusty
 

CHANGELOG.md

@@ -1,6 +1,12 @@
 Changelog
 =========
 
+1.3.14
+------
+
+* User context compatibility which was broken due to Symfony making responses
+  private if the session is started as of Symfony 3.4+.
+
 1.3.13
 ------
 

DependencyInjection/FOSHttpCacheExtension.php

@@ -19,6 +19,7 @@
 use Symfony\Component\DependencyInjection\Loader\XmlFileLoader;
 use Symfony\Component\DependencyInjection\Reference;
 use Symfony\Component\HttpKernel\DependencyInjection\Extension;
+use Symfony\Component\HttpKernel\Kernel;
 use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
 
 /**
@@ -224,6 +225,15 @@ private function loadUserContext(ContainerBuilder $container, XmlFileLoader $loa
                 ->addTag(HashGeneratorPass::TAG_NAME)
                 ->setAbstract(false);
         }
+
+        // Only decorate default session listener for Symfony 3.4+
+        if (version_compare(Kernel::VERSION, '3.4', '>=')) {
+            $container->getDefinition('fos_http_cache.user_context.session_listener')
+                ->setArgument(1, strtolower($config['user_hash_header']))
+                ->setArgument(2, array_map('strtolower', $config['user_identifier_headers']));
+        } else {
+            $container->removeDefinition('fos_http_cache.user_context.session_listener');
+        }
     }
 
     private function createRequestMatcher(ContainerBuilder $container, $path = null, $host = null, $methods = null, $ips = null, array $attributes = array())

EventListener/SessionListener.php

@@ -0,0 +1,85 @@
+<?php
+
+/*
+ * This file is part of the FOSHttpCacheBundle package.
+ *
+ * (c) FriendsOfSymfony <http://friendsofsymfony.github.com/>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace FOS\HttpCacheBundle\EventListener;
+
+use Symfony\Component\EventDispatcher\EventSubscriberInterface;
+use Symfony\Component\HttpKernel\Event\FilterResponseEvent;
+use Symfony\Component\HttpKernel\Event\GetResponseEvent;
+use Symfony\Component\HttpKernel\EventListener\SessionListener as BaseSessionListener;
+
+/**
+ * Decorates the default Symfony session listener.
+ *
+ * The default Symfony session listener automatically makes responses private
+ * in case the session was started. This kills the user context feature of
+ * FOSHttpCache. We disable the default behaviour only if the user context header
+ * is part of the Vary headers to reduce the possible impacts on other parts
+ * of your application.
+ *
+ * @author Yanick Witschi <yanick.witschi@terminal42.ch>
+ */
+final class SessionListener implements EventSubscriberInterface
+{
+    /**
+     * @var BaseSessionListener
+     */
+    private $inner;
+
+    /**
+     * @var string
+     */
+    private $userHashHeader;
+
+    /**
+     * @var array
+     */
+    private $userIdentifierHeaders;
+
+    /**
+     * @param BaseSessionListener $inner
+     * @param string              $userHashHeader        Must be lower-cased
+     * @param array               $userIdentifierHeaders Must be lower-cased
+     */
+    public function __construct(BaseSessionListener $inner, $userHashHeader, array $userIdentifierHeaders)
+    {
+        $this->inner = $inner;
+        $this->userHashHeader = $userHashHeader;
+        $this->userIdentifierHeaders = $userIdentifierHeaders;
+    }
+
+    public function onKernelRequest(GetResponseEvent $event)
+    {
+        return $this->inner->onKernelRequest($event);
+    }
+
+    public function onKernelResponse(FilterResponseEvent $event)
+    {
+        if (!$event->isMasterRequest()) {
+            return;
+        }
+
+        $varyHeaders = array_map('strtolower', $event->getResponse()->getVary());
+        $relevantHeaders = array_merge($this->userIdentifierHeaders, array($this->userHashHeader));
+
+        // Call default behaviour if it's an irrelevant request for the user context
+        if (0 === count(array_intersect($varyHeaders, $relevantHeaders))) {
+            $this->inner->onKernelResponse($event);
+        }
+
+        // noop, see class description
+    }
+
+    public static function getSubscribedEvents()
+    {
+        return BaseSessionListener::getSubscribedEvents();
+    }
+}

Resources/config/user_context.xml

@@ -41,6 +41,12 @@
             <argument />
         </service>
 
+        <service id="fos_http_cache.user_context.session_listener" class="FOS\HttpCacheBundle\EventListener\SessionListener" decorates="session_listener" public="false">
+            <argument type="service" id="fos_http_cache.user_context.session_listener.inner" />
+            <argument /> <!-- set by extension -->
+            <argument /> <!-- set by extension -->
+        </service>
+
         <service id="fos_http_cache.user_context.anonymous_request_matcher" class="FOS\HttpCache\UserContext\AnonymousRequestMatcher">
             <argument type="collection" />
         </service>

Tests/Resources/Fixtures/config/empty.yml

@@ -1 +1 @@
-fos_http_cache:
+fos_http_cache: []

Tests/Unit/DependencyInjection/FOSHttpCacheExtensionTest.php

@@ -16,6 +16,7 @@
 use Symfony\Component\DependencyInjection\Definition;
 use Symfony\Component\DependencyInjection\DefinitionDecorator;
 use Symfony\Component\DependencyInjection\ParameterBag\ParameterBag;
+use Symfony\Component\HttpKernel\Kernel;
 
 class FOSHttpCacheExtensionTest extends \PHPUnit_Framework_TestCase
 {
@@ -346,6 +347,37 @@ public function testConfigWithoutUserContext()
         $this->assertFalse($container->has('fos_http_cache.user_context.request_matcher'));
         $this->assertFalse($container->has('fos_http_cache.user_context.role_provider'));
         $this->assertFalse($container->has('fos_http_cache.user_context.logout_handler'));
+        $this->assertFalse($container->has('fos_http_cache.user_context.session_listener'));
+    }
+
+    /**
+     * @group sf34
+     */
+    public function testSessionListenerIsDecoratedIfNeeded()
+    {
+        $config = array(
+            array('user_context' => array(
+                'user_identifier_headers' => array('X-Foo'),
+                'user_hash_header' => 'X-Bar',
+                'hash_cache_ttl' => 30,
+                'role_provider' => true,
+            )),
+        );
+
+        $container = $this->createContainer();
+        $this->extension->load($config, $container);
+
+        // The whole definition should be removed for Symfony < 3.4
+        if (version_compare(Kernel::VERSION, '3.4', '<')) {
+            $this->assertFalse($container->hasDefinition('fos_http_cache.user_context.session_listener'));
+        } else {
+            $this->assertTrue($container->hasDefinition('fos_http_cache.user_context.session_listener'));
+
+            $definition = $container->getDefinition('fos_http_cache.user_context.session_listener');
+
+            $this->assertSame('x-bar', $definition->getArgument(1));
+            $this->assertSame(array('x-foo'), $definition->getArgument(2));
+        }
     }
 
     public function testConfigLoadFlashMessageSubscriber()

Tests/Unit/EventListener/SessionListenerTest.php

@@ -0,0 +1,102 @@
+<?php
+
+/*
+ * This file is part of the FOSHttpCacheBundle package.
+ *
+ * (c) FriendsOfSymfony <http://friendsofsymfony.github.com/>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace FOS\HttpCacheBundle\Tests\Unit\EventListener;
+
+use FOS\HttpCacheBundle\EventListener\SessionListener;
+use PHPUnit\Framework\TestCase;
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\Response;
+use Symfony\Component\HttpKernel\Event\FilterResponseEvent;
+use Symfony\Component\HttpKernel\EventListener\SessionListener as BaseSessionListener;
+use Symfony\Component\HttpKernel\HttpKernelInterface;
+use Symfony\Component\HttpKernel\Kernel;
+
+/**
+ * @group sf34
+ */
+class SessionListenerTest extends TestCase
+{
+    public function testOnKernelRequestRemainsUntouched()
+    {
+        $event = $this
+            ->getMockBuilder('Symfony\Component\HttpKernel\Event\GetResponseEvent')
+            ->disableOriginalConstructor()
+            ->getMock();
+
+        $inner = $this
+            ->getMockBuilder('Symfony\Component\HttpKernel\EventListener\SessionListener')
+            ->disableOriginalConstructor()
+            ->getMock();
+
+        $inner
+            ->expects($this->once())
+            ->method('onKernelRequest')
+            ->with($event)
+        ;
+
+        $listener = $this->getListener($inner);
+        $listener->onKernelRequest($event);
+    }
+
+    /**
+     * @dataProvider onKernelResponseProvider
+     */
+    public function testOnKernelResponse(Response $response, $shouldCallDecoratedListener)
+    {
+        if (version_compare(Kernel::VERSION, '3.4', '<')) {
+            $this->markTestSkipped('Irrelevant for Symfony < 3.4');
+        }
+
+        $httpKernel = $this
+            ->getMockBuilder('Symfony\Component\HttpKernel\HttpKernelInterface')
+            ->disableOriginalConstructor()
+            ->getMock();
+
+        $event = new FilterResponseEvent(
+            $httpKernel,
+            new Request(),
+            HttpKernelInterface::MASTER_REQUEST,
+            $response
+        );
+
+        $inner = $this
+            ->getMockBuilder('Symfony\Component\HttpKernel\EventListener\SessionListener')
+            ->disableOriginalConstructor()
+            ->getMock();
+
+        $inner
+            ->expects($shouldCallDecoratedListener ? $this->once() : $this->never())
+            ->method('onKernelResponse')
+            ->with($event)
+        ;
+
+        $listener = $this->getListener($inner);
+        $listener->onKernelResponse($event);
+    }
+
+    public function onKernelResponseProvider()
+    {
+        // Response, decorated listener should be called or not
+        return array(
+            'Irrelevant response' => array(new Response(), true),
+            'Irrelevant response header' => array(new Response('', 200, array('Content-Type' => 'Foobar')), true),
+            'Context hash header is present in Vary' => array(new Response('', 200, array('Vary' => 'X-User-Context-Hash')), false),
+            'User identifier header is present in Vary' => array(new Response('', 200, array('Vary' => 'cookie')), false),
+            'Both, context hash and identifier headers are present in Vary' => array(new Response('', 200, array('Vary' => 'Cookie, X-User-Context-Hash')), false),
+        );
+    }
+
+    private function getListener(BaseSessionListener $inner, $userHashHeader = 'x-user-context-hash', $userIdentifierHeaders = array('cookie', 'authorization'))
+    {
+        return new SessionListener($inner, $userHashHeader, $userIdentifierHeaders);
+    }
+}

composer.json

@@ -34,7 +34,8 @@
         "symfony/expression-language": "^2.4||^3.0",
         "symfony/monolog-bundle": "^2.3||^3.0",
         "polishsymfonycommunity/symfony-mocker-container": "^1.0",
-        "matthiasnoback/symfony-dependency-injection-test": "^0.7.4"
+        "matthiasnoback/symfony-dependency-injection-test": "^0.7.4",
+        "sebastian/exporter": "^1.2||^2.0||^3.0"
     },
     "suggest": {
         "sensio/framework-extra-bundle": "For Tagged Cache Invalidation",