Skip to content

Commit eff3069

Browse files
aschemppaschempp
committed
Fixed max-age/s-maxage when value is 0 and Expires header
1 parent e00726c commit eff3069

File tree

2 files changed

+100
-11
lines changed

2 files changed

+100
-11
lines changed

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -132,21 +132,23 @@ public function update(Response $response)
132132
$maxAge = null;
133133
$sMaxage = null;
134134

135-
if ($this->ageDirectives['max-age']) {
135+
if (\is_numeric($this->ageDirectives['max-age'])) {
136136
$maxAge = $this->ageDirectives['max-age'] + $this->age;
137137
$response->headers->addCacheControlDirective('max-age', $maxAge);
138138
}
139139

140-
if ($this->ageDirectives['s-maxage']) {
140+
if (\is_numeric($this->ageDirectives['s-maxage'])) {
141141
$sMaxage = $this->ageDirectives['s-maxage'] + $this->age;
142142

143143
if ($maxAge !== $sMaxage) {
144144
$response->headers->addCacheControlDirective('s-maxage', $sMaxage);
145145
}
146146
}
147147

148-
if ($this->ageDirectives['expires']) {
149-
$response->setExpires($response->getDate()->modify('+'.$this->ageDirectives['expires'].' seconds'));
148+
if (\is_numeric($this->ageDirectives['expires'])) {
149+
$date = clone $response->getDate();
150+
$date->modify('+'.($this->ageDirectives['expires']+$this->age).' seconds');
151+
$response->setExpires($date);
150152
}
151153
}
152154

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

Lines changed: 94 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,14 @@ public function testCacheControlMerging(array $expects, array $master, array $su
249249

250250
foreach ($config as $key => $value) {
251251
switch ($key) {
252+
case 'age':
253+
$response->headers->set('Age', $value);
254+
break;
255+
256+
case 'expires':
257+
$response->setExpires((clone $response->getDate())->modify('+'.$value.' seconds'));
258+
break;
259+
252260
case 'max-age':
253261
$response->setMaxAge($value);
254262
break;
@@ -281,12 +289,19 @@ public function testCacheControlMerging(array $expects, array $master, array $su
281289
$cacheStrategy->update($response);
282290

283291
foreach ($expects as $key => $value) {
284-
if (true === $value) {
292+
if ($key === 'expires') {
293+
$this->assertSame($value, $response->getExpires()->format('U') - $response->getDate()->format('U'));
294+
} else if ($key === 'age') {
295+
$this->assertSame($value, $response->getAge());
296+
} elseif (true === $value) {
285297
$this->assertTrue($response->headers->hasCacheControlDirective($key), sprintf('Cache-Control header must have "%s" flag', $key));
286298
} elseif (false === $value) {
287-
$this->assertFalse($response->headers->hasCacheControlDirective($key), sprintf('Cache-Control header must NOT have "%s" flag', $key));
299+
$this->assertFalse(
300+
$response->headers->hasCacheControlDirective($key),
301+
sprintf('Cache-Control header must NOT have "%s" flag', $key)
302+
);
288303
} else {
289-
$this->assertEquals($value, $response->headers->getCacheControlDirective($key), sprintf('Cache-Control flag "%s" should be "%s"', $key, $value));
304+
$this->assertSame($value, $response->headers->getCacheControlDirective($key), sprintf('Cache-Control flag "%s" should be "%s"', $key, $value));
290305
}
291306
}
292307
}
@@ -334,15 +349,15 @@ public function cacheControlMergingProvider()
334349
);
335350

336351
yield 'resolve to lowest possible max-age' => array(
337-
array('public' => false, 'private' => true, 's-maxage' => false, 'max-age' => 60),
352+
array('public' => false, 'private' => true, 's-maxage' => false, 'max-age' => '60'),
338353
array('public' => true, 'max-age' => 3600),
339354
array(
340355
array('private' => true, 'max-age' => 60),
341356
),
342357
);
343358

344359
yield 'resolves multiple max-age' => array(
345-
array('public' => false, 'private' => true, 's-maxage' => false, 'max-age' => 60),
360+
array('public' => false, 'private' => true, 's-maxage' => false, 'max-age' => '60'),
346361
array('private' => true, 'max-age' => 100),
347362
array(
348363
array('private' => true, 'max-age' => 3600),
@@ -352,7 +367,7 @@ public function cacheControlMergingProvider()
352367
);
353368

354369
yield 'merge max-age and s-maxage' => array(
355-
array('public' => true, 's-maxage' => 60, 'max-age' => null),
370+
array('public' => true, 's-maxage' => '60', 'max-age' => null),
356371
array('public' => true, 's-maxage' => 3600),
357372
array(
358373
array('public' => true, 'max-age' => 60),
@@ -368,13 +383,85 @@ public function cacheControlMergingProvider()
368383
);
369384

370385
yield 'result can have s-maxage and max-age' => array(
371-
array('public' => true, 'private' => false, 's-maxage' => 60, 'max-age' => 30),
386+
array('public' => true, 'private' => false, 's-maxage' => '60', 'max-age' => '30'),
372387
array('s-maxage' => 100, 'max-age' => 2000),
373388
array(
374389
array('s-maxage' => 1000, 'max-age' => 30),
375390
array('s-maxage' => 500, 'max-age' => 500),
376391
array('s-maxage' => 60, 'max-age' => 1000),
377392
),
378393
);
394+
395+
yield 'does not set headers without value' => array(
396+
array('max-age' => null, 's-maxage' => null, 'public' => null),
397+
array('private' => true),
398+
array(
399+
array('private' => true),
400+
),
401+
);
402+
403+
yield 'max-age 0 is sent to the client' => array(
404+
array('private' => true, 'max-age' => '0'),
405+
array('max-age' => 0, 'private' => true),
406+
array(
407+
array('max-age' => 60, 'private' => true),
408+
),
409+
);
410+
411+
yield 'max-age is relative to age' => array(
412+
array('max-age' => '240', 'age' => 60),
413+
array('max-age' => 180),
414+
array(
415+
array('max-age' => 600, 'age' => 60),
416+
),
417+
);
418+
419+
yield 'retains lowest age of all responses' => array(
420+
array('max-age' => '160', 'age' => 60),
421+
array('max-age' => 600, 'age' => 60),
422+
array(
423+
array('max-age' => 120, 'age' => 20),
424+
),
425+
);
426+
427+
yield 'max-age can be less than age, essentially expiring the response' => array(
428+
array('age' => 120, 'max-age' => '90'),
429+
array('max-age' => 90, 'age' => 120),
430+
array(
431+
array('max-age' => 120, 'age' => 60),
432+
),
433+
);
434+
435+
yield 'max-age is 0 regardless of age' => array(
436+
array('max-age' => '0'),
437+
array('max-age' => 60),
438+
array(
439+
array('max-age' => 0, 'age' => 60),
440+
),
441+
);
442+
443+
yield 'max-age is not negative' => array(
444+
array('max-age' => '0'),
445+
array('max-age' => 0),
446+
array(
447+
array('max-age' => 0, 'age' => 60),
448+
),
449+
);
450+
451+
yield 'calculates lowest Expires header' => array(
452+
array('expires' => 60),
453+
array('expires' => 60),
454+
array(
455+
array('expires' => 120),
456+
),
457+
);
458+
459+
yield 'calculates Expires header relative to age' => array(
460+
array('expires' => 210, 'age' => 120),
461+
array('expires' => 90),
462+
array(
463+
array('expires' => 600, 'age' => '120'),
464+
),
465+
);
379466
}
380467
}

0 commit comments

Comments
 (0)