Issue #3292908 by alexpott, dww, longwave, catch, Berdir, Charlie ChX Negyesi,...

Issue #3292908 by alexpott, dww, longwave, catch, Berdir, Charlie ChX Negyesi, mradcliffe, kndr: Fast 404s are slower than regular 404s (cherry picked from commit ac1a32ab)

Issue #3292908 by alexpott, dww, longwave, catch, Berdir, Charlie ChX Negyesi,...
01b74bea · catch · 660192c8 · 01b74bea · 01b74bea · 01b74bea
Commit 01b74bea authored 2 years ago by catch
--- a/core/assets/scaffold/files/default.settings.php
+++ b/core/assets/scaffold/files/default.settings.php
@@ -633,33 +633,6 @@
 # $config['system.site']['name'] = 'My Drupal site';
 # $config['user.settings']['anonymous'] = 'Visitor';

-/**
- * Fast 404 pages:
- *
- * Drupal can generate fully themed 404 pages. However, some of these responses
- * are for images or other resource files that are not displayed to the user.
- * This can waste bandwidth, and also generate server load.
- *
- * The options below return a simple, fast 404 page for URLs matching a
- * specific pattern:
- * - $config['system.performance']['fast_404']['exclude_paths']: A regular
- *   expression to match paths to exclude, such as images generated by image
- *   styles, or dynamically-resized images. The default pattern provided below
- *   also excludes the private file system. If you need to add more paths, you
- *   can add '|path' to the expression.
- * - $config['system.performance']['fast_404']['paths']: A regular expression to
- *   match paths that should return a simple 404 page, rather than the fully
- *   themed 404 page. If you don't have any aliases ending in htm or html you
- *   can add '|s?html?' to the expression.
- * - $config['system.performance']['fast_404']['html']: The html to return for
- *   simple 404 pages.
- *
- * Remove the leading hash signs if you would like to alter this functionality.
- */
-# $config['system.performance']['fast_404']['exclude_paths'] = '/\/(?:styles)|(?:system\/files)\//';
-# $config['system.performance']['fast_404']['paths'] = '/\.(?:txt|png|gif|jpe?g|css|js|ico|swf|flv|cgi|bat|pl|dll|exe|asp)$/i';
-# $config['system.performance']['fast_404']['html'] = '<!DOCTYPE html><html><head><title>404 Not Found</title></head><body><h1>Not Found</h1><p>The requested URL "@path" was not found on this server.</p></body></html>';
-
 /**
 * Load services definition file.
 */

--- a/core/core.services.yml
+++ b/core/core.services.yml
@@ -1299,7 +1299,7 @@ services:
    class: Drupal\Core\EventSubscriber\Fast404ExceptionHtmlSubscriber
    tags:
      - { name: event_subscriber }
-    arguments: ['@config.factory', '@http_kernel']
+    arguments: ['@config.factory', '@cache_tags.invalidator']
  exception.enforced_form_response:
    class: Drupal\Core\EventSubscriber\EnforcedFormResponseSubscriber
    tags:

--- a/core/lib/Drupal/Core/EventSubscriber/Fast404ExceptionHtmlSubscriber.php
+++ b/core/lib/Drupal/Core/EventSubscriber/Fast404ExceptionHtmlSubscriber.php
@@ -2,45 +2,61 @@

 namespace Drupal\Core\EventSubscriber;

-use Drupal\Core\Config\ConfigFactoryInterface;
 use Drupal\Component\Utility\Html;
+use Drupal\Core\Cache\CacheTagsInvalidatorInterface;
+use Drupal\Core\Config\ConfigCrudEvent;
+use Drupal\Core\Config\ConfigEvents;
+use Drupal\Core\Config\ConfigFactoryInterface;
+use Drupal\Core\Render\HtmlResponse;
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\HttpKernel\Event\ExceptionEvent;
-use Symfony\Component\HttpKernel\HttpKernelInterface;

 /**
 * High-performance 404 exception subscriber.
 *
 * This subscriber will return a minimalist 404 response for HTML requests
 * without running a full page theming operation.
+ *
+ * Fast 404s are configured using the system.performance configuration object.
+ * There are the following options:
+ * - system.performance:fast_404.exclude_paths: A regular expression to match
+ *   paths to exclude, such as images generated by image styles, or
+ *   dynamically-resized images. The default pattern provided below also
+ *   excludes the private file system. If you need to add more paths, you can
+ *   add '|path' to the expression.
+ * - system.performance:fast_404.paths: A regular expression to match paths that
+ *   should return a simple 404 page, rather than the fully themed 404 page. If
+ *   you don't have any aliases ending in htm or html you can add '|s?html?' to
+ *   the expression.
+ * - system.performance:fast_404.html: The html to return for simple 404 pages.
 */
 class Fast404ExceptionHtmlSubscriber extends HttpExceptionSubscriberBase {

  /**
-   * The HTTP kernel.
+   * The config factory.
   *
-   * @var \Symfony\Component\HttpKernel\HttpKernelInterface
+   * @var \Drupal\Core\Config\ConfigFactoryInterface
   */
-  protected $httpKernel;
+  protected $configFactory;

  /**
-   * The config factory.
+   * The cache tags invalidator.
   *
-   * @var \Drupal\Core\Config\ConfigFactoryInterface
+   * @var \Drupal\Core\Cache\CacheTagsInvalidatorInterface
   */
-  protected $configFactory;
+  protected $cacheTagsInvalidator;

  /**
   * Constructs a new Fast404ExceptionHtmlSubscriber.
   *
   * @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory
   *   The configuration factory.
-   * @param \Symfony\Component\HttpKernel\HttpKernelInterface $http_kernel
-   *   The HTTP Kernel service.
+   * @param \Drupal\Core\Cache\CacheTagsInvalidatorInterface $cache_tags_invalidator
+   *   The cache tags invalidator.
   */
-  public function __construct(ConfigFactoryInterface $config_factory, HttpKernelInterface $http_kernel) {
+  public function __construct(ConfigFactoryInterface $config_factory, CacheTagsInvalidatorInterface $cache_tags_invalidator) {
    $this->configFactory = $config_factory;
-    $this->httpKernel = $http_kernel;
+    $this->cacheTagsInvalidator = $cache_tags_invalidator;
  }

  /**
@@ -74,10 +90,32 @@ public function on404(ExceptionEvent $event) {
      $fast_paths = $config->get('fast_404.paths');
      if ($fast_paths && preg_match($fast_paths, $request->getPathInfo())) {
        $fast_404_html = strtr($config->get('fast_404.html'), ['@path' => Html::escape($request->getUri())]);
-        $response = new Response($fast_404_html, Response::HTTP_NOT_FOUND);
+        $response = new HtmlResponse($fast_404_html, Response::HTTP_NOT_FOUND);
        $event->setResponse($response);
      }
    }
  }

+  /**
+   * Invalidates 4xx-response cache tag if fast 404 config is changed.
+   *
+   * @param \Drupal\Core\Config\ConfigCrudEvent $event
+   *   The configuration event.
+   */
+  public function onConfigSave(ConfigCrudEvent $event): void {
+    $saved_config = $event->getConfig();
+    if ($saved_config->getName() === 'system.performance' && $event->isChanged('fast_404')) {
+      $this->cacheTagsInvalidator->invalidateTags(['4xx-response']);
+    }
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public static function getSubscribedEvents(): array {
+    $events = parent::getSubscribedEvents();
+    $events[ConfigEvents::SAVE] = 'onConfigSave';
+    return $events;
+  }
+
 }
--- a/core/tests/Drupal/FunctionalTests/EventSubscriber/Fast404Test.php
+++ b/core/tests/Drupal/FunctionalTests/EventSubscriber/Fast404Test.php
+<?php
+
+namespace Drupal\FunctionalTests\EventSubscriber;
+
+use Drupal\Tests\BrowserTestBase;
+
+/**
+ * Tests the fast 404 functionality.
+ *
+ * @group EventSubscriber
+ *
+ * @see \Drupal\Core\EventSubscriber\Fast404ExceptionHtmlSubscriber
+ */
+class Fast404Test extends BrowserTestBase {
+
+  /**
+   * {@inheritdoc}
+   */
+  protected $defaultTheme = 'stark';
+
+  /**
+   * Tests the fast 404 functionality.
+   */
+  public function testFast404(): void {
+    $this->drupalGet('does-not-exist');
+    $this->assertSession()->statusCodeEquals(404);
+    // Regular 404s will contain CSS from the system module.
+    $this->assertSession()->responseContains('modules/system/css/');
+    $this->drupalGet('does-not-exist.txt');
+    $this->assertSession()->statusCodeEquals(404);
+    // Fast 404s do not have any CSS.
+    $this->assertSession()->responseNotContains('modules/system/css/');
+    $this->assertSession()->responseHeaderContains('X-Drupal-Cache', 'Miss');
+    // Fast 404s can be cached.
+    $this->drupalGet('does-not-exist.txt');
+    $this->assertSession()->statusCodeEquals(404);
+    $this->assertSession()->responseHeaderContains('X-Drupal-Cache', 'Hit');
+    $this->assertSession()->pageTextNotContains('Oops I did it again!');
+
+    // Changing configuration should invalidate the cache.
+    $this->config('system.performance')->set('fast_404.html', '<!DOCTYPE html><html><head><title>404 Not Found</title></head><body><h1>Oops I did it again!</h1><p>The requested URL "@path" was not found on this server.</p></body></html>')->save();
+    $this->drupalGet('does-not-exist.txt');
+    $this->assertSession()->responseNotContains('modules/system/css/');
+    $this->assertSession()->statusCodeEquals(404);
+    $this->assertSession()->responseHeaderContains('X-Drupal-Cache', 'Miss');
+    $this->assertSession()->pageTextContains('Oops I did it again!');
+
+    // Ensure disabling works.
+    $this->config('system.performance')->set('fast_404.enabled', FALSE)->save();
+    $this->drupalGet('does-not-exist.txt');
+    $this->assertSession()->responseContains('modules/system/css/');
+    $this->assertSession()->statusCodeEquals(404);
+    $this->assertSession()->responseHeaderContains('X-Drupal-Cache', 'Miss');
+    $this->assertSession()->pageTextNotContains('Oops I did it again!');
+
+    // Ensure settings.php can override settings.
+    $settings['config']['system.performance']['fast_404']['enabled'] = (object) [
+      'value' => TRUE,
+      'required' => TRUE,
+    ];
+    $this->writeSettings($settings);
+    // Changing settings using an override means we need to rebuild everything.
+    $this->rebuildAll();
+    $this->drupalGet('does-not-exist.txt');
+    $this->assertSession()->statusCodeEquals(404);
+    $this->assertSession()->responseNotContains('modules/system/css/');
+    // Fast 404s returned via the exception subscriber still have the
+    // X-Generator header.
+    $this->assertSession()->responseHeaderContains('X-Generator', 'Drupal');
+  }
+
+}
--- a/sites/default/default.settings.php
+++ b/sites/default/default.settings.php
@@ -633,33 +633,6 @@
 # $config['system.site']['name'] = 'My Drupal site';
 # $config['user.settings']['anonymous'] = 'Visitor';

-/**
- * Fast 404 pages:
- *
- * Drupal can generate fully themed 404 pages. However, some of these responses
- * are for images or other resource files that are not displayed to the user.
- * This can waste bandwidth, and also generate server load.
- *
- * The options below return a simple, fast 404 page for URLs matching a
- * specific pattern:
- * - $config['system.performance']['fast_404']['exclude_paths']: A regular
- *   expression to match paths to exclude, such as images generated by image
- *   styles, or dynamically-resized images. The default pattern provided below
- *   also excludes the private file system. If you need to add more paths, you
- *   can add '|path' to the expression.
- * - $config['system.performance']['fast_404']['paths']: A regular expression to
- *   match paths that should return a simple 404 page, rather than the fully
- *   themed 404 page. If you don't have any aliases ending in htm or html you
- *   can add '|s?html?' to the expression.
- * - $config['system.performance']['fast_404']['html']: The html to return for
- *   simple 404 pages.
- *
- * Remove the leading hash signs if you would like to alter this functionality.
- */
-# $config['system.performance']['fast_404']['exclude_paths'] = '/\/(?:styles)|(?:system\/files)\//';
-# $config['system.performance']['fast_404']['paths'] = '/\.(?:txt|png|gif|jpe?g|css|js|ico|swf|flv|cgi|bat|pl|dll|exe|asp)$/i';
-# $config['system.performance']['fast_404']['html'] = '<!DOCTYPE html><html><head><title>404 Not Found</title></head><body><h1>Not Found</h1><p>The requested URL "@path" was not found on this server.</p></body></html>';
-
 /**
 * Load services definition file.
 */