Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 05933f3

Browse filesBrowse files
committed
minor #22033 Minor cleanups in HttpCache, especially centralize Date header fixup (mpdude)
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 ------- 30639c6 Minor cleanups in HttpCache, especially centralize Date header fixup
2 parents 0a5998d + 30639c6 commit 05933f3
Copy full SHA for 05933f3

File tree

Expand file treeCollapse file tree

1 file changed

+38
-21
lines changed
Filter options
Expand file treeCollapse file tree

1 file changed

+38
-21
lines changed

‎src/Symfony/Component/HttpKernel/HttpCache/HttpCache.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/HttpKernel/HttpCache/HttpCache.php
+38-21Lines changed: 38 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -176,24 +176,25 @@ public function handle(Request $request, $type = HttpKernelInterface::MASTER_REQ
176176
}
177177
}
178178

179-
$path = $request->getPathInfo();
180-
if ($qs = $request->getQueryString()) {
181-
$path .= '?'.$qs;
182-
}
183-
$this->traces[$request->getMethod().' '.$path] = array();
179+
$this->traces[$this->getTraceKey($request)] = array();
184180

185181
if (!$request->isMethodSafe(false)) {
186182
$response = $this->invalidate($request, $catch);
187183
} elseif ($request->headers->has('expect') || !$request->isMethodCacheable()) {
188184
$response = $this->pass($request, $catch);
185+
} elseif ($this->options['allow_reload'] && $request->isNoCache()) {
186+
/*
187+
If allow_reload is configured and the client requests "Cache-Control: no-cache",
188+
reload the cache by fetching a fresh response and caching it (if possible).
189+
*/
190+
$this->record($request, 'reload');
191+
$response = $this->fetch($request, $catch);
189192
} else {
190193
$response = $this->lookup($request, $catch);
191194
}
192195

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

195-
$response->setDate(\DateTime::createFromFormat('U', time(), new \DateTimeZone('UTC')));
196-
197198
if (HttpKernelInterface::MASTER_REQUEST === $type && $this->options['debug']) {
198199
$response->headers->set('X-Symfony-Cache', $this->getLog());
199200
}
@@ -299,13 +300,6 @@ protected function invalidate(Request $request, $catch = false)
299300
*/
300301
protected function lookup(Request $request, $catch = false)
301302
{
302-
// if allow_reload and no-cache Cache-Control, allow a cache reload
303-
if ($this->options['allow_reload'] && $request->isNoCache()) {
304-
$this->record($request, 'reload');
305-
306-
return $this->fetch($request, $catch);
307-
}
308-
309303
try {
310304
$entry = $this->store->lookup($request);
311305
} catch (\Exception $e) {
@@ -403,9 +397,8 @@ protected function validate(Request $request, Response $entry, $catch = false)
403397
}
404398

405399
/**
406-
* Forwards the Request to the backend and determines whether the response should be stored.
407-
*
408-
* This methods is triggered when the cache missed or a reload is required.
400+
* Unconditionally fetches a fresh response from the backend and
401+
* stores it in the cache if is cacheable.
409402
*
410403
* @param Request $request A Request instance
411404
* @param bool $catch whether to process exceptions
@@ -437,6 +430,9 @@ protected function fetch(Request $request, $catch = false)
437430
/**
438431
* Forwards the Request to the backend and returns the Response.
439432
*
433+
* All backend requests (cache passes, fetches, cache validations)
434+
* run through this method.
435+
*
440436
* @param Request $request A Request instance
441437
* @param bool $catch Whether to catch exceptions or not
442438
* @param Response $entry A Response instance (the stale entry if present, null otherwise)
@@ -484,6 +480,17 @@ protected function forward(Request $request, $catch = false, Response $entry = n
484480
}
485481
}
486482

483+
/*
484+
RFC 7231 Sect. 7.1.1.2 says that a server that does not have a reasonably accurate
485+
clock MUST NOT send a "Date" header, although it MUST send one in most other cases
486+
except for 1xx or 5xx responses where it MAY do so.
487+
488+
Anyway, a client that received a message without a "Date" header MUST add it.
489+
*/
490+
if (!$response->headers->has('Date')) {
491+
$response->setDate(\DateTime::createFromFormat('U', time()));
492+
}
493+
487494
$this->processResponseBody($request, $response);
488495

489496
if ($this->isPrivateRequest($request) && !$response->headers->hasCacheControlDirective('public')) {
@@ -584,9 +591,6 @@ protected function lock(Request $request, Response $entry)
584591
*/
585592
protected function store(Request $request, Response $response)
586593
{
587-
if (!$response->headers->has('Date')) {
588-
$response->setDate(\DateTime::createFromFormat('U', time()));
589-
}
590594
try {
591595
$this->store->write($request, $response);
592596

@@ -683,11 +687,24 @@ private function isPrivateRequest(Request $request)
683687
* @param string $event The event name
684688
*/
685689
private function record(Request $request, $event)
690+
{
691+
$this->traces[$this->getTraceKey($request)][] = $event;
692+
}
693+
694+
/**
695+
* Calculates the key we use in the "trace" array for a given request.
696+
*
697+
* @param Request $request
698+
*
699+
* @return string
700+
*/
701+
private function getTraceKey(Request $request)
686702
{
687703
$path = $request->getPathInfo();
688704
if ($qs = $request->getQueryString()) {
689705
$path .= '?'.$qs;
690706
}
691-
$this->traces[$request->getMethod().' '.$path][] = $event;
707+
708+
return $request->getMethod().' '.$path;
692709
}
693710
}

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.