diff --git a/core/modules/page_cache/src/StackMiddleware/PageCache.php b/core/modules/page_cache/src/StackMiddleware/PageCache.php index 14f9df5..a4d9053 100644 --- a/core/modules/page_cache/src/StackMiddleware/PageCache.php +++ b/core/modules/page_cache/src/StackMiddleware/PageCache.php @@ -7,6 +7,7 @@ use Drupal\Core\Cache\CacheBackendInterface; use Drupal\Core\PageCache\RequestPolicyInterface; use Drupal\Core\PageCache\ResponsePolicyInterface; +use Drupal\Core\Site\Settings; use Symfony\Component\HttpFoundation\BinaryFileResponse; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; @@ -243,15 +244,34 @@ protected function fetch(Request $request, $type = self::MASTER_REQUEST, $catch return $response; } - // The response passes all of the above checks, so cache it. + $request_time = $request->server->get('REQUEST_TIME'); + // The response passes all of the above checks, so cache it. Page cache + // entries default to Cache::PERMANENT since they will be expired via cache + // tags locally. Because of this, page cache ignores max age. // - Get the tags from CacheableResponseInterface per the earlier comments. // - Get the time expiration from the Expires header, rather than the // interface, but see https://www.drupal.org/node/2352009 about possibly // changing that. - $tags = $response->getCacheableMetadata()->getCacheTags(); - $date = $response->getExpires()->getTimestamp(); - $expire = ($date > time()) ? $date : Cache::PERMANENT; - $this->set($request, $response, $expire, $tags); + $expire = 0; + // 403 and 404 responses can fill non-LRU cache backends and generally are + // likely to have a low cache hit rate. So do not cache them permanently. + if ($response->isClientError()) { + // Cache for an hour by default. If the 'cache_ttl_4xx' setting is + // set to 0 then do not cache the response. + $cache_ttl_4xx = Settings::get('cache_ttl_4xx', 3600); + if ($cache_ttl_4xx > 0) { + $expire = $request_time + $cache_ttl_4xx; + } + } + else { + $date = $response->getExpires()->getTimestamp(); + $expire = ($date > $request_time) ? $date : Cache::PERMANENT; + } + + if ($expire === Cache::PERMANENT || $expire > $request_time) { + $tags = $response->getCacheableMetadata()->getCacheTags(); + $this->set($request, $response, $expire, $tags); + } // Mark response as a cache miss. $response->headers->set('X-Drupal-Cache', 'MISS'); diff --git a/core/modules/page_cache/src/Tests/PageCacheTest.php b/core/modules/page_cache/src/Tests/PageCacheTest.php index 44c94e1..9194e65 100644 --- a/core/modules/page_cache/src/Tests/PageCacheTest.php +++ b/core/modules/page_cache/src/Tests/PageCacheTest.php @@ -3,6 +3,7 @@ namespace Drupal\page_cache\Tests; use Drupal\Component\Datetime\DateTimePlus; +use Drupal\Core\Site\Settings; use Drupal\Core\Url; use Drupal\entity_test\Entity\EntityTest; use Drupal\simpletest\WebTestBase; @@ -341,6 +342,7 @@ function testPageCacheAnonymous403404() { 403 => $admin_url, 404 => $invalid_url, ]; + $cache_ttl_4xx = Settings::get('cache_ttl_4xx', 3600); foreach ($tests as $code => $content_url) { // Anonymous user, without permissions. $this->drupalGet($content_url); @@ -374,6 +376,35 @@ function testPageCacheAnonymous403404() { $this->drupalGet($content_url); $this->assertResponse($code); $this->assertEqual($this->drupalGetHeader('X-Drupal-Cache'), 'MISS'); + + // Ensure the 'expire' field on the cache entry uses cache_ttl_4xx. + $cache_item = \Drupal::service('cache.render')->get($this->getUrl() . ':html'); + $difference = $cache_item->expire - (int) $cache_item->created; + // Given that a second might have passed we cannot be sure that + // $difference will exactly equal the default cache_ttl_4xx setting. + // Account for any timing difference or rounding errors by ensuring the + // value is within 5 seconds. + $this->assertTrue( + $difference > $cache_ttl_4xx - 5 && + $difference < $cache_ttl_4xx + 5, + 'The cache entry expiry time uses the cache_ttl_4xx setting.' + ); + } + + // Disable 403 and 404 caching. + $settings['settings']['cache_ttl_4xx'] = (object) array( + 'value' => 0, + 'required' => TRUE, + ); + $this->writeSettings($settings); + \Drupal::service('cache.render')->deleteAll(); + + foreach ($tests as $code => $content_url) { + // Getting the 404 page twice should still result in a cache miss. + $this->drupalGet($content_url); + $this->drupalGet($content_url); + $this->assertResponse($code); + $this->assertEqual($this->drupalGetHeader('X-Drupal-Cache'), 'MISS'); } } diff --git a/sites/default/default.settings.php b/sites/default/default.settings.php index 7f28c29..770a3a7 100644 --- a/sites/default/default.settings.php +++ b/sites/default/default.settings.php @@ -420,6 +420,20 @@ */ # $settings['omit_vary_cookie'] = TRUE; + +/** + * Cache TTL for client error (4xx) responses. + * + * Items cached per-URL tend to result in a large number of cache items, and + * this can be problematic on 404 pages which by their nature are unbounded. A + * fixed TTL can be set for these items, defaulting to one hour, so that cache + * backends which do not support LRU can purge older entries. To disable caching + * of client error responses set the value to 0. Currently applies only to + * page_cache module. + */ +# $settings['cache_ttl_4xx'] = 3600; + + /** * Class Loader. *