From 09bcbc70e7c7cd9f57d2af10d2e8cd806b87ccc3 Mon Sep 17 00:00:00 2001
From: Matthias Pigulla <mp@webfactory.de>
Date: Sun, 11 Jun 2017 00:58:50 +0200
Subject: [PATCH] Embedding a response that combines expiration and validation,
 that should not defeat expiration on the combined response

---
 .../HttpCache/ResponseCacheStrategy.php       |  5 ++-
 .../HttpCache/ResponseCacheStrategyTest.php   | 41 +++++++++++++++++++
 2 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/src/Symfony/Component/HttpKernel/HttpCache/ResponseCacheStrategy.php b/src/Symfony/Component/HttpKernel/HttpCache/ResponseCacheStrategy.php
index 6e24016340ed..027b2b176133 100644
--- a/src/Symfony/Component/HttpKernel/HttpCache/ResponseCacheStrategy.php
+++ b/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;
         }
 
diff --git a/src/Symfony/Component/HttpKernel/Tests/HttpCache/ResponseCacheStrategyTest.php b/src/Symfony/Component/HttpKernel/Tests/HttpCache/ResponseCacheStrategyTest.php
index 4188bb37f243..5e4c322223eb 100644
--- a/src/Symfony/Component/HttpKernel/Tests/HttpCache/ResponseCacheStrategyTest.php
+++ b/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());
+    }
 }