merged branch bamarni/issue-6977 (PR #7857)

This PR was submitted for the master branch but it was merged into the 2.1 branch instead (closes #7857).

Discussion
----------

[HttpCache] remove validation related headers when needed

| Q             | A
| ------------- | ---
| Bug fix?      | [yes]
| New feature?  | [no]
| BC breaks?    | [yes]
| Deprecations? | [no]
| Tests pass?   | [yes]
| Fixed tickets | [#6977]
| License       | MIT

Fixes #6977 by removing validation related headers when there is at least one embedded response.

I've added an embedded response counter because the current check was wrong I think, it was checking count($this->ttls) which isn't updated for validateable responses.

And for the BC break, looking at the interface PHPDoc description, it supposes add() method should only be applied on esi responses and update() takes the master one at the end, what do you think?

Commits
-------

bb80139 [HttpCache] remove validation related headers when needed

src/Symfony/Component/HttpKernel/HttpCache/EsiResponseCacheStrategy.php

@@ -29,13 +29,12 @@
 class EsiResponseCacheStrategy implements EsiResponseCacheStrategyInterface
 {
     private $cacheable = true;
+    private $embeddedResponses = 0;
     private $ttls = array();
     private $maxAges = array();
 
     /**
-     * Adds a Response.
-     *
-     * @param Response $response
+     * {@inheritdoc}
      */
     public function add(Response $response)
     {
@@ -45,26 +44,38 @@ public function add(Response $response)
             $this->ttls[] = $response->getTtl();
             $this->maxAges[] = $response->getMaxAge();
         }
+
+        $this->embeddedResponses++;
     }
 
     /**
-     * Updates the Response HTTP headers based on the embedded Responses.
-     *
-     * @param Response $response
+     * {@inheritdoc}
      */
     public function update(Response $response)
     {
-        // if we only have one Response, do nothing
-        if (1 === count($this->ttls)) {
+        // if we have no embedded Response, do nothing
+        if (0 === $this->embeddedResponses) {
             return;
         }
 
+        // Remove validation related headers in order to avoid browsers using
+        // their own cache, because some of the response content comes from
+        // at least one embedded response (which likely has a different caching strategy).
+        if ($response->isValidateable()) {
+            $response->setEtag(null);
+            $response->setLastModified(null);
+            $this->cacheable = false;
+        }
+
         if (!$this->cacheable) {
             $response->headers->set('Cache-Control', 'no-cache, must-revalidate');
 
             return;
         }
 
+        $this->ttls[] = $response->getTtl();
+        $this->maxAges[] = $response->getMaxAge();
+
         if (null !== $maxAge = min($this->maxAges)) {
             $response->setSharedMaxAge($maxAge);
             $response->headers->set('Age', $maxAge - min($this->ttls));

src/Symfony/Component/HttpKernel/HttpCache/HttpCache.php

@@ -204,10 +204,10 @@ public function handle(Request $request, $type = HttpKernelInterface::MASTER_REQ
         }
 
         if (null !== $this->esi) {
-            $this->esiCacheStrategy->add($response);
-
             if (HttpKernelInterface::MASTER_REQUEST === $type) {
                 $this->esiCacheStrategy->update($response);
+            } else {
+                $this->esiCacheStrategy->add($response);
             }
         }
 

src/Symfony/Component/HttpKernel/Tests/HttpCache/HttpCacheTest.php

@@ -1075,4 +1075,32 @@ public function testXForwarderForHeaderForPassRequests()
 
         $this->assertEquals('10.0.0.1', $this->kernel->getBackendRequest()->headers->get('X-Forwarded-For'));
     }
+
+    public function testEsiCacheRemoveValidationHeadersIfEmbeddedResponses()
+    {
+        $time = new \DateTime;
+
+        $responses = array(
+            array(
+                'status'  => 200,
+                'body'    => '<esi:include src="/hey" />',
+                'headers' => array(
+                    'Surrogate-Control' => 'content="ESI/1.0"',
+                    'ETag' => 'hey',
+                    'Last-Modified' => $time->format(DATE_RFC2822),
+                ),
+            ),
+            array(
+                'status'  => 200,
+                'body'    => 'Hey!',
+                'headers' => array(),
+            ),
+        );
+
+        $this->setNextResponses($responses);
+
+        $this->request('GET', '/', array(), array(), true);
+        $this->assertNull($this->response->getETag());
+        $this->assertNull($this->response->getLastModified());
+    }
 }