diogo
/
symfony
Archived
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

bug #41663 [HttpKernel] [HttpCache] Keep s-maxage=0 from ESI sub-responses (mpdude) 

This PR was squashed before being merged into the 4.4 branch.

Discussion
----------

[HttpKernel] [HttpCache] Keep s-maxage=0 from ESI sub-responses

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       |
| License       | MIT
| Doc PR        |

When the `ResponseCacheStrategy` is merging ESI surrogates and the master response, it treats `s-maxage=0` as if no `s-maxage` has been set.

The result is that for a main and a surrogate response that both are `public, s-maxage=0`, the result will only be `public`, with no further expiration time.

https://datatracker.ietf.org/doc/html/rfc7234#section-4.2.2 allows caches to assign a _heuristic expiration time_ when no explicit expiration time has been given but the response has been marked as explicitly cacheable with `public`. Clearly, such a heuristic was _not_ intended or desired when `public, s-maxage=0` was given.

This PR ensures that `s-maxage=0` is passed along with the resulting response.

### Some notes on `s-maxage=0`

You might argue that `s-maxage=0` does not make sense on a response.

According to https://datatracker.ietf.org/doc/html/rfc7234#section-3.2, `s-maxage=0` is a valid setting to ensure that a cached response "cannot be used to satisfy a subsequent request without revalidating it on the origin server".

This setting can be used to keep responses in edge caches/CDNs, but to re-validate on every request. The bottom line result can still be faster (304 + response already at the edge vs. fetch response from origin).

To my understanding, the difference between `s-maxage=0` and `must-revalidate` is that a "disconnected" cache (one that cannot contact the origin server) _must not_ use a stale response when `must-revalidate` is used, but _is not prohibited_  from doing so for `s-maxage=0` (https://datatracker.ietf.org/doc/html/rfc7234#section-4.2.4). In other words, `must-revalidate` is not exactly the same as (or the "right" way instead of) `s-maxage=0`.

In the special case of ESI (composite) responses, revalidation is not possible (no `ETag`, no `Last-Modified`). But, as explained above, it is still important to pass on the explicit expiration time, instead of having no value for it.

Commits
-------

ee7bc0272e [HttpKernel] [HttpCache] Keep s-maxage=0 from ESI sub-responses
This commit is contained in:
Fabien Potencier 2021-06-23 11:45:01 +02:00
parent 0c1343f4ee ee7bc0272e
commit 4bbd76d4fe
2 changed files with 20 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
src/Symfony/Component/HttpKernel/HttpCache/ResponseCacheStrategy.php
View File

@ -81,8 +81,10 @@ class ResponseCacheStrategy implements ResponseCacheStrategyInterface
return;
}
$this->storeRelativeAgeDirective('max-age', $response->headers->getCacheControlDirective('max-age'), $age);
$this->storeRelativeAgeDirective('s-maxage', $response->headers->getCacheControlDirective('s-maxage') ?: $response->headers->getCacheControlDirective('max-age'), $age);
$maxAge = $response->headers->hasCacheControlDirective('max-age') ? (int) $response->headers->getCacheControlDirective('max-age') : null;
$this->storeRelativeAgeDirective('max-age', $maxAge, $age);
$sharedMaxAge = $response->headers->hasCacheControlDirective('s-maxage') ? (int) $response->headers->getCacheControlDirective('s-maxage') : $maxAge;
$this->storeRelativeAgeDirective('s-maxage', $sharedMaxAge, $age);
$expires = $response->getExpires();
$expires = null !== $expires ? (int) $expires->format('U') - (int) $response->getDate()->format('U') : null;

16
src/Symfony/Component/HttpKernel/Tests/HttpCache/ResponseCacheStrategyTest.php
View File

@ -377,6 +377,22 @@ class ResponseCacheStrategyTest extends TestCase
],
];
yield 's-maxage may be set to 0' => [
['public' => true, 's-maxage' => '0', 'max-age' => null],
['public' => true, 's-maxage' => '0'],
[
['public' => true, 's-maxage' => '60'],
],
];
yield 's-maxage may be set to 0, and works independently from maxage' => [
['public' => true, 's-maxage' => '0', 'max-age' => '30'],
['public' => true, 's-maxage' => '0', 'max-age' => '30'],
[
['public' => true, 'max-age' => '60'],
],
];
yield 'result is private when combining private responses' => [
['no-cache' => false, 'must-revalidate' => false, 'private' => true],
['s-maxage' => 60, 'private' => true],