Fixed two bugs in HttpCache
1. 304 responses always send "Content-Type: text/html; charset=UTF-8" header I discovered that the HttpCache::handle method calls Response::prepare after calling Response::isModified. Response::isModified removes the Content-Type header as it should, but Response::handle adds in the default Content-Type header when none is set. If the default Content-Type is not the correct Content-Type, then the Content-Type in the cache gets clobered. I solved this problem by moving the Response::isModified call after the Response::prepare call. I updated the testRespondsWith304WhenIfModifiedSinceMatchesLastModified and testRespondsWith304WhenIfNoneMatchMatchesETag tests to verify that the Content-Type header was not being sent for 304 responses. 2. Failure to invalidate cached entities referred to by the Location header I discovered that the Store::invalidate method was looking for Location and Content-Location headers to invalidate, but it was looking in the request headers instead of the response headers. Because the Store::invalidate method doesn't take a response, I decided it was better to move this logic to the HttpCache::invalidate method instead. I updated the testInvalidatesCachedResponsesOnPost test to verify that Location headers are getting invalidated correctly.
This commit is contained in:
parent
92399ff79a
commit
532160026f
@ -193,8 +193,6 @@ class HttpCache implements HttpKernelInterface, TerminableInterface
|
||||
$response = $this->lookup($request, $catch);
|
||||
}
|
||||
|
||||
$response->isNotModified($request);
|
||||
|
||||
$this->restoreResponseBody($request, $response);
|
||||
|
||||
$response->setDate(new \DateTime(null, new \DateTimeZone('UTC')));
|
||||
@ -213,6 +211,8 @@ class HttpCache implements HttpKernelInterface, TerminableInterface
|
||||
|
||||
$response->prepare($request);
|
||||
|
||||
$response->isNotModified($request);
|
||||
|
||||
return $response;
|
||||
}
|
||||
|
||||
@ -262,6 +262,15 @@ class HttpCache implements HttpKernelInterface, TerminableInterface
|
||||
try {
|
||||
$this->store->invalidate($request, $catch);
|
||||
|
||||
// As per the RFC, invalidate Location and Content-Location URLs if present
|
||||
foreach (array('Location', 'Content-Location') as $header) {
|
||||
if ($uri = $response->headers->get($header)) {
|
||||
$subRequest = $request::create($uri, 'get', array(), array(), array(), $request->server->all());
|
||||
|
||||
$this->store->invalidate($subRequest);
|
||||
}
|
||||
}
|
||||
|
||||
$this->record($request, 'invalidate');
|
||||
} catch (\Exception $e) {
|
||||
$this->record($request, 'invalidate-failed');
|
||||
|
@ -231,15 +231,6 @@ class Store implements StoreInterface
|
||||
throw new \RuntimeException('Unable to store the metadata.');
|
||||
}
|
||||
}
|
||||
|
||||
// As per the RFC, invalidate Location and Content-Location URLs if present
|
||||
foreach (array('Location', 'Content-Location') as $header) {
|
||||
if ($uri = $request->headers->get($header)) {
|
||||
$subRequest = $request::create($uri, 'get', array(), array(), array(), $request->server->all());
|
||||
|
||||
$this->invalidate($subRequest);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
|
@ -145,7 +145,7 @@ class HttpCacheTest extends HttpCacheTestCase
|
||||
|
||||
$this->assertHttpKernelIsCalled();
|
||||
$this->assertEquals(304, $this->response->getStatusCode());
|
||||
$this->assertEquals('text/html; charset=UTF-8', $this->response->headers->get('Content-Type'));
|
||||
$this->assertEquals('', $this->response->headers->get('Content-Type'));
|
||||
$this->assertEmpty($this->response->getContent());
|
||||
$this->assertTraceContains('miss');
|
||||
$this->assertTraceContains('store');
|
||||
@ -158,7 +158,7 @@ class HttpCacheTest extends HttpCacheTestCase
|
||||
|
||||
$this->assertHttpKernelIsCalled();
|
||||
$this->assertEquals(304, $this->response->getStatusCode());
|
||||
$this->assertEquals('text/html; charset=UTF-8', $this->response->headers->get('Content-Type'));
|
||||
$this->assertEquals('', $this->response->headers->get('Content-Type'));
|
||||
$this->assertTrue($this->response->headers->has('ETag'));
|
||||
$this->assertEmpty($this->response->getContent());
|
||||
$this->assertTraceContains('miss');
|
||||
@ -845,7 +845,7 @@ class HttpCacheTest extends HttpCacheTestCase
|
||||
$this->assertTraceContains('fresh');
|
||||
|
||||
// now POST to same URL
|
||||
$this->request('POST', '/');
|
||||
$this->request('POST', '/helloworld');
|
||||
$this->assertHttpKernelIsCalled();
|
||||
$this->assertEquals('/', $this->response->headers->get('Location'));
|
||||
$this->assertTraceContains('invalidate');
|
||||
|
Reference in New Issue
Block a user