Merge pull request #287 from FriendsOfSymfony/1.3

1.3 to master

Author: dbu

CHANGELOG.md

@@ -1,6 +1,19 @@
 Changelog
 =========
 
+
+1.3.7
+-----
+
+* Add a sanity check on UserContextHash to avoid invalid content being cached
+  (example: anonymous cache set for authenticated user). This scenario occures
+  when the UserContextHash is cached by Varnish via 
+  `fos_http_cache.user_context.hash_cache_ttl` > 0 and the session is lost via 
+  garbage collector. The data given is the anonymous one despite having a hash 
+  for authenticated, all authenticated users will then have the anonymous version.
+  Same problem could occurs with users having is role changed or anything else
+  that can modify the hash.
+
 1.3.2
 -----
 

CacheManager.php

@@ -72,6 +72,8 @@ public function setGenerateUrlType($generateUrlType)
      */
     public function tagResponse(Response $response, array $tags, $replace = false)
     {
+        @trigger_error('The '.__METHOD__.' method is deprecated since version 1.2 and will be removed in 2.0. Use the TagHandler instead.', E_USER_DEPRECATED);
+
         if (!$replace && $response->headers->has($this->getTagsHeader())) {
             $header = $response->headers->get($this->getTagsHeader());
             if ('' !== $header) {

Command/InvalidateTagCommand.php

@@ -54,6 +54,10 @@ public function __construct($tagHandler = null, $commandName = 'fos:httpcache:in
                 )
             );
         }
+        if ($tagHandler instanceof CacheManager) {
+            @trigger_error('Passing the CacheManager to '.__CLASS__.' is deprecated since version 1.2 and will be removed in 2.0. Provide the TagHandler instead.', E_USER_DEPRECATED);
+
+        }
         $this->commandName = $commandName;
         $this->tagHandler = $tagHandler;
         parent::__construct();

EventListener/UserContextSubscriber.php

@@ -46,6 +46,11 @@ class UserContextSubscriber implements EventSubscriberInterface
      */
     private $userIdentifierHeaders;
 
+    /**
+     * @var string
+     */
+    private $hash;
+
     /**
      * @var string
      */
@@ -88,6 +93,10 @@ public function onKernelRequest(GetResponseEvent $event)
         }
 
         if (!$this->requestMatcher->matches($event->getRequest())) {
+            if ($event->getRequest()->headers->has($this->hashHeader)) {
+                $this->hash = $this->hashGenerator->generateHash();
+            }
+
             return;
         }
 
@@ -124,9 +133,19 @@ public function onKernelResponse(FilterResponseEvent $event)
         }
 
         $response = $event->getResponse();
+        $request = $event->getRequest();
+
         $vary = $response->getVary();
 
-        if ($event->getRequest()->headers->has($this->hashHeader)) {
+        if ($request->headers->has($this->hashHeader)) {
+            // hash has changed, session has most certainly changed, prevent setting incorrect cache
+            if (!is_null($this->hash) && $this->hash !== $request->headers->get($this->hashHeader)) {
+                $response->setClientTtl(0);
+                $response->headers->addCacheControlDirective('no-cache');
+
+                return;
+            }
+
             if (!in_array($this->hashHeader, $vary)) {
                 $vary[] = $this->hashHeader;
             }

HttpCache.php

@@ -20,6 +20,8 @@
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpKernel\HttpKernelInterface;
 
+@trigger_error('The '.__NAMESPACE__.'\HttpCache class is deprecated since version 1.2 and will be removed in 2.0. Use FOS\HttpCacheBundle\SymfonyCache\EventDispatchingHttpCache instead.', E_USER_DEPRECATED);
+
 /**
  * Base class for enhanced Symfony reverse proxy based on the symfony FrameworkBundle HttpCache.
  *

Resources/doc/features/testing.rst

@@ -72,7 +72,7 @@ configure the proxy server:
                     port: 8080
                     config_file: /etc/varnish/your-config.vcl
 
-.. seealso:: :doc:`test configuration </reference/configuration/test>`.
+.. seealso:: The complete reference for the configuration for testing is in the :doc:`test configuration </reference/configuration/test>` section.
 
 The custom ``@clearCache`` PHPUnit annotation will start the proxy server
 (if it was not yet running) and clear any previously cached content. This

Tests/Unit/CacheManagerTest.php

@@ -80,6 +80,9 @@ public function testRefreshRoute()
         ;
     }
 
+    /**
+     * @group legacy
+     */
     public function testTagResponse()
     {
         $ban = \Mockery::mock('\FOS\HttpCache\ProxyClient\Invalidation\BanInterface');

Tests/Unit/EventListener/UserContextSubscriberTest.php

@@ -104,6 +104,7 @@ public function testOnKernelRequestNotMatched()
 
         $requestMatcher = $this->getRequestMatcher($request, false);
         $hashGenerator = \Mockery::mock('\FOS\HttpCache\UserContext\HashGenerator');
+        $hashGenerator->shouldReceive('generateHash')->andReturn('hash');
 
         $userContextSubscriber = new UserContextSubscriber($requestMatcher, $hashGenerator, array('X-SessionId'), 'X-Hash');
         $event = $this->getKernelRequestEvent($request);
@@ -168,6 +169,67 @@ public function testOnKernelResponseNotCached()
         $this->assertEquals('X-SessionId', $event->getResponse()->headers->get('Vary'));
     }
 
+    /**
+     * If there is no hash in the request, vary on the user identifier.
+     */
+    public function testFullRequestHashOk()
+    {
+        $request = new Request();
+        $request->setMethod('GET');
+        $request->headers->set('X-Hash', 'hash');
+
+        $requestMatcher = $this->getRequestMatcher($request, false);
+        $hashGenerator = \Mockery::mock('\FOS\HttpCache\UserContext\HashGenerator');
+        $hashGenerator->shouldReceive('generateHash')->andReturn('hash');
+
+        // onKernelRequest
+        $userContextSubscriber = new UserContextSubscriber($requestMatcher, $hashGenerator, array('X-SessionId'), 'X-Hash');
+        $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
+     */
+    public function testFullRequestHashChanged()
+    {
+        $request = new Request();
+        $request->setMethod('GET');
+        $request->headers->set('X-Hash', 'hash');
+
+        $requestMatcher = $this->getRequestMatcher($request, false);
+        $hashGenerator = \Mockery::mock('\FOS\HttpCache\UserContext\HashGenerator');
+        $hashGenerator->shouldReceive('generateHash')->andReturn('hash-changed');
+
+        // onKernelRequest
+        $userContextSubscriber = new UserContextSubscriber($requestMatcher, $hashGenerator, array('X-SessionId'), 'X-Hash');
+        $event = $this->getKernelRequestEvent($request);
+
+        $userContextSubscriber->onKernelRequest($event);
+
+        $response = $event->getResponse();
+
+        $this->assertNull($response);
+
+        // onKernelResponse
+        $event = $this->getKernelResponseEvent($request);
+        $userContextSubscriber->onKernelResponse($event);
+
+        $this->assertFalse($event->getResponse()->headers->has('Vary'));
+        $this->assertEquals('max-age=0, no-cache, private', $event->getResponse()->headers->get('Cache-Control'));
+    }
+
     protected function getKernelRequestEvent(Request $request, $type = HttpKernelInterface::MASTER_REQUEST)
     {
         return new GetResponseEvent(

Tests/Unit/HttpCacheTest.php

@@ -16,6 +16,9 @@
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\HttpKernel\HttpKernelInterface;
 
+/**
+ * @group legacy
+ */
 class HttpCacheTest extends \PHPUnit_Framework_TestCase
 {
     /**