Skip to content

Commit

Permalink
Fixed two bugs in HttpCache
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jdesrosiers authored and fabpot committed May 20, 2013
1 parent 92399ff commit 5321600
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 14 deletions.
13 changes: 11 additions & 2 deletions src/Symfony/Component/HttpKernel/HttpCache/HttpCache.php
Expand Up @@ -193,8 +193,6 @@ public function handle(Request $request, $type = HttpKernelInterface::MASTER_REQ
$response = $this->lookup($request, $catch);
}

$response->isNotModified($request);

$this->restoreResponseBody($request, $response);

$response->setDate(new \DateTime(null, new \DateTimeZone('UTC')));
Expand All @@ -213,6 +211,8 @@ public function handle(Request $request, $type = HttpKernelInterface::MASTER_REQ

$response->prepare($request);

$response->isNotModified($request);

return $response;
}

Expand Down Expand Up @@ -262,6 +262,15 @@ protected function invalidate(Request $request, $catch = false)
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');
Expand Down
9 changes: 0 additions & 9 deletions src/Symfony/Component/HttpKernel/HttpCache/Store.php
Expand Up @@ -231,15 +231,6 @@ public function invalidate(Request $request)
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);
}
}
}

/**
Expand Down
Expand Up @@ -145,7 +145,7 @@ public function testRespondsWith304WhenIfModifiedSinceMatchesLastModified()

$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');
Expand All @@ -158,7 +158,7 @@ public function testRespondsWith304WhenIfNoneMatchMatchesETag()

$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');
Expand Down Expand Up @@ -845,7 +845,7 @@ public function testInvalidatesCachedResponsesOnPost()
$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');
Expand Down

0 comments on commit 5321600

Please sign in to comment.