diogo/symfony
Archived
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

minor #22033 Minor cleanups in HttpCache, especially centralize Date header fixup (mpdude)

Browse Source 
This PR was submitted for the 2.8 branch but it was merged into the 3.3-dev branch instead (closes #22033).

Discussion
----------

Minor cleanups in HttpCache, especially centralize Date header fixup

| Q             | A
| ------------- | ---
| Branch?       | 2.8
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets |
| License       | MIT
| Doc PR        |

I am trying to hunt some issues where the HTTP cache does not revalidate responses (or does not use a cached response?) in the edge case of having `s-maxage = 0` and possibly a backend that's slow to respond.

I don't see yet what exactly my problem is there, but it must somehow be related with the `Date` header handling and/or age calculations in the `HTTPCache`.

As a first step, I'd like to suggest this cleanup in `HTTPCache`. Especially, it would be helpful if we could make sure that there is just one place where we set the `Date` header if the backend does not send one (?). This makes sure we always have this header to base later age calculations on it.

Commits
-------

30639c6af6 Minor cleanups in HttpCache, especially centralize Date header fixup
This commit is contained in:
Fabien Potencier 2017-03-22 15:30:57 -07:00
commit 05933f3749
1 changed files with 38 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

59
src/Symfony/Component/HttpKernel/HttpCache/HttpCache.php
View File

@ -176,24 +176,25 @@ class HttpCache implements HttpKernelInterface, TerminableInterface
}
}
$path = $request->getPathInfo();
if ($qs = $request->getQueryString()) {
$path .= '?'.$qs;
}
$this->traces[$request->getMethod().' '.$path] = array();
$this->traces[$this->getTraceKey($request)] = array();
if (!$request->isMethodSafe(false)) {
$response = $this->invalidate($request, $catch);
} elseif ($request->headers->has('expect') || !$request->isMethodCacheable()) {
$response = $this->pass($request, $catch);
} elseif ($this->options['allow_reload'] && $request->isNoCache()) {
/*
If allow_reload is configured and the client requests "Cache-Control: no-cache",
reload the cache by fetching a fresh response and caching it (if possible).
*/
$this->record($request, 'reload');
$response = $this->fetch($request, $catch);
} else {
$response = $this->lookup($request, $catch);
}
$this->restoreResponseBody($request, $response);
$response->setDate(\DateTime::createFromFormat('U', time(), new \DateTimeZone('UTC')));
if (HttpKernelInterface::MASTER_REQUEST === $type && $this->options['debug']) {
$response->headers->set('X-Symfony-Cache', $this->getLog());
}
@ -299,13 +300,6 @@ class HttpCache implements HttpKernelInterface, TerminableInterface
*/
protected function lookup(Request $request, $catch = false)
{
// if allow_reload and no-cache Cache-Control, allow a cache reload
if ($this->options['allow_reload'] && $request->isNoCache()) {
$this->record($request, 'reload');
return $this->fetch($request, $catch);
}
try {
$entry = $this->store->lookup($request);
} catch (\Exception $e) {
@ -403,9 +397,8 @@ class HttpCache implements HttpKernelInterface, TerminableInterface
}
/**
* Forwards the Request to the backend and determines whether the response should be stored.
*
* This methods is triggered when the cache missed or a reload is required.
* Unconditionally fetches a fresh response from the backend and
* stores it in the cache if is cacheable.
*
* @param Request $request A Request instance
* @param bool $catch whether to process exceptions
@ -437,6 +430,9 @@ class HttpCache implements HttpKernelInterface, TerminableInterface
/**
* Forwards the Request to the backend and returns the Response.
*
* All backend requests (cache passes, fetches, cache validations)
* run through this method.
*
* @param Request $request A Request instance
* @param bool $catch Whether to catch exceptions or not
* @param Response $entry A Response instance (the stale entry if present, null otherwise)
@ -484,6 +480,17 @@ class HttpCache implements HttpKernelInterface, TerminableInterface
}
}
/*
RFC 7231 Sect. 7.1.1.2 says that a server that does not have a reasonably accurate
clock MUST NOT send a "Date" header, although it MUST send one in most other cases
except for 1xx or 5xx responses where it MAY do so.
Anyway, a client that received a message without a "Date" header MUST add it.
*/
if (!$response->headers->has('Date')) {
$response->setDate(\DateTime::createFromFormat('U', time()));
}
$this->processResponseBody($request, $response);
if ($this->isPrivateRequest($request) && !$response->headers->hasCacheControlDirective('public')) {
@ -584,9 +591,6 @@ class HttpCache implements HttpKernelInterface, TerminableInterface
*/
protected function store(Request $request, Response $response)
{
if (!$response->headers->has('Date')) {
$response->setDate(\DateTime::createFromFormat('U', time()));
}
try {
$this->store->write($request, $response);
@ -683,11 +687,24 @@ class HttpCache implements HttpKernelInterface, TerminableInterface
* @param string $event The event name
*/
private function record(Request $request, $event)
{
$this->traces[$this->getTraceKey($request)][] = $event;
}
/**
* Calculates the key we use in the "trace" array for a given request.
*
* @param Request $request
*
* @return string
*/
private function getTraceKey(Request $request)
{
$path = $request->getPathInfo();
if ($qs = $request->getQueryString()) {
$path .= '?'.$qs;
}
$this->traces[$request->getMethod().' '.$path][] = $event;
return $request->getMethod().' '.$path;
}
}