Commit 17c954a1 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 1dbc4128

core/lib/Drupal/Core/EventSubscriber/Fast404ExceptionHtmlSubscriber.php

+9 −0
Original line number Diff line number Diff line
@@ -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);

      }

    }

core/modules/file/src/Entity/File.php

+17 −0
Original line number Diff line number Diff line
@@ -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;
@@ -268,4 +269,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);

  }



}

core/modules/file/tests/src/Functional/DownloadTest.php

+22 −8
Original line number Diff line number Diff line
@@ -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');

core/tests/Drupal/FunctionalTests/EventSubscriber/Fast404Test.php

+35 −0
Original line number Diff line number Diff line
@@ -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);

  }



}