diff --git a/core/modules/page_cache/src/StackMiddleware/PageCache.php b/core/modules/page_cache/src/StackMiddleware/PageCache.php index 14f9df5..76a8042 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; @@ -244,14 +245,33 @@ protected function fetch(Request $request, $type = self::MASTER_REQUEST, $catch } // 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. They also ignore max age, since this allows the + // local page cache to be treated differently from external proxies or CDNs. // - 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 ttl is set to 0 then do not cache + // the response + $settings_ttl = Settings::get('negative_cache_ttl', 3600); + if ($settings_ttl > 0) { + $expire = REQUEST_TIME + $settings_ttl; + } + } + else { + $date = $response->getExpires()->getTimestamp(); + $expire = ($date > 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..e957462 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; @@ -374,6 +375,30 @@ function testPageCacheAnonymous403404() { $this->drupalGet($content_url); $this->assertResponse($code); $this->assertEqual($this->drupalGetHeader('X-Drupal-Cache'), 'MISS'); + + // Ensure the expires date on the cache entry uses negative_cache_ttl. + $cache_entry = \Drupal::service('cache.render')->get($this->getUrl() . ':html'); + $difference = $cache_entry->expire - (int) $cache_entry->created; + // Account for any rounding errors by rounding the difference ot the + // nearest 10. As long as negative_cache_ttl is a multiple of 10 then this + // will work. + $this->assertEqual(round($difference, -1), Settings::get('negative_cache_ttl', 3600), 'The cache entry expiry time uses the negative_cache_ttl setting.'); + } + + // Disable 403 and 404 caching. + $settings['settings']['negative_cache_ttl'] = (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 you 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..640dcc5 100644 --- a/sites/default/default.settings.php +++ b/sites/default/default.settings.php @@ -420,6 +420,19 @@ */ # $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 + * set the value to 0. Currently applies only to page_cache module. + */ +# $settings['negative_cache_ttl'] = 3600; + + /** * Class Loader. *