diff --git a/src/Symfony/Component/HttpFoundation/Response.php b/src/Symfony/Component/HttpFoundation/Response.php index a7459224c2a9..ad551a85b9a8 100644 --- a/src/Symfony/Component/HttpFoundation/Response.php +++ b/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 */ diff --git a/src/Symfony/Component/HttpKernel/HttpCache/ResponseCacheStrategy.php b/src/Symfony/Component/HttpKernel/HttpCache/ResponseCacheStrategy.php index 027b2b176133..672cc893feb6 100644 --- a/src/Symfony/Component/HttpKernel/HttpCache/ResponseCacheStrategy.php +++ b/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; } diff --git a/src/Symfony/Component/HttpKernel/Tests/HttpCache/ResponseCacheStrategyTest.php b/src/Symfony/Component/HttpKernel/Tests/HttpCache/ResponseCacheStrategyTest.php index 5e4c322223eb..6d67a177398c 100644 --- a/src/Symfony/Component/HttpKernel/Tests/HttpCache/ResponseCacheStrategyTest.php +++ b/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()