Merge pull request #288 from FriendsOfSymfony/optional-vary-user-hash

Added always_vary_on_context_hash option

Author: dbu

CHANGELOG.md

@@ -1,6 +1,12 @@
 Changelog
 =========
 
+2.0.0 (unreleased)
+------------------
+
+* [User Context] Added an option always_vary_on_context_hash to make it 
+  possible to disable automatically setting the vary headers for the user 
+  hash.
 
 1.3.7
 -----

DependencyInjection/Configuration.php

@@ -611,6 +611,10 @@ private function addUserContextListenerSection(ArrayNodeDefinition $rootNode)
                             ->defaultValue(0)
                             ->info('Cache the response for the hash for the specified number of seconds. Setting this to 0 will not cache those responses at all.')
                         ->end()
+                        ->booleanNode('always_vary_on_context_hash')
+                            ->defaultTrue()
+                            ->info('Whether to always add the user context hash header name in the response Vary header.')
+                        ->end()
                         ->arrayNode('user_identifier_headers')
                             ->prototype('scalar')->end()
                             ->defaultValue(array('Cookie', 'Authorization'))

DependencyInjection/FOSHttpCacheExtension.php

@@ -198,7 +198,8 @@ private function loadUserContext(ContainerBuilder $container, XmlFileLoader $loa
             ->replaceArgument(0, new Reference($config['match']['matcher_service']))
             ->replaceArgument(2, $config['user_identifier_headers'])
             ->replaceArgument(3, $config['user_hash_header'])
-            ->replaceArgument(4, $config['hash_cache_ttl']);
+            ->replaceArgument(4, $config['hash_cache_ttl'])
+            ->replaceArgument(5, $config['always_vary_on_context_hash']);
 
         if ($config['logout_handler']['enabled']) {
             $container->getDefinition($this->getAlias().'.user_context.logout_handler')

EventListener/UserContextSubscriber.php

@@ -61,12 +61,18 @@ class UserContextSubscriber implements EventSubscriberInterface
      */
     private $ttl;
 
+    /**
+     * @var bool Whether to automatically add the Vary header for the hash / user identifier if there was no hash.
+     */
+    private $addVaryOnHash;
+
     public function __construct(
         RequestMatcherInterface $requestMatcher,
         HashGenerator $hashGenerator,
         array $userIdentifierHeaders = array('Cookie', 'Authorization'),
         $hashHeader = "X-User-Context-Hash",
-        $ttl = 0
+        $ttl = 0,
+        $addVaryOnHash = true
     ) {
         if (!count($userIdentifierHeaders)) {
             throw new \InvalidArgumentException('The user context must vary on some request headers');
@@ -76,6 +82,7 @@ public function __construct(
         $this->userIdentifierHeaders = $userIdentifierHeaders;
         $this->hashHeader            = $hashHeader;
         $this->ttl                   = $ttl;
+        $this->addVaryOnHash         = $addVaryOnHash;
     }
 
     /**
@@ -146,10 +153,16 @@ public function onKernelResponse(FilterResponseEvent $event)
                 return;
             }
 
-            if (!in_array($this->hashHeader, $vary)) {
+            if ($this->addVaryOnHash && !in_array($this->hashHeader, $vary)) {
                 $vary[] = $this->hashHeader;
             }
-        } else {
+        } elseif ($this->addVaryOnHash) {
+            /*
+             * Additional precaution: If for some reason we get requests without a user hash, vary
+             * on user identifier headers to avoid the caching proxy mixing up caches between
+             * users. For the hash lookup request, those Vary headers are already added in
+             * onKernelRequest above.
+             */
             foreach ($this->userIdentifierHeaders as $header) {
                 if (!in_array($header, $vary)) {
                     $vary[] = $header;

Resources/config/user_context.xml

@@ -27,6 +27,7 @@
             <argument />
             <argument />
             <argument />
+            <argument />
             <tag name="kernel.event_subscriber" />
         </service>
 

Resources/doc/reference/configuration/user-context.rst

@@ -120,6 +120,21 @@ request. However, when you decide to cache hash responses, you must invalidate
 them when the user context changes, particularly when the user logs in or out.
 This bundle provides a logout handler that takes care of this for you.
 
+``always_vary_on_context_hash``
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+**type**: ``boolean`` **default**: ``true``
+
+This bundle automatically adds the Vary header for the user context hash, so
+you don't need to do this yourself or :doc:`configure it as header <headers>`.
+If the hash header is missing from a request for some reason, the response is
+set to vary on the user identifier headers to avoid problems.
+
+If not all your pages depend on the hash, you can set
+``always_vary_on_context_hash`` to  ``false`` and handle the Vary yourself.
+When doing that, you should be careful to set the Vary header whenever needed,
+or you will end up with mixed up caches.
+
 ``logout_handler``
 ~~~~~~~~~~~~~~~~~~
 

Tests/Resources/Fixtures/config/full.php

@@ -99,6 +99,7 @@
             'method' => 'GET',
         ),
         'hash_cache_ttl' => 300,
+        'always_vary_on_context_hash' => true,
         'user_identifier_headers' => array('Cookie', 'Authorization'),
         'user_hash_header' => 'FOS-User-Context-Hash',
         'role_provider' => true,

Tests/Resources/Fixtures/config/full.yml

@@ -93,6 +93,7 @@ fos_http_cache:
         match:
             method: GET
         hash_cache_ttl: 300
+        always_vary_on_context_hash: true
         role_provider: true
         user_identifier_headers:
             - Cookie

Tests/Unit/DependencyInjection/ConfigurationTest.php

@@ -153,6 +153,7 @@ public function testSupportsAllConfigFormats()
                     'method' => 'GET',
                 ),
                 'hash_cache_ttl' => 300,
+                'always_vary_on_context_hash' => true,
                 'user_identifier_headers' => array('Cookie', 'Authorization'),
                 'user_hash_header' => 'FOS-User-Context-Hash',
                 'role_provider' => true,
@@ -460,6 +461,7 @@ private function getEmptyConfig()
                     'method' => null,
                 ),
                 'hash_cache_ttl' => 0,
+                'always_vary_on_context_hash' => true,
                 'user_identifier_headers' => array('Cookie', 'Authorization'),
                 'user_hash_header' => 'X-User-Context-Hash',
                 'role_provider' => false,

Tests/Unit/DependencyInjection/FOSHttpCacheExtensionTest.php

@@ -286,6 +286,7 @@ public function testConfigUserContext()
                 'user_identifier_headers' => array('X-Foo'),
                 'user_hash_header' => 'X-Bar',
                 'hash_cache_ttl' => 30,
+                'always_vary_on_context_hash' => true,
                 'role_provider' => true,
             ),
         );
@@ -315,6 +316,7 @@ public function testConfigWithoutUserContext()
                 'user_identifier_headers' => array('X-Foo'),
                 'user_hash_header' => 'X-Bar',
                 'hash_cache_ttl' => 30,
+                'always_vary_on_context_hash' => true,
                 'role_provider' => true,
             )),
         );

Tests/Unit/EventListener/UserContextSubscriberTest.php

@@ -130,6 +130,7 @@ public function testOnKernelResponse()
 
         $userContextSubscriber->onKernelResponse($event);
 
+        $this->assertTrue($event->getResponse()->headers->has('Vary'), 'Vary header must be set');
         $this->assertContains('X-Hash', $event->getResponse()->headers->get('Vary'));
     }