Fix that ESI/SSI processing can turn a \"private\" response \"public\"

Author: mpdude

src/Symfony/Component/HttpFoundation/Response.php

@@ -507,13 +507,19 @@ public function getCharset()
     }
 
     /**
-     * Returns true if the response is worth caching under any circumstance.
+     * Returns true if the response may safely be kept in a shared (surrogate) cache.
      *
      * Responses marked "private" with an explicit Cache-Control directive are
      * considered uncacheable.
      *
      * Responses with neither a freshness lifetime (Expires, max-age) nor cache
-     * validator (Last-Modified, ETag) are considered uncacheable.
+     * validator (Last-Modified, ETag) are considered uncacheable because there is
+     * no way to tell when or how to remove them from the cache.
+     *
+     * Note that RFC 7231 and RFC 7234 possibly allow for a more permissive implementation,
+     * for example "status codes that are defined as cacheable by default [...]
+     * can be reused by a cache with heuristic expiration unless otherwise indicated"
+     * (https://tools.ietf.org/html/rfc7231#section-6.1)
      *
      * @return bool true if the response is worth caching, false otherwise
      */

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

@@ -72,7 +72,7 @@ public function update(Response $response)
             $response->setLastModified(null);
         }
 
-        if (!$response->isFresh()) {
+        if (!$response->isFresh() || !$response->isCacheable()) {
             $this->cacheable = false;
         }
 

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

@@ -175,8 +175,26 @@ public function testEmbeddingPrivateResponseMakesMainResponsePrivate()
         $cacheStrategy->update($masterResponse);
 
         $this->assertTrue($masterResponse->headers->hasCacheControlDirective('private'));
-        // 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...?
+        $this->assertFalse($masterResponse->headers->hasCacheControlDirective('public'));
+    }
+
+    public function testEmbeddingPublicResponseDoesNotMakeMainResponsePublic()
+    {
+        $cacheStrategy = new ResponseCacheStrategy();
+
+        $masterResponse = new Response();
+        $masterResponse->setPrivate(); // this is the default, but let's be explicit
+        $masterResponse->setMaxAge(100);
+
+        $embeddedResponse = new Response();
+        $embeddedResponse->setPublic();
+        $embeddedResponse->setSharedMaxAge(100);
+
+        $cacheStrategy->add($embeddedResponse);
+        $cacheStrategy->update($masterResponse);
+
+        $this->assertTrue($masterResponse->headers->hasCacheControlDirective('private'));
+        $this->assertFalse($masterResponse->headers->hasCacheControlDirective('public'));
     }
 
     public function testResponseIsExiprableWhenEmbeddedResponseCombinesExpiryAndValidation()