From 5e9a1b25443b400025f8ffaf20ee245c0d726dc5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edi=20Modri=C4=87?= Date: Sat, 14 Apr 2018 10:55:01 +0200 Subject: [PATCH] [1.3] Backport Symfony session listener for Symfony 3.4+ to restore user context functionality (#441) * Decorate default Symfony session listener for Symfony 3.4+ to restore user context functionality --- .travis.yml | 5 + CHANGELOG.md | 6 ++ DependencyInjection/FOSHttpCacheExtension.php | 10 ++ EventListener/SessionListener.php | 85 +++++++++++++++ Resources/config/user_context.xml | 6 ++ Tests/Resources/Fixtures/config/empty.yml | 2 +- .../FOSHttpCacheExtensionTest.php | 32 ++++++ .../EventListener/SessionListenerTest.php | 102 ++++++++++++++++++ composer.json | 3 +- 9 files changed, 249 insertions(+), 2 deletions(-) create mode 100644 EventListener/SessionListener.php create mode 100755 Tests/Unit/EventListener/SessionListenerTest.php diff --git a/.travis.yml b/.travis.yml index 44319d07..74272eda 100644 --- a/.travis.yml +++ b/.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 diff --git a/CHANGELOG.md b/CHANGELOG.md index 2cf648e9..808ffe58 100644 --- a/CHANGELOG.md +++ b/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 ------ diff --git a/DependencyInjection/FOSHttpCacheExtension.php b/DependencyInjection/FOSHttpCacheExtension.php index 8a0caed0..0e915d3d 100644 --- a/DependencyInjection/FOSHttpCacheExtension.php +++ b/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()) diff --git a/EventListener/SessionListener.php b/EventListener/SessionListener.php new file mode 100644 index 00000000..4ac63f5c --- /dev/null +++ b/EventListener/SessionListener.php @@ -0,0 +1,85 @@ + + * + * 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 + */ +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(); + } +} diff --git a/Resources/config/user_context.xml b/Resources/config/user_context.xml index 37166c76..fee98831 100644 --- a/Resources/config/user_context.xml +++ b/Resources/config/user_context.xml @@ -41,6 +41,12 @@ + + + + + + diff --git a/Tests/Resources/Fixtures/config/empty.yml b/Tests/Resources/Fixtures/config/empty.yml index 315647a0..87a48a61 100644 --- a/Tests/Resources/Fixtures/config/empty.yml +++ b/Tests/Resources/Fixtures/config/empty.yml @@ -1 +1 @@ -fos_http_cache: +fos_http_cache: [] diff --git a/Tests/Unit/DependencyInjection/FOSHttpCacheExtensionTest.php b/Tests/Unit/DependencyInjection/FOSHttpCacheExtensionTest.php index 386d304d..22a824b0 100644 --- a/Tests/Unit/DependencyInjection/FOSHttpCacheExtensionTest.php +++ b/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() diff --git a/Tests/Unit/EventListener/SessionListenerTest.php b/Tests/Unit/EventListener/SessionListenerTest.php new file mode 100755 index 00000000..d09166a0 --- /dev/null +++ b/Tests/Unit/EventListener/SessionListenerTest.php @@ -0,0 +1,102 @@ + + * + * 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); + } +} diff --git a/composer.json b/composer.json index 6458e4eb..cf5e973a 100644 --- a/composer.json +++ b/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",