bug #23130 Keep s-maxage when expiry and validation are used in combi…

…nation (mpdude)

This PR was merged into the 2.7 branch.

Discussion
----------

Keep s-maxage when expiry and validation are used in combination

| Q             | A
| ------------- | ---
| Branch?       | 2.7
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets |
| License       | MIT
| Doc PR        |

(Symfony) docs say that [expiration wins over validation](https://symfony.com/doc/current/http_cache/validation.html). So,

a) when both the master and embedded response are public with an s-maxage, the result should be public as well and use the lower s-maxage of both, *also* in the case that the embedded response carries validation headers. (The cache may use those for revalidating the embedded response once it has become stale, but that does not impact expiration-based caching of the combined response.)

b) when both the master and embedded response are public with an s-maxage, the result should be public as well and use the lower s-maxage of both, *also* in the case that the master response carries validation headers. However, those *must not* be passed on to the client: They do not apply to the combined response, but may only be used by the cache itself to revalidate the (raw) master response.

Commits
-------

09bcbc7 Embedding a response that combines expiration and validation, that should not defeat expiration on the combined response

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

@@ -39,7 +39,7 @@ class ResponseCacheStrategy implements ResponseCacheStrategyInterface
      */
     public function add(Response $response)
     {
-        if ($response->isValidateable() || !$response->isCacheable()) {
+        if (!$response->isFresh() || !$response->isCacheable()) {
             $this->cacheable = false;
         } else {
             $maxAge = $response->getMaxAge();
@@ -70,6 +70,9 @@ public function update(Response $response)
         if ($response->isValidateable()) {
             $response->setEtag(null);
             $response->setLastModified(null);
+        }
+
+        if (!$response->isFresh()) {
             $this->cacheable = false;
         }
 

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

@@ -178,4 +178,45 @@ public function testEmbeddingPrivateResponseMakesMainResponsePrivate()
         // Not sure if we should pass "max-age: 60" in this case, as long as the response is private and
         // that's the more conservative of both the master and embedded response...?
     }
+
+    public function testResponseIsExiprableWhenEmbeddedResponseCombinesExpiryAndValidation()
+    {
+        /* When "expiration wins over validation" (https://symfony.com/doc/current/http_cache/validation.html)
+         * and both the main and embedded response provide s-maxage, then the more restricting value of both
+         * should be fine, regardless of whether the embedded response can be validated later on or must be
+         * completely regenerated.
+         */
+        $cacheStrategy = new ResponseCacheStrategy();
+
+        $masterResponse = new Response();
+        $masterResponse->setSharedMaxAge(3600);
+
+        $embeddedResponse = new Response();
+        $embeddedResponse->setSharedMaxAge(60);
+        $embeddedResponse->setEtag('foo');
+
+        $cacheStrategy->add($embeddedResponse);
+        $cacheStrategy->update($masterResponse);
+
+        $this->assertSame('60', $masterResponse->headers->getCacheControlDirective('s-maxage'));
+    }
+
+    public function testResponseIsExpirableButNotValidateableWhenMasterResponseCombinesExpirationAndValidation()
+    {
+        $cacheStrategy = new ResponseCacheStrategy();
+
+        $masterResponse = new Response();
+        $masterResponse->setSharedMaxAge(3600);
+        $masterResponse->setEtag('foo');
+        $masterResponse->setLastModified(new \DateTime());
+
+        $embeddedResponse = new Response();
+        $embeddedResponse->setSharedMaxAge(60);
+
+        $cacheStrategy->add($embeddedResponse);
+        $cacheStrategy->update($masterResponse);
+
+        $this->assertSame('60', $masterResponse->headers->getCacheControlDirective('s-maxage'));
+        $this->assertFalse($masterResponse->isValidateable());
+    }
 }