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 @@ use Symfony\Component\HttpFoundation\Response;
 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 @@ class EsiResponseCacheStrategy implements EsiResponseCacheStrategyInterface
             $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 @@ class HttpCache implements HttpKernelInterface, TerminableInterface
         }
 
         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 @@ class HttpCacheTest extends HttpCacheTestCase
 
         $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());
+    }
 }