From 19e6cd180a0d6012f59be73c38c7ab9d63a2d52d Mon Sep 17 00:00:00 2001 From: Alexandre Quercia Date: Wed, 24 Oct 2018 08:59:59 +0200 Subject: [PATCH] Fix compatibility with HTTP specs about cacheable status codes and methods - Add pages with response 200, 203, 300, 301, 302, 404, 410 to cache. - Add to cache response from HEAD method request. --- lib/controller/sfController.class.php | 16 +++-- lib/controller/sfWebController.class.php | 3 +- lib/filter/sfCacheFilter.class.php | 82 +++++++++++++++++++++++- lib/filter/sfExecutionFilter.class.php | 30 +++++++-- lib/filter/sfRenderingFilter.class.php | 26 +++++++- lib/response/sfWebResponse.class.php | 28 ++++++++ lib/test/sfTesterViewCache.class.php | 13 ++-- lib/util/sfBrowser.class.php | 22 ++++++- lib/view/sfView.class.php | 5 ++ lib/view/sfViewCacheManager.class.php | 45 +++++++------ 10 files changed, 229 insertions(+), 41 deletions(-) diff --git a/lib/controller/sfController.class.php b/lib/controller/sfController.class.php index 1d1deec76..717cba2a5 100644 --- a/lib/controller/sfController.class.php +++ b/lib/controller/sfController.class.php @@ -331,6 +331,8 @@ public function getActionStack() * @return int One of the following: * - sfView::RENDER_CLIENT * - sfView::RENDER_VAR + * - sfView::RENDER_NONE + * - sfView::RENDER_REDIRECTION */ public function getRenderMode() { @@ -472,6 +474,7 @@ public function getPresentationFor($module, $action, $viewName = null) * - sfView::RENDER_CLIENT * - sfView::RENDER_VAR * - sfView::RENDER_NONE + * - sfView::RENDER_REDIRECTION * * @return void * @@ -479,11 +482,16 @@ public function getPresentationFor($module, $action, $viewName = null) */ public function setRenderMode($mode) { - if ($mode == sfView::RENDER_CLIENT || $mode == sfView::RENDER_VAR || $mode == sfView::RENDER_NONE) + switch ($mode) { - $this->renderMode = $mode; - - return; + case sfView::RENDER_CLIENT: + case sfView::RENDER_VAR: + case sfView::RENDER_NONE: + case sfView::RENDER_REDIRECTION: + $this->renderMode = $mode; + return; + default: + break; } // invalid rendering mode type diff --git a/lib/controller/sfWebController.class.php b/lib/controller/sfWebController.class.php index b7eb9f466..aba0a3441 100644 --- a/lib/controller/sfWebController.class.php +++ b/lib/controller/sfWebController.class.php @@ -205,6 +205,7 @@ public function redirect($url, $delay = 0, $statusCode = 302) } $response->setContent(sprintf('', $delay, htmlspecialchars($url, ENT_QUOTES, sfConfig::get('sf_charset')))); - $response->send(); + + $this->setRenderMode(sfView::RENDER_REDIRECTION); } } diff --git a/lib/filter/sfCacheFilter.class.php b/lib/filter/sfCacheFilter.class.php index 9d0420a3b..5fc3eb98d 100644 --- a/lib/filter/sfCacheFilter.class.php +++ b/lib/filter/sfCacheFilter.class.php @@ -25,6 +25,23 @@ class sfCacheFilter extends sfFilter $routing = null, $cache = array(); + /** + * Responses with its status codes may safely be kept in a shared (surrogate) cache. + * + * Put status codes as key in ordder to be able to use `isset()`. + * + * @var array + */ + private $cacheableStatusCodes = array( + 200 => true, + 203 => true, + 300 => true, + 301 => true, + 302 => true, + 404 => true, + 410 => true, + ); + /** * Initializes this Filter. * @@ -60,12 +77,30 @@ public function execute($filterChain) return; } + $exception = null; + if ($this->executeBeforeExecution()) { - $filterChain->execute(); + try + { + // execute next filter + $filterChain->execute(); + } + catch (sfStopException $exception) + { + if (sfView::RENDER_REDIRECTION !== $this->context->getController()->getRenderMode()) + { + throw $exception; + } + } } $this->executeBeforeRendering(); + + if (null !== $exception) + { + throw $exception; + } } public function executeBeforeExecution() @@ -102,8 +137,7 @@ public function executeBeforeExecution() */ public function executeBeforeRendering() { - // cache only 200 HTTP status - if (200 != $this->response->getStatusCode()) + if (!$this->isCacheableResponse($this->response)) { return; } @@ -224,4 +258,46 @@ protected function checkCacheValidation() } } } + + /** + * Returns true if the response may safely be kept in a shared (surrogate) cache. + * + * Responses marked "private" with an explicit Cache-Control directive are + * considered uncacheable. + * + * Responses with neither a freshness lifetime (Expires, max-age) nor cache + * validator (Last-Modified, ETag) are considered uncacheable because there is + * no way to tell when or how to remove them from the cache. + * + * Note that RFC 7231 and RFC 7234 possibly allow for a more permissive implementation, + * for example "status codes that are defined as cacheable by default [...] + * can be reused by a cache with heuristic expiration unless otherwise indicated" + * (https://tools.ietf.org/html/rfc7231#section-6.1) + * + * @param sfWebResponse $response + * + * @return bool + * + * @see https://github.com/symfony/symfony/blob/v4.1.6/src/Symfony/Component/HttpFoundation/Response.php#L523 + */ + protected function isCacheableResponse($response) + { + if (!$response instanceof sfWebResponse) + { + return false; + } + + if (!isset($this->cacheableStatusCodes[$response->getStatusCode()])) + { + return false; + } + + if ($response->isPrivate()) + { + return false; + } + + // Cache validation and expiration headers are always sets before save on cache. + return true /* $this->isValidateable() || $this->isFresh() */; + } } diff --git a/lib/filter/sfExecutionFilter.class.php b/lib/filter/sfExecutionFilter.class.php index 828c04db2..994fc0581 100644 --- a/lib/filter/sfExecutionFilter.class.php +++ b/lib/filter/sfExecutionFilter.class.php @@ -88,10 +88,32 @@ protected function handleAction($filterChain, $actionInstance) */ protected function executeAction($actionInstance) { - // execute the action - $actionInstance->preExecute(); - $viewName = $actionInstance->execute($this->context->getRequest()); - $actionInstance->postExecute(); + try { + // execute the action + $actionInstance->preExecute(); + + $viewName = $actionInstance->execute($this->context->getRequest()); + + $actionInstance->postExecute(); + } catch (sfStopException $e) { + if (!sfConfig::get('sf_cache')) { + throw $e; + } + + if (sfView::RENDER_REDIRECTION === $this->context->getController()->getRenderMode()) + { + $viewCache = $this->context->getViewCacheManager(); + $response = $this->context->getResponse(); + $uri = $viewCache->getCurrentCacheKey(); + + if (null !== $uri) + { + $viewCache->setActionCache($uri, $response->getContent(), false); + } + } + + throw $e; + } return null === $viewName ? sfView::SUCCESS : $viewName; } diff --git a/lib/filter/sfRenderingFilter.class.php b/lib/filter/sfRenderingFilter.class.php index e77001101..32b992120 100644 --- a/lib/filter/sfRenderingFilter.class.php +++ b/lib/filter/sfRenderingFilter.class.php @@ -29,8 +29,22 @@ class sfRenderingFilter extends sfFilter */ public function execute($filterChain) { + $controller = $this->context->getController(); + $exception = null; + // execute next filter - $filterChain->execute(); + try + { + $filterChain->execute(); + } + catch (sfStopException $exception) + { + // Send the response when stop the execution for a redirection. + if (sfView::RENDER_REDIRECTION !== $controller->getRenderMode()) + { + throw $exception; + } + } // get response object $response = $this->context->getResponse(); @@ -46,9 +60,15 @@ public function execute($filterChain) } // send headers + content - if (sfView::RENDER_VAR != $this->context->getController()->getRenderMode()) + if (sfView::RENDER_VAR != $controller->getRenderMode()) + { + $response->send(); + } + + // Re-throw the exception to keep the encapsulation. + if (null !== $exception) { - $response->send(); + throw $exception; } } } diff --git a/lib/response/sfWebResponse.class.php b/lib/response/sfWebResponse.class.php index 4a9a6585a..a3c7c1910 100644 --- a/lib/response/sfWebResponse.class.php +++ b/lib/response/sfWebResponse.class.php @@ -84,6 +84,16 @@ class sfWebResponse extends sfResponse '505' => 'HTTP Version Not Supported', ); + /** + * A list of cache control private directives. + * + * @var array + */ + protected $cacheControlPrivateDirectives = array( + 'no-store', + 'private', + ); + /** * Initializes this sfWebResponse. * @@ -843,6 +853,9 @@ public function clearHttpHeaders() public function copyProperties(sfWebResponse $response) { $this->options = $response->getOptions(); + $this->statusCode = $response->getStatusCode(); + $this->statusText = $response->getStatusText(); + $this->headerOnly = $response->isHeaderOnly(); $this->headers = $response->getHttpHeaders(); $this->metas = $response->getMetas(); $this->httpMetas = $response->getHttpMetas(); @@ -888,6 +901,21 @@ public function unserialize($serialized) list($this->content, $this->statusCode, $this->statusText, $this->options, $this->headerOnly, $this->headers, $this->metas, $this->httpMetas, $this->stylesheets, $this->javascripts, $this->slots) = unserialize($serialized); } + /** + * Checks whether the response contains a private drective on cache control. + * + * @return bool + */ + public function isPrivate() + { + $privateDirectives = $this->cacheControlPrivateDirectives; + $cacheControl = $this->getHttpHeader('Cache-Control', ''); + + $cacheControlDirectives = explode(', ', $cacheControl); + + return $privateDirectives !== array_diff($privateDirectives, $cacheControlDirectives); + } + /** * Validate a position name. * diff --git a/lib/test/sfTesterViewCache.class.php b/lib/test/sfTesterViewCache.class.php index a846b31a3..d47b5dc45 100644 --- a/lib/test/sfTesterViewCache.class.php +++ b/lib/test/sfTesterViewCache.class.php @@ -106,21 +106,26 @@ public function isUriCached($uri, $boolean, $with_layout = false) // check that the content is ok in cache if ($boolean) { + $fullContent = $this->response->getContent(); + $withContent = !$this->response->isHeaderOnly() || '' !== $fullContent; + if (!$ret) { $this->tester->fail('content in cache is ok'); } - else if ($with_layout) + else if ($with_layout && $withContent) { $response = unserialize($cacheManager->get($uri)); $content = $response->getContent(); - $this->tester->ok($content == $this->response->getContent(), 'content in cache is ok'); + + $this->tester->is($content, $fullContent, 'content in cache with layout is ok'); } - else + else if ($withContent) { $ret = unserialize($cacheManager->get($uri)); $content = $ret['content']; - $this->tester->ok(false !== strpos($this->response->getContent(), $content), 'content in cache is ok'); + + $this->tester->ok(false !== strpos($fullContent, $content), 'content in cache without layout is ok'); } } } diff --git a/lib/util/sfBrowser.class.php b/lib/util/sfBrowser.class.php index a4f89691c..2734bd553 100644 --- a/lib/util/sfBrowser.class.php +++ b/lib/util/sfBrowser.class.php @@ -162,8 +162,28 @@ class sfFakeRenderingFilter extends sfFilter { public function execute($filterChain) { - $filterChain->execute(); + $controller = $this->context->getController(); + $exception = null; + + try + { + $filterChain->execute(); + } + catch (sfStopException $exception) + { + // Send the response when stop the execution for a redirection. + if (sfView::RENDER_REDIRECTION !== $controller->getRenderMode()) + { + throw $exception; + } + } $this->context->getResponse()->sendContent(); + + // Re-throw the exception to keep the encapsulation. + if (null !== $exception) + { + throw $exception; + } } } diff --git a/lib/view/sfView.class.php b/lib/view/sfView.class.php index 87107d59c..3b23b5da8 100644 --- a/lib/view/sfView.class.php +++ b/lib/view/sfView.class.php @@ -62,6 +62,11 @@ abstract class sfView */ const RENDER_VAR = 4; + /** + * Render the presentation as redirection. + */ + const RENDER_REDIRECTION = 16; + /** * Skip view rendering but output http headers */ diff --git a/lib/view/sfViewCacheManager.class.php b/lib/view/sfViewCacheManager.class.php index 834d46697..7dfb51d2e 100644 --- a/lib/view/sfViewCacheManager.class.php +++ b/lib/view/sfViewCacheManager.class.php @@ -182,6 +182,12 @@ public function generateCacheKey($internalUri, $hostName = '', $vary = '', $cont $cacheKey = '/'.$hostNamePart.'/'.ltrim($cacheKey, '/'); } + // BC layer to avoid invalidate all cache. + if (sfRequest::GET !== $method = $this->request->getMethod()) + { + $cacheKey = '/'.$this->request->getMethod().'/'.ltrim($cacheKey, '/'); + } + // normalize to a leading slash if (0 !== strpos($cacheKey, '/')) { @@ -447,30 +453,14 @@ protected function getCacheConfig($internalUri, $key, $defaultValue = null) */ public function isCacheable($internalUri) { - if ($this->request instanceof sfWebRequest && !$this->request->isMethod(sfRequest::GET)) - { - return false; - } - list($route_name, $params) = $this->controller->convertUrlStringToParameters($internalUri); - if (!isset($params['module'])) + if (!isset($params['module']) || !isset($params['action'])) { return false; } - $this->registerConfiguration($params['module']); - - if (isset($this->cacheConfig[$params['module']][$params['action']])) - { - return ($this->cacheConfig[$params['module']][$params['action']]['lifeTime'] > 0); - } - else if (isset($this->cacheConfig[$params['module']]['DEFAULT'])) - { - return ($this->cacheConfig[$params['module']]['DEFAULT']['lifeTime'] > 0); - } - - return false; + return $this->isActionCacheable($params['module'], $params['action']); } /** @@ -485,7 +475,16 @@ public function isCacheable($internalUri) */ public function isActionCacheable($moduleName, $actionName) { - if ($this->request instanceof sfWebRequest && !$this->request->isMethod(sfRequest::GET)) + if ( + $this->request instanceof sfWebRequest + && !in_array(strtoupper($this->request->getMethod()), array(sfRequest::GET, sfRequest::HEAD), true) + ) { + return false; + } + + $response = $this->context->getResponse(); + + if ($response instanceof sfWebResponse && $response->isPrivate()) { return false; } @@ -936,9 +935,13 @@ public function setActionCache($uri, $content, $decoratorTemplate) */ public function setPageCache($uri) { - if (sfView::RENDER_CLIENT != $this->controller->getRenderMode()) + switch ($this->controller->getRenderMode()) { - return; + case sfView::RENDER_CLIENT: + case sfView::RENDER_REDIRECTION: + break; + default: + return; } // save content in cache