Skip to content

Commit

Permalink
Fixed max-age/s-maxage when value is 0 and Expires header
Browse files Browse the repository at this point in the history
  • Loading branch information
@aschempp
aschempp committed Dec 21, 2018
1 parent 13febfb commit 8fd595e
Show file tree
Hide file tree
Showing 2 changed files with 100 additions and 11 deletions.
10 changes: 6 additions & 4 deletions src/Symfony/Component/HttpKernel/HttpCache/ResponseCacheStrategy.php
View file
Expand Up @@ -132,21 +132,23 @@ public function update(Response $response)
$maxAge = null;
$sMaxage = null;

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

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

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

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

Expand Down
101 changes: 94 additions & 7 deletions src/Symfony/Component/HttpKernel/Tests/HttpCache/ResponseCacheStrategyTest.php
View file
Expand Up @@ -249,6 +249,14 @@ public function testCacheControlMerging(array $expects, array $master, array $su

foreach ($config as $key => $value) {
switch ($key) {
case 'age':
$response->headers->set('Age', $value);
break;

case 'expires':
$response->setExpires((clone $response->getDate())->modify('+'.$value.' seconds'));
break;

case 'max-age':
$response->setMaxAge($value);
break;
Expand Down Expand Up @@ -281,12 +289,19 @@ public function testCacheControlMerging(array $expects, array $master, array $su
$cacheStrategy->update($response);

foreach ($expects as $key => $value) {
if (true === $value) {
if ($key === 'expires') {
$this->assertSame($value, $response->getExpires()->format('U') - $response->getDate()->format('U'));
} else if ($key === 'age') {
$this->assertSame($value, $response->getAge());
} elseif (true === $value) {
$this->assertTrue($response->headers->hasCacheControlDirective($key), sprintf('Cache-Control header must have "%s" flag', $key));
} elseif (false === $value) {
$this->assertFalse($response->headers->hasCacheControlDirective($key), sprintf('Cache-Control header must NOT have "%s" flag', $key));
$this->assertFalse(
$response->headers->hasCacheControlDirective($key),
sprintf('Cache-Control header must NOT have "%s" flag', $key)
);
} else {
$this->assertEquals($value, $response->headers->getCacheControlDirective($key), sprintf('Cache-Control flag "%s" should be "%s"', $key, $value));
$this->assertSame($value, $response->headers->getCacheControlDirective($key), sprintf('Cache-Control flag "%s" should be "%s"', $key, $value));
}
}
}
Expand Down Expand Up @@ -334,15 +349,15 @@ public function cacheControlMergingProvider()
);

yield 'resolve to lowest possible max-age' => array(
array('public' => false, 'private' => true, 's-maxage' => false, 'max-age' => 60),
array('public' => false, 'private' => true, 's-maxage' => false, 'max-age' => '60'),
array('public' => true, 'max-age' => 3600),
array(
array('private' => true, 'max-age' => 60),
),
);

yield 'resolves multiple max-age' => array(
array('public' => false, 'private' => true, 's-maxage' => false, 'max-age' => 60),
array('public' => false, 'private' => true, 's-maxage' => false, 'max-age' => '60'),
array('private' => true, 'max-age' => 100),
array(
array('private' => true, 'max-age' => 3600),
Expand All @@ -352,7 +367,7 @@ public function cacheControlMergingProvider()
);

yield 'merge max-age and s-maxage' => array(
array('public' => true, 's-maxage' => 60, 'max-age' => null),
array('public' => true, 's-maxage' => '60', 'max-age' => null),
array('public' => true, 's-maxage' => 3600),
array(
array('public' => true, 'max-age' => 60),
Expand All @@ -368,13 +383,85 @@ public function cacheControlMergingProvider()
);

yield 'result can have s-maxage and max-age' => array(
array('public' => true, 'private' => false, 's-maxage' => 60, 'max-age' => 30),
array('public' => true, 'private' => false, 's-maxage' => '60', 'max-age' => '30'),
array('s-maxage' => 100, 'max-age' => 2000),
array(
array('s-maxage' => 1000, 'max-age' => 30),
array('s-maxage' => 500, 'max-age' => 500),
array('s-maxage' => 60, 'max-age' => 1000),
),
);

yield 'does not set headers without value' => array(
array('max-age' => null, 's-maxage' => null, 'public' => null),
array('private' => true),
array(
array('private' => true),
),
);

yield 'max-age 0 is sent to the client' => array(
array('private' => true, 'max-age' => '0'),
array('max-age' => 0, 'private' => true),
array(
array('max-age' => 60, 'private' => true),
),
);

yield 'max-age is relative to age' => array(
array('max-age' => '240', 'age' => 60),
array('max-age' => 180),
array(
array('max-age' => 600, 'age' => 60),
),
);

yield 'retains lowest age of all responses' => array(
array('max-age' => '160', 'age' => 60),
array('max-age' => 600, 'age' => 60),
array(
array('max-age' => 120, 'age' => 20),
),
);

yield 'max-age can be less than age, essentially expiring the response' => array(
array('age' => 120, 'max-age' => '90'),
array('max-age' => 90, 'age' => 120),
array(
array('max-age' => 120, 'age' => 60),
),
);

yield 'max-age is 0 regardless of age' => array(
array('max-age' => '0'),
array('max-age' => 60),
array(
array('max-age' => 0, 'age' => 60),
),
);

yield 'max-age is not negative' => array(
array('max-age' => '0'),
array('max-age' => 0),
array(
array('max-age' => 0, 'age' => 60),
),
);

yield 'calculates lowest Expires header' => array(
array('expires' => 60),
array('expires' => 60),
array(
array('expires' => 120),
),
);

yield 'calculates Expires header relative to age' => array(
array('expires' => 210, 'age' => 120),
array('expires' => 90),
array(
array('expires' => 600, 'age' => '120'),
),
);
}
}

0 comments on commit 8fd595e

Please sign in to comment.