Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit c6e8c07

Browse filesBrowse files
mpdudefabpot
authored andcommitted
Fix two edge cases in ResponseCacheStrategy
1 parent 53a9111 commit c6e8c07
Copy full SHA for c6e8c07

File tree

2 files changed

+42
-1
lines changed
Filter options

2 files changed

+42
-1
lines changed

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/HttpKernel/HttpCache/ResponseCacheStrategy.php
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ class ResponseCacheStrategy implements ResponseCacheStrategyInterface
3939
*/
4040
public function add(Response $response)
4141
{
42-
if ($response->isValidateable()) {
42+
if ($response->isValidateable() || !$response->isCacheable()) {
4343
$this->cacheable = false;
4444
} else {
4545
$maxAge = $response->getMaxAge();

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/HttpKernel/Tests/HttpCache/ResponseCacheStrategyTest.php
+41Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,4 +137,45 @@ public function testMasterResponseWithExpirationIsUnchangedWhenThereIsNoEmbedded
137137

138138
$this->assertTrue($masterResponse->isFresh());
139139
}
140+
141+
public function testMasterResponseIsNotCacheableWhenEmbeddedResponseIsNotCacheable()
142+
{
143+
$cacheStrategy = new ResponseCacheStrategy();
144+
145+
$masterResponse = new Response();
146+
$masterResponse->setSharedMaxAge(3600); // Public, cacheable
147+
148+
/* This response has no validation or expiration information.
149+
That makes it uncacheable, it is always stale.
150+
(It does *not* make this private, though.) */
151+
$embeddedResponse = new Response();
152+
$this->assertFalse($embeddedResponse->isFresh()); // not fresh, as no lifetime is provided
153+
154+
$cacheStrategy->add($embeddedResponse);
155+
$cacheStrategy->update($masterResponse);
156+
157+
$this->assertTrue($masterResponse->headers->hasCacheControlDirective('no-cache'));
158+
$this->assertTrue($masterResponse->headers->hasCacheControlDirective('must-revalidate'));
159+
$this->assertFalse($masterResponse->isFresh());
160+
}
161+
162+
public function testEmbeddingPrivateResponseMakesMainResponsePrivate()
163+
{
164+
$cacheStrategy = new ResponseCacheStrategy();
165+
166+
$masterResponse = new Response();
167+
$masterResponse->setSharedMaxAge(3600); // public, cacheable
168+
169+
// The embedded response might for example contain per-user data that remains valid for 60 seconds
170+
$embeddedResponse = new Response();
171+
$embeddedResponse->setPrivate();
172+
$embeddedResponse->setMaxAge(60); // this would implicitly set "private" as well, but let's be explicit
173+
174+
$cacheStrategy->add($embeddedResponse);
175+
$cacheStrategy->update($masterResponse);
176+
177+
$this->assertTrue($masterResponse->headers->hasCacheControlDirective('private'));
178+
// Not sure if we should pass "max-age: 60" in this case, as long as the response is private and
179+
// that's the more conservative of both the master and embedded response...?
180+
}
140181
}

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.