Description
| Q | A |
|---|---|
| Bug report? | yes |
| Feature request? | no |
| BC Break report? | no |
| RFC? | no |
| Symfony version | 2.8.14 |
We have unexpected behaviour which frequently occurs when developing locally where if an exception is introduced into a Twig page, a (previously valid) stale version of the twig page is served. This is because the Symfony cache implements stale-if-error, which is implemented here:
symfony/src/Symfony/Component/HttpKernel/HttpCache/HttpCache.php
Lines 466 to 481 in 776b964
My understanding of this, particularly line 476, is that stale content will only be served if the ttl of the previously served content is less than 60 (which is the default, as per the below):
The problem appears to be that we're not setting a max age (our intention is that the decision as to whether something is up to date or not is purely ETag based) and so the ttl is always null.
According to the RFC (https://tools.ietf.org/html/rfc5861#page-3):
Its value indicates the upper limit to staleness; when the cached response is more stale than the indicated amount, the cached response SHOULD NOT be used to satisfy the request, absent other information.
As abs(null) < 60, line 476 will always be true, this means we will always serve stale content if an exception occurs.
Is this the correct behaviour?
I've implemented a test below which help to illustrate what I'd expect the behaviour to be as I understand it regarding the abs(null) and normal behaviour if max-age is defined - I couldn't see any obvious tests that exercise this currently and I'm happy to open a PR if they're useful.
public function testMaxAgeHeaderMissing()
{
$this->setNextResponses([
[
'body' => "Hello World",
'status' => 200,
'headers' => [
'Cache-Control' => 'public',
'ETag' => '"12345"',
]
],
[
'body' => "Failure",
'status' => 500,
'headers' => [
]
]
]);
// build initial request
$this->request('GET', '/');
$this->assertHttpKernelIsCalled();
$this->assertEquals(200, $this->response->getStatusCode());
$this->assertNotNull($this->response->headers->get('ETag'));
$this->assertNotNull($this->response->headers->get('X-Content-Digest'));
$this->assertEquals('Hello World', $this->response->getContent());
$this->assertTraceContains('miss');
$this->assertTraceContains('store');
// build subsequent request; exception should be served because there's no max-age
$this->request('GET', '/');
$this->assertHttpKernelIsCalled();
$this->assertEquals(500, $this->response->getStatusCode());
$this->assertNull($this->response->headers->get('ETag'));
$this->assertNull($this->response->headers->get('X-Content-Digest'));
$this->assertEquals('Failure', $this->response->getContent());
$this->assertTraceContains('stale');
$this->assertTraceContains('invalid');
}
public function testServedStaleFromCacheUntilMaxAgePlusStaleIfErrorDefault()
{
$this->setNextResponses([
[
'body' => "Hello World",
'status' => 200,
'headers' => [
'Cache-Control' => 'public, max-age=120',
'ETag' => '"12345"',
]
],
// All subsequent requests are going to fail and we're testing if they're served from cache
[
'body' => "Failure",
'status' => 500,
'headers' => [
]
],
[
'body' => "Failure",
'status' => 500,
'headers' => [
]
],
[
'body' => "Failure",
'status' => 500,
'headers' => [
]
]
]);
$start = time();
// build initial request. This will populate the cache
$this->request('GET', '/');
$this->assertHttpKernelIsCalled();
$this->assertEquals(200, $this->response->getStatusCode());
$this->assertNotNull($this->response->headers->get('ETag'));
$this->assertNotNull($this->response->headers->get('X-Content-Digest'));
$this->assertEquals('Hello World', $this->response->getContent());
$this->assertTraceContains('miss');
$this->assertTraceContains('store');
// First failed request will be served from cache as it's inside the Max Age.
$this->request('GET', '/');
$this->assertHttpKernelIsNotCalled(); // will serve from cache
$this->assertEquals(200, $this->response->getStatusCode());
$this->assertNotNull($this->response->headers->get('ETag'));
$this->assertNotNull($this->response->headers->get('X-Content-Digest'));
$this->assertLessThan(120, $this->response->headers->get("age"));
$this->assertEquals('Hello World', $this->response->getContent());
$this->assertTraceContains('fresh');
// expires the cache
$values = $this->getMetaStorageValues();
$this->assertCount(1, $values);
$tmp = unserialize($values[0]);
// max age + 50 of the 60 stale seconds
$time = \DateTime::createFromFormat('U', $start - 130); // longer than max-age, but less than max-age + 60 which is the default
$tmp[0][1]['date'] = $time->format(DATE_RFC2822);
$r = new \ReflectionObject($this->store);
$m = $r->getMethod('save');
$m->setAccessible(true);
$m->invoke($this->store, 'md'.hash('sha256', 'http://localhost/'), serialize($tmp));
// build subsequent request; should be served from cache because stale-if-error adds 60 seconds to max-age
$this->request('GET', '/');
$this->assertHttpKernelIsCalled(); // Will attempt to rebuild content
$this->assertEquals(200, $this->response->getStatusCode());
$this->assertNotNull($this->response->headers->get('ETag'));
$this->assertNotNull($this->response->headers->get('X-Content-Digest'));
// The returned content should be ~130 seconds old
$this->assertLessThan(120+60, $this->response->headers->get("age"));
$this->assertGreaterThan(120, $this->response->headers->get("age"));
$this->assertEquals('Hello World', $this->response->getContent());
$this->assertTraceContains('stale');
$this->assertTraceContains('stale-if-error');
$this->assertTraceContains('invalid');
$this->assertTraceContains('store');
// expires the cache a bit more
$values = $this->getMetaStorageValues();
$this->assertCount(1, $values);
$tmp = unserialize($values[0]);
// max age + 50 of the 60 stale seconds
$time = \DateTime::createFromFormat('U', $start - 190); // longer than max-age + 60
$tmp[0][1]['date'] = $time->format(DATE_RFC2822);
$r = new \ReflectionObject($this->store);
$m = $r->getMethod('save');
$m->setAccessible(true);
$m->invoke($this->store, 'md'.hash('sha256', 'http://localhost/'), serialize($tmp));
// build subsequent request; it's too long since the cache expired. We can't serve the stale content
$this->request('GET', '/');
$this->assertHttpKernelIsCalled();
$this->assertEquals(500, $this->response->getStatusCode());
$this->assertNull($this->response->headers->get('ETag'));
$this->assertNull($this->response->headers->get('X-Content-Digest'));
$this->assertEquals('Failure', $this->response->getContent());
$this->assertTraceContains('stale');
$this->assertTraceContains('invalid');
}