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

bug #41665 [HttpKernel] Keep max lifetime also when part of the responses don't set it (mpdude) 

This PR was merged into the 4.4 branch.

Discussion
----------

[HttpKernel] Keep max lifetime also when part of the responses don't set it

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

https://datatracker.ietf.org/doc/html/rfc7234#section-4.2.2 allows caches to assign a "heuristic expiration time" for responses that have no explicit expiration time set, but are explicitly marked as being cacheable by `public`. We can say that such responses are "more liberal" in what is allowed than a response with an explicit `max-age` or `s-maxage` header.

When merging responses in `ResponseCacheStrategy`, such `public` responses without explicit expiration time should  not cause the `max-age` or `s-maxage` values being dropped on the final response. The most restrictive settings from all responses involved should be used, and any given expiration time is more strict than not setting one when being `public`.

Commits
-------

ad1f057ce2 Public responses without lifetime should not remove lifetime for the resulting response
This commit is contained in:
Fabien Potencier 2021-06-24 12:44:38 +02:00
parent 7685645b46 ad1f057ce2
commit 396bdcc08b
2 changed files with 49 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -81,14 +81,15 @@ class ResponseCacheStrategy implements ResponseCacheStrategyInterface
return;
}
$isHeuristicallyCacheable = $response->headers->hasCacheControlDirective('public');
$maxAge = $response->headers->hasCacheControlDirective('max-age') ? (int) $response->headers->getCacheControlDirective('max-age') : null;
$this->storeRelativeAgeDirective('max-age', $maxAge, $age);
$this->storeRelativeAgeDirective('max-age', $maxAge, $age, $isHeuristicallyCacheable);
$sharedMaxAge = $response->headers->hasCacheControlDirective('s-maxage') ? (int) $response->headers->getCacheControlDirective('s-maxage') : $maxAge;
$this->storeRelativeAgeDirective('s-maxage', $sharedMaxAge, $age);
$this->storeRelativeAgeDirective('s-maxage', $sharedMaxAge, $age, $isHeuristicallyCacheable);
$expires = $response->getExpires();
$expires = null !== $expires ? (int) $expires->format('U') - (int) $response->getDate()->format('U') : null;
$this->storeRelativeAgeDirective('expires', $expires >= 0 ? $expires : null, 0);
$this->storeRelativeAgeDirective('expires', $expires >= 0 ? $expires : null, 0, $isHeuristicallyCacheable);
}
/**
@ -199,11 +200,29 @@ class ResponseCacheStrategy implements ResponseCacheStrategyInterface
* we have to subtract the age so that the value is normalized for an age of 0.
*
* If the value is lower than the currently stored value, we update the value, to keep a rolling
* minimal value of each instruction. If the value is NULL, the directive will not be set on the final response.
* minimal value of each instruction.
*
* If the value is NULL and the isHeuristicallyCacheable parameter is false, the directive will
* not be set on the final response. In this case, not all responses had the directive set and no
* value can be found that satisfies the requirements of all responses. The directive will be dropped
* from the final response.
*
* If the isHeuristicallyCacheable parameter is true, however, the current response has been marked
* as cacheable in a public (shared) cache, but did not provide an explicit lifetime that would serve
* as an upper bound. In this case, we can proceed and possibly keep the directive on the final response.
*/
private function storeRelativeAgeDirective(string $directive, ?int $value, int $age)
private function storeRelativeAgeDirective(string $directive, ?int $value, int $age, bool $isHeuristicallyCacheable)
{
if (null === $value) {
if ($isHeuristicallyCacheable) {
/*
* See https://datatracker.ietf.org/doc/html/rfc7234#section-4.2.2
* This particular response does not require maximum lifetime; heuristics might be applied.
* Other responses, however, might have more stringent requirements on maximum lifetime.
* So, return early here so that the final response can have the more limiting value set.
*/
return;
}
$this->ageDirectives[$directive] = false;
}

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

@ -370,7 +370,7 @@ class ResponseCacheStrategyTest extends TestCase
];
yield 'merge max-age and s-maxage' => [
['public' => true, 's-maxage' => '60', 'max-age' => null],
['public' => true, 'max-age' => '60'],
['public' => true, 's-maxage' => 3600],
[
['public' => true, 'max-age' => 60],
@ -393,6 +393,30 @@ class ResponseCacheStrategyTest extends TestCase
],
];
yield 'public subresponse without lifetime does not remove lifetime for main response' => [
['public' => true, 's-maxage' => '30', 'max-age' => null],
['public' => true, 's-maxage' => '30'],
[
['public' => true],
],
];
yield 'lifetime for subresponse is kept when main response has no lifetime' => [
['public' => true, 'max-age' => '30'],
['public' => true],
[
['public' => true, 'max-age' => '30'],
],
];
yield 's-maxage on the subresponse implies public, so the result is public as well' => [
['public' => true, 'max-age' => '10', 's-maxage' => null],
['public' => true, 'max-age' => '10'],
[
['max-age' => '30', 's-maxage' => '20'],
],
];
yield 'result is private when combining private responses' => [
['no-cache' => false, 'must-revalidate' => false, 'private' => true],
['s-maxage' => 60, 'private' => true],