From 532160026fb98f7ec7bd83c740d5500ba5994d19 Mon Sep 17 00:00:00 2001 From: Jason Desrosiers Date: Sun, 19 May 2013 11:41:10 -0700 Subject: [PATCH] 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. --- .../Component/HttpKernel/HttpCache/HttpCache.php | 13 +++++++++++-- .../Component/HttpKernel/HttpCache/Store.php | 9 --------- .../HttpKernel/Tests/HttpCache/HttpCacheTest.php | 6 +++--- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/Symfony/Component/HttpKernel/HttpCache/HttpCache.php b/src/Symfony/Component/HttpKernel/HttpCache/HttpCache.php index 2592d3b171..e3d8c29fe6 100644 --- a/src/Symfony/Component/HttpKernel/HttpCache/HttpCache.php +++ b/src/Symfony/Component/HttpKernel/HttpCache/HttpCache.php @@ -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'); diff --git a/src/Symfony/Component/HttpKernel/HttpCache/Store.php b/src/Symfony/Component/HttpKernel/HttpCache/Store.php index f7d4e2f1c3..b2caff64cc 100644 --- a/src/Symfony/Component/HttpKernel/HttpCache/Store.php +++ b/src/Symfony/Component/HttpKernel/HttpCache/Store.php @@ -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); - } - } } /** diff --git a/src/Symfony/Component/HttpKernel/Tests/HttpCache/HttpCacheTest.php b/src/Symfony/Component/HttpKernel/Tests/HttpCache/HttpCacheTest.php index 448ebc362b..a8064b832b 100644 --- a/src/Symfony/Component/HttpKernel/Tests/HttpCache/HttpCacheTest.php +++ b/src/Symfony/Component/HttpKernel/Tests/HttpCache/HttpCacheTest.php @@ -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');