Movatterモバイル変換

[0]ホーム
URL:

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

 Sign up
Appearance settings

Commit908a91f

Browse files
bug#58376 [HttpKernel] Correctly mergemax-age/s-maxage andExpires headers (aschempp)
This PR was squashed before being merged into the 5.4 branch.Discussion----------[HttpKernel] Correctly merge `max-age`/`s-maxage` and `Expires` headers| Q | A| ------------- | ---| Branch? | 5.4| Bug fix? | yes| New feature? | no| Deprecations? | no| Issues |Fixcontao/contao#7494| License | MITThe `ResponseCacheStrategy` does not currently merge `Expires` and `Cache-Control: max-age/s-maxage` headers. Before#41665 this was not an issue, because if not all respones had all headers, they were not added to the final reponse. And we could assume a response itself is consistent between `Expires` and `max-age`.`@mpdude` added _heuristic caching_ of public responses in#41665. Unfortunately, it only looks at `Cache-Control: public` but if should also check if no cache information (max-age/s-maxage/Expires) is present. If that were the case, the behavior would not have changed. But it now leads to inconsistent header values because it independently keeps `Expires` and `max-age/s-maxage`.This PR does not only fix the _heuristic caching_, but also merges `Expires` and `Cache-Control` headers to make sure only the lowest value is retained across all headers. For semi-BC reasons I also made sure to only add an `Expires` header if any of the responses contains one.Commits-------d3e65d6 [HttpKernel] Correctly merge `max-age`/`s-maxage` and `Expires` headers
2 parents7fe5bba +d3e65d6 commit908a91f

File tree

2 files changed

+89
-33
lines changed

2 files changed

+89
-33
lines changed

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

Lines changed: 28 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ class ResponseCacheStrategy implements ResponseCacheStrategyInterface
5050
private$ageDirectives = [
5151
'max-age' =>null,
5252
's-maxage' =>null,
53-
'expires' =>null,
53+
'expires' =>false,
5454
];
5555

5656
/**
@@ -81,15 +81,30 @@ public function add(Response $response)
8181
return;
8282
}
8383

84-
$isHeuristicallyCacheable =$response->headers->hasCacheControlDirective('public');
8584
$maxAge =$response->headers->hasCacheControlDirective('max-age') ? (int)$response->headers->getCacheControlDirective('max-age') :null;
86-
$this->storeRelativeAgeDirective('max-age',$maxAge,$age,$isHeuristicallyCacheable);
8785
$sharedMaxAge =$response->headers->hasCacheControlDirective('s-maxage') ? (int)$response->headers->getCacheControlDirective('s-maxage') :$maxAge;
88-
$this->storeRelativeAgeDirective('s-maxage',$sharedMaxAge,$age,$isHeuristicallyCacheable);
89-
9086
$expires =$response->getExpires();
9187
$expires =null !==$expires ? (int)$expires->format('U') - (int)$response->getDate()->format('U') :null;
92-
$this->storeRelativeAgeDirective('expires',$expires >=0 ?$expires :null,0,$isHeuristicallyCacheable);
88+
89+
// See https://datatracker.ietf.org/doc/html/rfc7234#section-4.2.2
90+
// If a response is "public" but does not have maximum lifetime, heuristics might be applied.
91+
// Do not store NULL values so the final response can have more limiting value from other responses.
92+
$isHeuristicallyCacheable =$response->headers->hasCacheControlDirective('public')
93+
&&null ===$maxAge
94+
&&null ===$sharedMaxAge
95+
&&null ===$expires;
96+
97+
if (!$isHeuristicallyCacheable ||null !==$maxAge ||null !==$expires) {
98+
$this->storeRelativeAgeDirective('max-age',$maxAge,$expires,$age);
99+
}
100+
101+
if (!$isHeuristicallyCacheable ||null !==$sharedMaxAge ||null !==$expires) {
102+
$this->storeRelativeAgeDirective('s-maxage',$sharedMaxAge,$expires,$age);
103+
}
104+
105+
if (null !==$expires) {
106+
$this->ageDirectives['expires'] =true;
107+
}
93108
}
94109

95110
/**
@@ -102,7 +117,7 @@ public function update(Response $response)
102117
return;
103118
}
104119

105-
// Remove validation related headers of themaster response,
120+
// Remove validation related headers of thefinal response,
106121
// because some of the response content comes from at least
107122
// one embedded response (which likely has a different caching strategy).
108123
$response->setEtag(null);
@@ -145,9 +160,9 @@ public function update(Response $response)
145160
}
146161
}
147162

148-
if (is_numeric($this->ageDirectives['expires'])) {
163+
if ($this->ageDirectives['expires'] &&null !==$maxAge) {
149164
$date =clone$response->getDate();
150-
$date =$date->modify('+'.($this->ageDirectives['expires'] +$this->age).' seconds');
165+
$date =$date->modify('+'.$maxAge.' seconds');
151166
$response->setExpires($date);
152167
}
153168
}
@@ -200,33 +215,16 @@ private function willMakeFinalResponseUncacheable(Response $response): bool
200215
* we have to subtract the age so that the value is normalized for an age of 0.
201216
*
202217
* If the value is lower than the currently stored value, we update the value, to keep a rolling
203-
* minimal value of each instruction.
204-
*
205-
* If the value is NULL and the isHeuristicallyCacheable parameter is false, the directive will
206-
* not be set on the final response. In this case, not all responses had the directive set and no
207-
* value can be found that satisfies the requirements of all responses. The directive will be dropped
208-
* from the final response.
209-
*
210-
* If the isHeuristicallyCacheable parameter is true, however, the current response has been marked
211-
* as cacheable in a public (shared) cache, but did not provide an explicit lifetime that would serve
212-
* as an upper bound. In this case, we can proceed and possibly keep the directive on the final response.
218+
* minimal value of each instruction. If the value is NULL, the directive will not be set on the final response.
213219
*/
214-
privatefunctionstoreRelativeAgeDirective(string$directive, ?int$value,int$age,bool$isHeuristicallyCacheable)
220+
privatefunctionstoreRelativeAgeDirective(string$directive, ?int$value,?int$expires,int$age):void
215221
{
216-
if (null ===$value) {
217-
if ($isHeuristicallyCacheable) {
218-
/*
219-
* See https://datatracker.ietf.org/doc/html/rfc7234#section-4.2.2
220-
* This particular response does not require maximum lifetime; heuristics might be applied.
221-
* Other responses, however, might have more stringent requirements on maximum lifetime.
222-
* So, return early here so that the final response can have the more limiting value set.
223-
*/
224-
return;
225-
}
222+
if (null ===$value &&null ===$expires) {
226223
$this->ageDirectives[$directive] =false;
227224
}
228225

229226
if (false !==$this->ageDirectives[$directive]) {
227+
$value =min($value ??PHP_INT_MAX,$expires ??PHP_INT_MAX);
230228
$value -=$age;
231229
$this->ageDirectives[$directive] =null !==$this->ageDirectives[$directive] ?min($this->ageDirectives[$directive],$value) :$value;
232230
}

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

Lines changed: 61 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,64 @@ public function testSharedMaxAgeNotSetIfNotSetInMainRequest()
7676
$this->assertFalse($response->headers->hasCacheControlDirective('s-maxage'));
7777
}
7878

79+
publicfunctiontestExpiresHeaderUpdatedFromMaxAge()
80+
{
81+
$cacheStrategy =newResponseCacheStrategy();
82+
83+
$response1 =newResponse();
84+
$response1->setExpires(new \DateTime('+ 1 hour'));
85+
$response1->setPublic();
86+
$cacheStrategy->add($response1);
87+
88+
$response =newResponse();
89+
$response->setMaxAge(0);
90+
$response->setSharedMaxAge(86400);
91+
$cacheStrategy->update($response);
92+
93+
$this->assertSame('0',$response->headers->getCacheControlDirective('max-age'));
94+
$this->assertSame('3600',$response->headers->getCacheControlDirective('s-maxage'));
95+
96+
// Expires header must be same as Date header because "max-age" is 0.
97+
$this->assertSame($response->headers->get('Date'),$response->headers->get('Expires'));
98+
}
99+
100+
publicfunctiontestMaxAgeUpdatedFromExpiresHeader()
101+
{
102+
$cacheStrategy =newResponseCacheStrategy();
103+
104+
$response1 =newResponse();
105+
$response1->setExpires(new \DateTime('+ 1 hour'));
106+
$response1->setPublic();
107+
$cacheStrategy->add($response1);
108+
109+
$response =newResponse();
110+
$response->setMaxAge(86400);
111+
$cacheStrategy->update($response);
112+
113+
$this->assertSame('3600',$response->headers->getCacheControlDirective('max-age'));
114+
$this->assertNull($response->headers->getCacheControlDirective('s-maxage'));
115+
$this->assertSame((new \DateTime('+ 1 hour'))->format('D, d M Y H:i:s').' GMT',$response->headers->get('Expires'));
116+
}
117+
118+
publicfunctiontestMaxAgeAndSharedMaxAgeUpdatedFromExpiresHeader()
119+
{
120+
$cacheStrategy =newResponseCacheStrategy();
121+
122+
$response1 =newResponse();
123+
$response1->setExpires(new \DateTime('+ 1 day'));
124+
$response1->setPublic();
125+
$cacheStrategy->add($response1);
126+
127+
$response =newResponse();
128+
$response->setMaxAge(3600);
129+
$response->setSharedMaxAge(86400);
130+
$cacheStrategy->update($response);
131+
132+
$this->assertSame('3600',$response->headers->getCacheControlDirective('max-age'));
133+
$this->assertSame('86400',$response->headers->getCacheControlDirective('s-maxage'));
134+
$this->assertSame((new \DateTime('+ 1 hour'))->format('D, d M Y H:i:s').' GMT',$response->headers->get('Expires'));
135+
}
136+
79137
publicfunctiontestMainResponseNotCacheableWhenEmbeddedResponseRequiresValidation()
80138
{
81139
$cacheStrategy =newResponseCacheStrategy();
@@ -243,7 +301,7 @@ public function testResponseIsExpirableButNotValidateableWhenMainResponseCombine
243301
*
244302
* @dataProvider cacheControlMergingProvider
245303
*/
246-
publicfunctiontestCacheControlMerging(array$expects,array$master,array$surrogates)
304+
publicfunctiontestCacheControlMerging(array$expects,array$main,array$surrogates)
247305
{
248306
$cacheStrategy =newResponseCacheStrategy();
249307
$buildResponse =function ($config) {
@@ -289,7 +347,7 @@ public function testCacheControlMerging(array $expects, array $master, array $su
289347
$cacheStrategy->add($buildResponse($config));
290348
}
291349

292-
$response =$buildResponse($master);
350+
$response =$buildResponse($main);
293351
$cacheStrategy->update($response);
294352

295353
foreach ($expectsas$key =>$value) {
@@ -371,7 +429,7 @@ public static function cacheControlMergingProvider()
371429
];
372430

373431
yield'merge max-age and s-maxage' => [
374-
['public' =>true,'max-age' =>'60'],
432+
['public' =>true,'max-age' =>null,'s-maxage' =>'60'],
375433
['public' =>true,'s-maxage' =>3600],
376434
[
377435
['public' =>true,'max-age' =>60],

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp