bug #34438 [HttpFoundation] Use Cache-Control: must-revalidate only…

… if explicit lifetime has been given (mpdude)

This PR was merged into the 3.4 branch.

Discussion
----------

[HttpFoundation] Use `Cache-Control: must-revalidate` only if explicit lifetime has been given

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       |
| License       | MIT
| Doc PR        |

This is really nit-picking: The conservative, safe default for `Cache-Control` is `private, no-cache` which means the response must not be served from cache unless it has been validated.

If `Last-Modified` or `Expires` are present, we can relax `no-cache` to be `must-revalidate`, which means that _once the response has become stale_, it must be revalidated.

An `ETag` alone does not give the response a lifetime, so IMO sticking with `no-cache` in this case would be more consistent.

Commits
-------

1b1002b [HttpFoundation] Use `Cache-Control: must-revalidate` only if explicit lifetime has been given

src/Symfony/Component/HttpFoundation/ResponseHeaderBag.php

@@ -309,13 +309,13 @@ public function makeDisposition($disposition, $filename, $filenameFallback = '')
      */
     protected function computeCacheControlValue()
     {
-        if (!$this->cacheControl && !$this->has('ETag') && !$this->has('Last-Modified') && !$this->has('Expires')) {
-            return 'no-cache, private';
-        }
-
         if (!$this->cacheControl) {
+            if ($this->has('Last-Modified') || $this->has('Expires')) {
+                return 'private, must-revalidate'; // allows for heuristic expiration (RFC 7234 Section 4.2.2) in the case of "Last-Modified"
+            }
+
             // conservative by default
-            return 'private, must-revalidate';
+            return 'no-cache, private';
         }
 
         $header = $this->getCacheControlHeader();

src/Symfony/Component/HttpFoundation/Tests/ResponseHeaderBagTest.php

@@ -51,9 +51,9 @@ public function testCacheControlHeader()
         $this->assertTrue($bag->hasCacheControlDirective('public'));
 
         $bag = new ResponseHeaderBag(['ETag' => 'abcde']);
-        $this->assertEquals('private, must-revalidate', $bag->get('Cache-Control'));
+        $this->assertEquals('no-cache, private', $bag->get('Cache-Control'));
         $this->assertTrue($bag->hasCacheControlDirective('private'));
-        $this->assertTrue($bag->hasCacheControlDirective('must-revalidate'));
+        $this->assertTrue($bag->hasCacheControlDirective('no-cache'));
         $this->assertFalse($bag->hasCacheControlDirective('max-age'));
 
         $bag = new ResponseHeaderBag(['Expires' => 'Wed, 16 Feb 2011 14:17:43 GMT']);

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

@@ -110,8 +110,6 @@ public function update(Response $response)
         $response->headers->set('Age', $this->age);
 
         if ($this->isNotCacheableResponseEmbedded) {
-            $response->setExpires($response->getDate());
-
             if ($this->flagDirectives['no-store']) {
                 $response->headers->set('Cache-Control', 'no-cache, no-store, must-revalidate');
             } else {

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

@@ -1242,7 +1242,6 @@ public function testEsiCacheForceValidation()
         $this->request('GET', '/', [], [], true);
         $this->assertEquals('Hello World! My name is Bobby.', $this->response->getContent());
         $this->assertNull($this->response->getTtl());
-        $this->assertTrue($this->response->mustRevalidate());
         $this->assertTrue($this->response->headers->hasCacheControlDirective('private'));
         $this->assertTrue($this->response->headers->hasCacheControlDirective('no-cache'));
     }
@@ -1273,7 +1272,6 @@ public function testEsiCacheForceValidationForHeadRequests()
         // This can neither be cached nor revalidated, so it should be private/no cache
         $this->assertEmpty($this->response->getContent());
         $this->assertNull($this->response->getTtl());
-        $this->assertTrue($this->response->mustRevalidate());
         $this->assertTrue($this->response->headers->hasCacheControlDirective('private'));
         $this->assertTrue($this->response->headers->hasCacheControlDirective('no-cache'));
     }