Fix compatibility with HTTP specs about cacheable status codes and me…

…thods - Add pages with response 200, 203, 300, 301, 302, 404, 410 to cache. - Add to cache response from HEAD method request.
FriendsOfSymfony1 · Nov 10, 2018 · 19e6cd1 · 19e6cd1
1 parent e9527d0
commit 19e6cd1
Show file tree

Hide file tree

Showing 10 changed files with 229 additions and 41 deletions.
diff --git 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,18 +474,24 @@ public function getPresentationFor($module, $action, $viewName = null)
    *                  - sfView::RENDER_CLIENT
    *                  - sfView::RENDER_VAR
    *                  - sfView::RENDER_NONE
+   *                  - sfView::RENDER_REDIRECTION
    *
    * @return void
    *
    * @throws sfRenderException If an invalid render mode has been set
    */
   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
@@ -205,6 +205,7 @@ public function redirect($url, $delay = 0, $statusCode = 302)
     }
 
     $response->setContent(sprintf('<html><head><meta http-equiv="refresh" content="%d;url=%s"/></head></html>', $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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
    */