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
Appearance settings

Commit 3447f59

Browse filesBrowse files
mpdudempdude
committed
Fix stale-if-error behaviour, add tests
1 parent d1e31a4 commit 3447f59
Copy full SHA for 3447f59

File tree

Expand file treeCollapse file tree

3 files changed

+159
-5
lines changed
Filter options
Expand file treeCollapse file tree

3 files changed

+159
-5
lines changed

‎src/Symfony/Component/HttpFoundation/Response.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/HttpFoundation/Response.php
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -649,7 +649,7 @@ public function isImmutable()
649649
}
650650

651651
/**
652-
* Returns true if the response must be revalidated by caches.
652+
* Returns true if the response must be revalidated by shared caches once it has become stale.
653653
*
654654
* This method indicates that the response must not be served stale by a
655655
* cache in any circumstance without first revalidating with the origin.

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/HttpKernel/HttpCache/HttpCache.php
+25-4Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -448,15 +448,36 @@ protected function forward(Request $request, $catch = false, Response $entry = n
448448
// always a "master" request (as the real master request can be in cache)
449449
$response = SubRequestHandler::handle($this->kernel, $request, HttpKernelInterface::MASTER_REQUEST, $catch);
450450

451-
// we don't implement the stale-if-error on Requests, which is nonetheless part of the RFC
452-
if (null !== $entry && \in_array($response->getStatusCode(), [500, 502, 503, 504])) {
451+
/*
452+
* Support stale-if-error given on Responses or as a config option.
453+
* RFC 7234 summarizes in Section 4.2.4 (but also mentions with the individual
454+
* Cache-Control directives) that
455+
*
456+
* A cache MUST NOT generate a stale response if it is prohibited by an
457+
* explicit in-protocol directive (e.g., by a "no-store" or "no-cache"
458+
* cache directive, a "must-revalidate" cache-response-directive, or an
459+
* applicable "s-maxage" or "proxy-revalidate" cache-response-directive;
460+
* see Section 5.2.2).
461+
*
462+
* https://tools.ietf.org/html/rfc7234#section-4.2.4
463+
*/
464+
if (null !== $entry
465+
&& \in_array($response->getStatusCode(), [500, 502, 503, 504])
466+
&& !$entry->headers->hasCacheControlDirective('no-cache')
467+
&& !$entry->mustRevalidate()
468+
&& !$entry->headers->hasCacheControlDirective('s-maxage')
469+
) {
453470
if (null === $age = $entry->headers->getCacheControlDirective('stale-if-error')) {
454471
$age = $this->options['stale_if_error'];
455472
}
456473

457-
if (abs($entry->getTtl()) < $age) {
474+
/*
475+
* stale-if-error gives the (extra) time that the Response may be used *after* it has become stale.
476+
* So we compare the time the $entry has been sitting in the cache already with the
477+
* time it was fresh plus the allowed grace period.
478+
*/
479+
if ($entry->getAge() <= $entry->getMaxAge() + $age) {
458480
$this->record($request, 'stale-if-error');
459-
460481
return $entry;
461482
}
462483
}

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

Copy file name to clipboardExpand all lines: src/Symfony/Component/HttpKernel/Tests/HttpCache/HttpCacheTest.php
+133Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1507,6 +1507,139 @@ public function testUsesOriginalRequestForSurrogate()
15071507
// Surrogate request
15081508
$cache->handle($request, HttpKernelInterface::SUB_REQUEST);
15091509
}
1510+
1511+
public function testStaleIfErrorMustNotResetLifetime()
1512+
{
1513+
// Make sure we don't accidentally treat the response as fresh (revalidated) again
1514+
// when stale-if-error handling kicks in.
1515+
1516+
$responses = [
1517+
[
1518+
'status' => 200,
1519+
'body' => 'OK',
1520+
// This is cacheable and can be used in stale-if-error cases:
1521+
'headers' => ['Cache-Control' => 'public, max-age=10', 'ETag' => 'some-etag'],
1522+
],
1523+
[
1524+
'status' => 500,
1525+
'body' => 'FAIL',
1526+
'headers' => [],
1527+
],
1528+
[
1529+
'status' => 500,
1530+
'body' => 'FAIL',
1531+
'headers' => [],
1532+
],
1533+
];
1534+
1535+
$this->setNextResponses($responses);
1536+
$this->cacheConfig['stale_if_error'] = 10;
1537+
1538+
$this->request('GET', '/'); // warm cache
1539+
1540+
sleep(15); // now the entry is stale, but still within the grace period (10s max-age + 10s stale-if-error)
1541+
1542+
$this->request('GET', '/'); // hit backend error
1543+
$this->assertEquals(200, $this->response->getStatusCode()); // stale-if-error saved the day
1544+
$this->assertEquals(15, $this->response->getAge());
1545+
1546+
sleep(10); // now we're outside the grace period
1547+
1548+
$this->request('GET', '/'); // hit backend error
1549+
$this->assertEquals(500, $this->response->getStatusCode()); // fail
1550+
}
1551+
1552+
/**
1553+
* @dataProvider getResponseDataThatMayBeServedStaleIfError
1554+
*/
1555+
public function testResponsesThatMayBeUsedStaleIfError($responseHeaders, $sleepBetweenRequests = null)
1556+
{
1557+
$responses = [
1558+
[
1559+
'status' => 200,
1560+
'body' => 'OK',
1561+
'headers' => $responseHeaders,
1562+
],
1563+
[
1564+
'status' => 500,
1565+
'body' => 'FAIL',
1566+
'headers' => [],
1567+
],
1568+
];
1569+
1570+
$this->setNextResponses($responses);
1571+
$this->cacheConfig['stale_if_error'] = 10; // after stale, may be served for 10s
1572+
1573+
$this->request('GET', '/'); // warm cache
1574+
1575+
if ($sleepBetweenRequests) {
1576+
sleep($sleepBetweenRequests);
1577+
}
1578+
1579+
$this->request('GET', '/'); // hit backend error
1580+
1581+
$this->assertEquals(200, $this->response->getStatusCode());
1582+
$this->assertEquals('OK', $this->response->getContent());
1583+
$this->assertTraceContains('stale-if-error');
1584+
}
1585+
1586+
public function getResponseDataThatMayBeServedStaleIfError()
1587+
{
1588+
// All data sets assume that a 10s stale-if-error grace period has been configured
1589+
yield 'public, max-age expired' => [['Cache-Control' => 'public, max-age=60'], 65];
1590+
yield 'public, validateable with ETag, no TTL' => [['Cache-Control' => 'public', 'ETag' => 'some-etag'], 5];
1591+
yield 'public, validateable with Last-Modified, no TTL' => [['Cache-Control' => 'public', 'Last-Modified' => 'yesterday'], 5];
1592+
}
1593+
1594+
/**
1595+
* @dataProvider getResponseDataThatMustNotBeServedStaleIfError
1596+
*/
1597+
public function testResponsesThatMustNotBeUsedStaleIfError($responseHeaders, $sleepBetweenRequests = null)
1598+
{
1599+
$responses = [
1600+
[
1601+
'status' => 200,
1602+
'body' => 'OK',
1603+
'headers' => $responseHeaders,
1604+
],
1605+
[
1606+
'status' => 500,
1607+
'body' => 'FAIL',
1608+
'headers' => [],
1609+
],
1610+
];
1611+
1612+
$this->setNextResponses($responses);
1613+
$this->cacheConfig['stale_if_error'] = 10; // after stale, may be served for 10s
1614+
1615+
$this->request('GET', '/'); // warm cache
1616+
1617+
if ($sleepBetweenRequests) {
1618+
sleep($sleepBetweenRequests);
1619+
}
1620+
1621+
$this->request('GET', '/'); // hit backend error
1622+
1623+
$this->assertEquals(500, $this->response->getStatusCode());
1624+
}
1625+
1626+
public function getResponseDataThatMustNotBeServedStaleIfError()
1627+
{
1628+
// All data sets assume that a 10s stale-if-error grace period has been configured
1629+
yield 'public, no TTL but beyond grace period' => [['Cache-Control' => 'public'], 15];
1630+
yield 'public, validateable with ETag, no TTL but beyond grace period' => [['Cache-Control' => 'public', 'ETag' => 'some-etag'], 15];
1631+
yield 'public, validateable with Last-Modified, no TTL but beyond grace period' => [['Cache-Control' => 'public', 'Last-Modified' => 'yesterday'], 15];
1632+
yield 'public, stale beyond grace period' => [['Cache-Control' => 'public, max-age=10'], 30];
1633+
1634+
// Cache-control values that prohibit serving stale responses or responses without positive validation -
1635+
// see https://tools.ietf.org/html/rfc7234#section-4.2.4 and
1636+
// https://tools.ietf.org/html/rfc7234#section-5.2.2
1637+
yield 's-maxage prohibits serving stale responses' => [['Cache-Control' => 'public, s-maxage=20'], 25];
1638+
yield 'no-cache requires positive validation' => [['Cache-Control' => 'public, no-cache', 'ETag' => 'some-etag']];
1639+
yield 'no-cache requires positive validation, even if fresh' => [['Cache-Control' => 'public, no-cache, max-age=10']];
1640+
yield 'must-revalidate requires positive validation once stale' => [['Cache-Control' => 'public, max-age=10, must-revalidate'], 15];
1641+
yield 'proxy-revalidate requires positive validation once stale' => [['Cache-Control' => 'public, max-age=10, proxy-revalidate'], 15];
1642+
}
15101643
}
15111644

15121645
class TestKernel implements HttpKernelInterface

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.