Merge branch '1.3'

Conflicts:
	EventListener/UserContextSubscriber.php
	Resources/config/user_context.xml
	composer.json

Author: dbu

DependencyInjection/FOSHttpCacheExtension.php

@@ -201,6 +201,9 @@ private function loadUserContext(ContainerBuilder $container, XmlFileLoader $loa
             ->replaceArgument(4, $config['hash_cache_ttl'])
             ->replaceArgument(5, $config['always_vary_on_context_hash']);
 
+        $container->getDefinition($this->getAlias().'.user_context.anonymous_request_matcher')
+            ->replaceArgument(0, $config['user_identifier_headers']);
+
         if ($config['logout_handler']['enabled']) {
             $container->getDefinition($this->getAlias().'.user_context.logout_handler')
                 ->replaceArgument(1, $config['user_identifier_headers'])

EventListener/UserContextSubscriber.php

@@ -13,6 +13,7 @@
 
 use FOS\HttpCache\UserContext\HashGenerator;
 use Symfony\Component\EventDispatcher\EventSubscriberInterface;
+use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\RequestMatcherInterface;
 use Symfony\Component\HttpKernel\Event\FilterResponseEvent;
 use Symfony\Component\HttpKernel\Event\GetResponseEvent;
@@ -66,13 +67,23 @@ class UserContextSubscriber implements EventSubscriberInterface
      */
     private $addVaryOnHash;
 
+    /**
+     * Used to exclude anonymous requests (no authentication nor session) from user hash sanity check.
+     * It prevents issues when the hash generator that is used returns a customized value for anonymous users,
+     * that differs from the documented, hardcoded one.
+     *
+     * @var RequestMatcherInterface
+     */
+    private $anonymousRequestMatcher;
+
     public function __construct(
         RequestMatcherInterface $requestMatcher,
         HashGenerator $hashGenerator,
         array $userIdentifierHeaders = array('Cookie', 'Authorization'),
         $hashHeader = 'X-User-Context-Hash',
         $ttl = 0,
-        $addVaryOnHash = true
+        $addVaryOnHash = true,
+        RequestMatcherInterface $anonymousRequestMatcher = null
     ) {
         if (!count($userIdentifierHeaders)) {
             throw new \InvalidArgumentException('The user context must vary on some request headers');
@@ -83,6 +94,7 @@ public function __construct(
         $this->hashHeader = $hashHeader;
         $this->ttl = $ttl;
         $this->addVaryOnHash = $addVaryOnHash;
+        $this->anonymousRequestMatcher = $anonymousRequestMatcher;
     }
 
     /**
@@ -100,7 +112,7 @@ public function onKernelRequest(GetResponseEvent $event)
         }
 
         if (!$this->requestMatcher->matches($event->getRequest())) {
-            if ($event->getRequest()->headers->has($this->hashHeader)) {
+            if ($event->getRequest()->headers->has($this->hashHeader) && !$this->isAnonymous($event->getRequest())) {
                 $this->hash = $this->hashGenerator->generateHash();
             }
 
@@ -127,6 +139,20 @@ public function onKernelRequest(GetResponseEvent $event)
         $event->setResponse($response);
     }
 
+    /**
+     * Tests if $request is an anonymous request or not.
+     *
+     * For backward compatibility reasons, true will be returned if no anonymous request matcher was provided.
+     *
+     * @param Request $request
+     *
+     * @return bool
+     */
+    private function isAnonymous(Request $request)
+    {
+        return $this->anonymousRequestMatcher ? $this->anonymousRequestMatcher->matches($request) : false;
+    }
+
     /**
      * Add the context hash header to the headers to vary on if the header was
      * present in the request.

Resources/config/user_context.xml

@@ -28,6 +28,7 @@
             <argument />
             <argument />
             <argument />
+            <argument type="service" id="fos_http_cache.user_context.anonymous_request_matcher" />
             <tag name="kernel.event_subscriber" />
         </service>
 
@@ -40,5 +41,9 @@
             <argument />
             <argument />
         </service>
+
+        <service id="fos_http_cache.user_context.anonymous_request_matcher" class="FOS\HttpCacheBundle\UserContext\AnonymousRequestMatcher">
+            <argument type="collection" />
+        </service>
     </services>
 </container>

Tests/Unit/EventListener/UserContextSubscriberTest.php

@@ -199,6 +199,39 @@ public function testFullRequestHashOk()
         $this->assertContains('X-Hash', $event->getResponse()->headers->get('Vary'));
     }
 
+    /**
+     * If the request is an anonymous one, no hash should be generated/validated.
+     */
+    public function testFullAnonymousRequestHashNotGenerated()
+    {
+        $request = new Request();
+        $request->setMethod('GET');
+        $request->headers->set('X-Hash', 'anonymous-hash');
+
+        $requestMatcher = $this->getRequestMatcher($request, false);
+        $hashGenerator = \Mockery::mock('\FOS\HttpCache\UserContext\HashGenerator');
+        $hashGenerator->shouldReceive('generateHash')->never();
+
+        $anonymousRequestMatcher = \Mockery::mock('\Symfony\Component\HttpFoundation\RequestMatcherInterface');
+        $anonymousRequestMatcher->shouldReceive('matches')->andReturn(true);
+
+        // onKernelRequest
+        $userContextSubscriber = new UserContextSubscriber($requestMatcher, $hashGenerator, array('X-SessionId'), 'X-Hash', 0, true, $anonymousRequestMatcher);
+        $event = $this->getKernelRequestEvent($request);
+
+        $userContextSubscriber->onKernelRequest($event);
+
+        $response = $event->getResponse();
+
+        $this->assertNull($response);
+
+        // onKernelResponse
+        $event = $this->getKernelResponseEvent($request);
+        $userContextSubscriber->onKernelResponse($event);
+
+        $this->assertContains('X-Hash', $event->getResponse()->headers->get('Vary'));
+    }
+
     /**
      * If there is no hash in the requests but session changed, prevent setting bad cache.
      */

Tests/Unit/UserContext/AnonymousRequestMatcherTest.php

@@ -0,0 +1,69 @@
+<?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\UserContext;
+
+use FOS\HttpCacheBundle\UserContext\AnonymousRequestMatcher;
+use PHPUnit_Framework_TestCase;
+use Symfony\Component\HttpFoundation\Request;
+
+class AnonymousRequestMatcherTest extends PHPUnit_Framework_TestCase
+{
+    public function testMatchAnonymousRequest()
+    {
+        $request = new Request();
+
+        $requestMatcher = new AnonymousRequestMatcher(['Cookie', 'Authorization']);
+
+        $this->assertTrue($requestMatcher->matches($request));
+    }
+
+    public function testNoMatchIfCookie()
+    {
+        $request = new Request();
+        $request->headers->set('Cookie', 'PHPSESSID7e476fc9f29f69d2ad6f11dbcd663b42=25f6d9c5a843e3c948cd26902385a527');
+        $request->cookies->set('PHPSESSID7e476fc9f29f69d2ad6f11dbcd663b42', '25f6d9c5a843e3c948cd26902385a527');
+
+        $requestMatcher = new AnonymousRequestMatcher(['Cookie', 'Authorization']);
+
+        $this->assertFalse($requestMatcher->matches($request));
+    }
+
+    public function testNoMatchIfEmptyCookieHeader()
+    {
+        $request = new Request();
+        $request->headers->set('Cookie', '');
+
+        $requestMatcher = new AnonymousRequestMatcher(['Cookie', 'Authorization']);
+
+        $this->assertTrue($requestMatcher->matches($request));
+    }
+
+    public function testNoMatchIfAuthenticationHeader()
+    {
+        $request = new Request();
+        $request->headers->set('Authorization', 'foo: bar');
+
+        $requestMatcher = new AnonymousRequestMatcher(['Cookie', 'Authorization']);
+
+        $this->assertFalse($requestMatcher->matches($request));
+    }
+
+    public function testMatchEmptyCookieHeaderHeader()
+    {
+        $request = new Request();
+        $request->headers->set('Cookie', '');
+
+        $requestMatcher = new AnonymousRequestMatcher(['Cookie', 'Authorization']);
+
+        $this->assertTrue($requestMatcher->matches($request));
+    }
+}

UserContext/AnonymousRequestMatcher.php

@@ -0,0 +1,47 @@
+<?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\UserContext;
+
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\RequestMatcherInterface;
+
+/**
+ * Matches anonymous requests using a list of identification headers.
+ */
+class AnonymousRequestMatcher implements RequestMatcherInterface
+{
+    private $userIdentifierHeaders;
+
+    /**
+     * @param array $userIdentifierHeaders List of request headers that authenticate a non-anonymous request
+     */
+    public function __construct(array $userIdentifierHeaders)
+    {
+        $this->userIdentifierHeaders = $userIdentifierHeaders;
+    }
+
+    public function matches(Request $request)
+    {
+        foreach ($this->userIdentifierHeaders as $header) {
+            if ($request->headers->has($header)) {
+                if (strtolower($header) === 'cookie' && 0 === $request->cookies->count()) {
+                    // ignore empty cookie header
+                    continue;
+                }
+
+                return false;
+            }
+        }
+
+        return true;
+    }
+}

composer.json

@@ -30,7 +30,7 @@
         "mockery/mockery": "0.9.*",
         "monolog/monolog": "*",
         "sensio/framework-extra-bundle": "^3.0",
-        "symfony/symfony": "^2.3||^3.0",
+        "symfony/symfony": "^2.8||^3.0",
         "symfony/phpunit-bridge": "^2.7||^3.0",
         "symfony/expression-language": "^2.4||^3.0",
         "symfony/monolog-bundle": "^2.3||^3.0",