Skip to content
Snippets Groups Projects
Commit e96bee87 authored by Alex Bronstein's avatar Alex Bronstein
Browse files

Issue #3312198 by catch, alexpott, longwave, Nixou, Spokje, darvanen:...

Issue #3312198 by catch, alexpott, longwave, Nixou, Spokje, darvanen: Regression concerning the cache of private files

(cherry picked from commit c95109e2)
parent 8da49ba2
Branches
Tags
22 merge requests!8394[warning] array_flip(): Can only flip STRING and INTEGER values, when saving a non-revisionable custom content entity,!7780issue 3443822: fix for 'No route found for the specified format html. Supported formats: json, xml.',!5013Issue #3071143: Table Render Array Example Is Incorrect,!4848Issue #1566662: Update module should send notifications on Thursdays,!4792Issue #2230689: Remove redundant "Italic" style,!4220Issue #3368223: Link field > Access to internal links is not checked on display.,!3884Issue #3356842,!3870Issue #3087868,!3812Draft: Issue #3339373 by alexpott, andypost, mondrake:...,!3686Issue #3219967 against 9.5.x,!3683Issue #2939397: Clearing AliasManager cache with root path raises warning,!3543Issue #3344259: Allow ajax dialog to have focus configurable,!3356Issue #3209129: Scrolling problems when adding a block via layout builder,!2280Issue #3280415: Metapackage Generator Breaks Under Composer --no-dev,!2205Quote all names in the regions section.,!2050Issue #3272969: Remove UnqiueField constraint.,!1956Issue #3268872: hook_views_invalidate_cache not called when a view is deleted,!1893Issue #3217260: Add a way to make media captions not editable in CKEditor,!1459Issue #3087632: menu_name max length is too long,!878Issue #3221534: throw an exception when IDs passed to loadMultiple() are badly formed,!866Issue #2845319: The highlighting of the 'Home' menu-link does not respect query strings and fragment identifiers,!204Issue #3040556: It is not possible to react to an entity being duplicated
......@@ -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 @@ public function on404(ExceptionEvent $event) {
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);
}
}
......
......@@ -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 @@ public static function getDefaultEntityOwner() {
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);
}
}
......@@ -82,7 +82,7 @@ protected function doPrivateFileTransferTest() {
// 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 @@ protected function doPrivateFileTransferTest() {
$this->assertSame($contents, $this->getSession()->getPage()->getContent(), 'Contents of the file are correct.');
$http_client = $this->getHttpClient();
// 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]);
$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 requesting the private file url without a file specified.
file_test_reset();
$this->drupalGet('/system/files');
......
......@@ -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 @@ public function testFast404(): void {
$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);
}
}
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment