bug #26643 Fix that ESI/SSI processing can turn a "private" response "public" (mpdude)
This PR was squashed before being merged into the 2.7 branch (closes #26643).
Discussion
----------
Fix that ESI/SSI processing can turn a "private" response "public"
| 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 |
Under the condition that
* we are merging in at least one *embedded* response,
* all *embedded* responses are `public`,
* the *main* response is `private` and
* all responses use expiration-based caching (note: no `s-maxage` on the *main* response)
... the resulting response will turn to `Cache-Control: public`.
The real issue is that when all responses use expiration-based caching, a combined max age is computed. This is set on the *main* response using `Response::setSharedMaxAge()`, which implicitly sets `Cache-Control: public`.
The fix provided in this PR solves the problem by applying the same logic to the *main* response that is applied for *embedded* responses, namely that responses with `!Response::isCacheable()` will make the resulting response have `Cache-Control: private, no-cache, must-revalidate` and have `(s)max-age` removed.
This makes the change easy to understand, but makes responses uncacheable too often. This is because the `Response::isCacheable()` method was written to determine whether it is safe for a shared cache to keep the response, which is not the case as soon as a `private` response is involved. This might be improved upon in another PR.
Commits
-------
3d27b5946d
Fix that ESI/SSI processing can turn a \"private\" response \"public\"
This commit is contained in:
commit
d17d38d291
@ -507,13 +507,19 @@ class Response
|
||||
}
|
||||
|
||||
/**
|
||||
* 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
|
||||
*/
|
||||
|
@ -72,7 +72,7 @@ class ResponseCacheStrategy implements ResponseCacheStrategyInterface
|
||||
$response->setLastModified(null);
|
||||
}
|
||||
|
||||
if (!$response->isFresh()) {
|
||||
if (!$response->isFresh() || !$response->isCacheable()) {
|
||||
$this->cacheable = false;
|
||||
}
|
||||
|
||||
|
@ -175,8 +175,26 @@ class ResponseCacheStrategyTest extends TestCase
|
||||
$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()
|
||||
|
Reference in New Issue
Block a user