diff --git a/core/lib/Drupal/Core/EventSubscriber/Fast404ExceptionHtmlSubscriber.php b/core/lib/Drupal/Core/EventSubscriber/Fast404ExceptionHtmlSubscriber.php index bd9594ab052..5ebf3f41766 100644 --- a/core/lib/Drupal/Core/EventSubscriber/Fast404ExceptionHtmlSubscriber.php +++ b/core/lib/Drupal/Core/EventSubscriber/Fast404ExceptionHtmlSubscriber.php @@ -3,6 +3,7 @@ namespace Drupal\Core\EventSubscriber; use Drupal\Component\Utility\Html; +use Drupal\Core\Cache\CacheableMetadata; use Drupal\Core\Cache\CacheTagsInvalidatorInterface; use Drupal\Core\Config\ConfigCrudEvent; use Drupal\Core\Config\ConfigEvents; @@ -91,6 +92,14 @@ class Fast404ExceptionHtmlSubscriber extends HttpExceptionSubscriberBase { 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 HtmlResponse($fast_404_html, Response::HTTP_NOT_FOUND); + // Some routes such as system.files conditionally throw a + // NotFoundHttpException depending on URL parameters instead of just the + // route and route parameters, so add the URL cache context to account + // for this. + $cacheable_metadata = new CacheableMetadata(); + $cacheable_metadata->setCacheContexts(['url']); + $cacheable_metadata->addCacheTags(['4xx-response']); + $response->addCacheableDependency($cacheable_metadata); $event->setResponse($response); } } diff --git a/core/modules/file/src/Entity/File.php b/core/modules/file/src/Entity/File.php index 57276f495e8..e977337d001 100644 --- a/core/modules/file/src/Entity/File.php +++ b/core/modules/file/src/Entity/File.php @@ -2,6 +2,7 @@ namespace Drupal\file\Entity; +use Drupal\Core\Cache\Cache; use Drupal\Core\Entity\ContentEntityBase; use Drupal\Core\Entity\EntityChangedTrait; use Drupal\Core\Entity\EntityStorageInterface; @@ -276,4 +277,20 @@ class File extends ContentEntityBase implements FileInterface { return NULL; } + /** + * {@inheritdoc} + */ + protected function invalidateTagsOnSave($update) { + $tags = $this->getListCacheTagsToInvalidate(); + // Always invalidate the 404 or 403 response cache because while files do + // not have a canonical URL as such, they may be served via routes such as + // private files. + // Creating or updating an entity may change a cached 403 or 404 response. + $tags = Cache::mergeTags($tags, ['4xx-response']); + if ($update) { + $tags = Cache::mergeTags($tags, $this->getCacheTagsToInvalidate()); + } + Cache::invalidateTags($tags); + } + } diff --git a/core/modules/file/tests/src/Functional/DownloadTest.php b/core/modules/file/tests/src/Functional/DownloadTest.php index 72d75abe35e..3bf34c1c545 100644 --- a/core/modules/file/tests/src/Functional/DownloadTest.php +++ b/core/modules/file/tests/src/Functional/DownloadTest.php @@ -82,7 +82,7 @@ class DownloadTest extends FileManagedTestBase { // Create a file. $contents = $this->randomMachineName(8); - $file = $this->createFile(NULL, $contents, 'private'); + $file = $this->createFile($contents . '.txt', $contents, 'private'); // Created private files without usage are by default not accessible // for a user different from the owner, but createFile always uses uid 1 // as the owner of the files. Therefore make it permanent to allow access @@ -108,19 +108,33 @@ class DownloadTest extends FileManagedTestBase { $this->assertSame($contents, $this->getSession()->getPage()->getContent(), 'Contents of the file are correct.'); $http_client = $this->getHttpClient(); + // Try non-existent file. + file_test_reset(); + $not_found_url = $this->fileUrlGenerator->generateAbsoluteString('private://' . $this->randomMachineName() . '.txt'); + $response = $http_client->head($not_found_url, ['http_errors' => FALSE]); + $this->assertSame(404, $response->getStatusCode(), 'Correctly returned 404 response for a non-existent file.'); + // Assert that hook_file_download is not called. + $this->assertEquals([], \Drupal::state()->get('file_test.results')['download']); + + // Having tried a non-existent file, try the original file again to ensure + // it's returned instead of a 404 response. + // Set file_test access header to allow the download. + file_test_reset(); + file_test_set_return('download', ['x-foo' => 'Bar']); + $this->drupalGet($url); + // Verify that header is set by file_test module on private download. + $this->assertSession()->responseHeaderEquals('x-foo', 'Bar'); + // Verify that page cache is disabled on private file download. + $this->assertSession()->responseHeaderDoesNotExist('x-drupal-cache'); + $this->assertSession()->statusCodeEquals(200); + // Test that the file transferred correctly. + $this->assertSame($contents, $this->getSession()->getPage()->getContent(), 'Contents of the file are correct.'); + // Deny access to all downloads via a -1 header. file_test_set_return('download', -1); $response = $http_client->head($url, ['http_errors' => FALSE]); $this->assertSame(403, $response->getStatusCode(), 'Correctly denied access to a file when file_test sets the header to -1.'); - // Try non-existent file. - file_test_reset(); - $url = $this->fileUrlGenerator->generateAbsoluteString('private://' . $this->randomMachineName()); - $response = $http_client->head($url, ['http_errors' => FALSE]); - $this->assertSame(404, $response->getStatusCode(), 'Correctly returned 404 response for a non-existent file.'); - // Assert that hook_file_download is not called. - $this->assertEquals([], \Drupal::state()->get('file_test.results')['download']); - // Try requesting the private file url without a file specified. file_test_reset(); $this->drupalGet('/system/files'); diff --git a/core/tests/Drupal/FunctionalTests/EventSubscriber/Fast404Test.php b/core/tests/Drupal/FunctionalTests/EventSubscriber/Fast404Test.php index be6d9f42dc7..21444c746e9 100644 --- a/core/tests/Drupal/FunctionalTests/EventSubscriber/Fast404Test.php +++ b/core/tests/Drupal/FunctionalTests/EventSubscriber/Fast404Test.php @@ -2,6 +2,7 @@ namespace Drupal\FunctionalTests\EventSubscriber; +use Drupal\file\Entity\File; use Drupal\Tests\BrowserTestBase; /** @@ -18,6 +19,11 @@ class Fast404Test extends BrowserTestBase { */ protected $defaultTheme = 'stark'; + /** + * {@inheritdoc} + */ + protected static $modules = ['file']; + /** * Tests the fast 404 functionality. */ @@ -69,4 +75,33 @@ class Fast404Test extends BrowserTestBase { $this->assertSession()->responseHeaderContains('X-Generator', 'Drupal'); } + /** + * Tests the fast 404 functionality. + */ + public function testFast404PrivateFiles(): void { + $admin = $this->createUser([], NULL, TRUE); + $this->drupalLogin($admin); + + $file_url = 'system/files/test/private-file-test.txt'; + $this->drupalGet($file_url); + $this->assertSession()->statusCodeEquals(404); + $this->drupalGet($file_url); + $this->assertSession()->statusCodeEquals(404); + + // Create a private file for testing accessible by the admin user. + \Drupal::service('file_system')->mkdir($this->privateFilesDirectory . '/test'); + $filepath = 'private://test/private-file-test.txt'; + $contents = "file_put_contents() doesn't seem to appreciate empty strings so let's put in some data."; + file_put_contents($filepath, $contents); + $file = File::create([ + 'uri' => $filepath, + 'uid' => $admin->id(), + ]); + $file->save(); + + $this->drupalGet($file_url); + $this->assertSession()->statusCodeEquals(200); + $this->assertSession()->pageTextContains($contents); + } + }