Skip to content

Commit 2e1aa09

Browse files
nicolas-grekasnicolas-grekas
committed
Merge branch '4.4' into 5.0
* 4.4: [Validator] fix access to uninitialized property when getting value [HttpClient] Fix regex bearer [Translator] Default value for 'sort' option in translation:update should be 'asc' [HttpKernel] Fix stale-if-error behavior, add tests [Intl] Provide more locale translations [Mailer] Fix STARTTLS support for Postmark and Mandrill [Messenger] Check for all serialization exceptions during message dec… [Messenger] Fix bug when using single route with XML config Fix exception message in Doctrine Messenger [DI] CheckTypeDeclarationsPass now checks if value is type of parameter type [SecurityBundle] fix security.authentication.provider.ldap_bind arguments Improved error message when no supported user provider is found Mysqli doesn't support the named parameters used by PdoAdapter Added debug argument to decide if debug page should be shown or not Mysqli doesn't support the named parameters used by PdoStore Properly handle phpunit arguments for configuration file [Mailer] add tests for http transports
2 parents b19063f + 7e0a993 commit 2e1aa09

File tree

2 files changed

+189
-3
lines changed

2 files changed

+189
-3
lines changed

‎HttpCache/HttpCache.php

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -476,13 +476,37 @@ protected function forward(Request $request, bool $catch = false, Response $entr
476476
// always a "master" request (as the real master request can be in cache)
477477
$response = SubRequestHandler::handle($this->kernel, $request, HttpKernelInterface::MASTER_REQUEST, $catch);
478478

479-
// we don't implement the stale-if-error on Requests, which is nonetheless part of the RFC
480-
if (null !== $entry && \in_array($response->getStatusCode(), [500, 502, 503, 504])) {
479+
/*
480+
* Support stale-if-error given on Responses or as a config option.
481+
* RFC 7234 summarizes in Section 4.2.4 (but also mentions with the individual
482+
* Cache-Control directives) that
483+
*
484+
* A cache MUST NOT generate a stale response if it is prohibited by an
485+
* explicit in-protocol directive (e.g., by a "no-store" or "no-cache"
486+
* cache directive, a "must-revalidate" cache-response-directive, or an
487+
* applicable "s-maxage" or "proxy-revalidate" cache-response-directive;
488+
* see Section 5.2.2).
489+
*
490+
* https://tools.ietf.org/html/rfc7234#section-4.2.4
491+
*
492+
* We deviate from this in one detail, namely that we *do* serve entries in the
493+
* stale-if-error case even if they have a `s-maxage` Cache-Control directive.
494+
*/
495+
if (null !== $entry
496+
&& \in_array($response->getStatusCode(), [500, 502, 503, 504])
497+
&& !$entry->headers->hasCacheControlDirective('no-cache')
498+
&& !$entry->mustRevalidate()
499+
) {
481500
if (null === $age = $entry->headers->getCacheControlDirective('stale-if-error')) {
482501
$age = $this->options['stale_if_error'];
483502
}
484503

485-
if (abs($entry->getTtl()) < $age) {
504+
/*
505+
* stale-if-error gives the (extra) time that the Response may be used *after* it has become stale.
506+
* So we compare the time the $entry has been sitting in the cache already with the
507+
* time it was fresh plus the allowed grace period.
508+
*/
509+
if ($entry->getAge() <= $entry->getMaxAge() + $age) {
486510
$this->record($request, 'stale-if-error');
487511

488512
return $entry;

‎Tests/HttpCache/HttpCacheTest.php

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1522,6 +1522,168 @@ public function testUsesOriginalRequestForSurrogate()
15221522
$cache->handle($request, HttpKernelInterface::SUB_REQUEST);
15231523
}
15241524

1525+
public function testStaleIfErrorMustNotResetLifetime()
1526+
{
1527+
// Make sure we don't accidentally treat the response as fresh (revalidated) again
1528+
// when stale-if-error handling kicks in.
1529+
1530+
$responses = [
1531+
[
1532+
'status' => 200,
1533+
'body' => 'OK',
1534+
// This is cacheable and can be used in stale-if-error cases:
1535+
'headers' => ['Cache-Control' => 'public, max-age=10', 'ETag' => 'some-etag'],
1536+
],
1537+
[
1538+
'status' => 500,
1539+
'body' => 'FAIL',
1540+
'headers' => [],
1541+
],
1542+
[
1543+
'status' => 500,
1544+
'body' => 'FAIL',
1545+
'headers' => [],
1546+
],
1547+
];
1548+
1549+
$this->setNextResponses($responses);
1550+
$this->cacheConfig['stale_if_error'] = 10;
1551+
1552+
$this->request('GET', '/'); // warm cache
1553+
1554+
sleep(15); // now the entry is stale, but still within the grace period (10s max-age + 10s stale-if-error)
1555+
1556+
$this->request('GET', '/'); // hit backend error
1557+
$this->assertEquals(200, $this->response->getStatusCode()); // stale-if-error saved the day
1558+
$this->assertEquals(15, $this->response->getAge());
1559+
1560+
sleep(10); // now we're outside the grace period
1561+
1562+
$this->request('GET', '/'); // hit backend error
1563+
$this->assertEquals(500, $this->response->getStatusCode()); // fail
1564+
}
1565+
1566+
/**
1567+
* @dataProvider getResponseDataThatMayBeServedStaleIfError
1568+
*/
1569+
public function testResponsesThatMayBeUsedStaleIfError($responseHeaders, $sleepBetweenRequests = null)
1570+
{
1571+
$responses = [
1572+
[
1573+
'status' => 200,
1574+
'body' => 'OK',
1575+
'headers' => $responseHeaders,
1576+
],
1577+
[
1578+
'status' => 500,
1579+
'body' => 'FAIL',
1580+
'headers' => [],
1581+
],
1582+
];
1583+
1584+
$this->setNextResponses($responses);
1585+
$this->cacheConfig['stale_if_error'] = 10; // after stale, may be served for 10s
1586+
1587+
$this->request('GET', '/'); // warm cache
1588+
1589+
if ($sleepBetweenRequests) {
1590+
sleep($sleepBetweenRequests);
1591+
}
1592+
1593+
$this->request('GET', '/'); // hit backend error
1594+
1595+
$this->assertEquals(200, $this->response->getStatusCode());
1596+
$this->assertEquals('OK', $this->response->getContent());
1597+
$this->assertTraceContains('stale-if-error');
1598+
}
1599+
1600+
public function getResponseDataThatMayBeServedStaleIfError()
1601+
{
1602+
// All data sets assume that a 10s stale-if-error grace period has been configured
1603+
yield 'public, max-age expired' => [['Cache-Control' => 'public, max-age=60'], 65];
1604+
yield 'public, validateable with ETag, no TTL' => [['Cache-Control' => 'public', 'ETag' => 'some-etag'], 5];
1605+
yield 'public, validateable with Last-Modified, no TTL' => [['Cache-Control' => 'public', 'Last-Modified' => 'yesterday'], 5];
1606+
yield 'public, s-maxage will be served stale-if-error, even if the RFC mandates otherwise' => [['Cache-Control' => 'public, s-maxage=20'], 25];
1607+
}
1608+
1609+
/**
1610+
* @dataProvider getResponseDataThatMustNotBeServedStaleIfError
1611+
*/
1612+
public function testResponsesThatMustNotBeUsedStaleIfError($responseHeaders, $sleepBetweenRequests = null)
1613+
{
1614+
$responses = [
1615+
[
1616+
'status' => 200,
1617+
'body' => 'OK',
1618+
'headers' => $responseHeaders,
1619+
],
1620+
[
1621+
'status' => 500,
1622+
'body' => 'FAIL',
1623+
'headers' => [],
1624+
],
1625+
];
1626+
1627+
$this->setNextResponses($responses);
1628+
$this->cacheConfig['stale_if_error'] = 10; // after stale, may be served for 10s
1629+
$this->cacheConfig['strict_smaxage'] = true; // full RFC compliance for this feature
1630+
1631+
$this->request('GET', '/'); // warm cache
1632+
1633+
if ($sleepBetweenRequests) {
1634+
sleep($sleepBetweenRequests);
1635+
}
1636+
1637+
$this->request('GET', '/'); // hit backend error
1638+
1639+
$this->assertEquals(500, $this->response->getStatusCode());
1640+
}
1641+
1642+
public function getResponseDataThatMustNotBeServedStaleIfError()
1643+
{
1644+
// All data sets assume that a 10s stale-if-error grace period has been configured
1645+
yield 'public, no TTL but beyond grace period' => [['Cache-Control' => 'public'], 15];
1646+
yield 'public, validateable with ETag, no TTL but beyond grace period' => [['Cache-Control' => 'public', 'ETag' => 'some-etag'], 15];
1647+
yield 'public, validateable with Last-Modified, no TTL but beyond grace period' => [['Cache-Control' => 'public', 'Last-Modified' => 'yesterday'], 15];
1648+
yield 'public, stale beyond grace period' => [['Cache-Control' => 'public, max-age=10'], 30];
1649+
1650+
// Cache-control values that prohibit serving stale responses or responses without positive validation -
1651+
// see https://tools.ietf.org/html/rfc7234#section-4.2.4 and
1652+
// https://tools.ietf.org/html/rfc7234#section-5.2.2
1653+
yield 'no-cache requires positive validation' => [['Cache-Control' => 'public, no-cache', 'ETag' => 'some-etag']];
1654+
yield 'no-cache requires positive validation, even if fresh' => [['Cache-Control' => 'public, no-cache, max-age=10']];
1655+
yield 'must-revalidate requires positive validation once stale' => [['Cache-Control' => 'public, max-age=10, must-revalidate'], 15];
1656+
yield 'proxy-revalidate requires positive validation once stale' => [['Cache-Control' => 'public, max-age=10, proxy-revalidate'], 15];
1657+
}
1658+
1659+
public function testStaleIfErrorWhenStrictSmaxageDisabled()
1660+
{
1661+
$responses = [
1662+
[
1663+
'status' => 200,
1664+
'body' => 'OK',
1665+
'headers' => ['Cache-Control' => 'public, s-maxage=20'],
1666+
],
1667+
[
1668+
'status' => 500,
1669+
'body' => 'FAIL',
1670+
'headers' => [],
1671+
],
1672+
];
1673+
1674+
$this->setNextResponses($responses);
1675+
$this->cacheConfig['stale_if_error'] = 10;
1676+
$this->cacheConfig['strict_smaxage'] = false;
1677+
1678+
$this->request('GET', '/'); // warm cache
1679+
sleep(25);
1680+
$this->request('GET', '/'); // hit backend error
1681+
1682+
$this->assertEquals(200, $this->response->getStatusCode());
1683+
$this->assertEquals('OK', $this->response->getContent());
1684+
$this->assertTraceContains('stale-if-error');
1685+
}
1686+
15251687
public function testTraceHeaderNameCanBeChanged()
15261688
{
15271689
$this->cacheConfig['trace_header'] = 'X-My-Header';

0 commit comments

Comments
 (0)