diff --git a/core/modules/config/config.module b/core/modules/config/config.module index 84e0210..e5c9943 100644 --- a/core/modules/config/config.module +++ b/core/modules/config/config.module @@ -28,9 +28,9 @@ function config_help($route_name, RouteMatchInterface $route_match) { } /** - * Implements hook_file_download(). + * Implements hook_unmanaged_file_download_headers(). */ -function config_file_download($uri) { +function config_unmanaged_file_download_headers($uri) { $scheme = file_uri_scheme($uri); $target = file_uri_target($uri); if ($scheme == 'temporary' && $target == 'config.tar.gz') { diff --git a/core/modules/file/file.module b/core/modules/file/file.module index 61aa1bc..00cd257 100644 --- a/core/modules/file/file.module +++ b/core/modules/file/file.module @@ -550,26 +550,6 @@ function file_save_data($data, $destination = NULL, $replace = FILE_EXISTS_RENAM } /** - * Examines a file entity and returns appropriate content headers for download. - * - * @param \Drupal\file\FileInterface $file - * A file entity. - * - * @return - * An associative array of headers, as expected by - * \Symfony\Component\HttpFoundation\StreamedResponse. - */ -function file_get_content_headers(FileInterface $file) { - $type = mime_header_encode($file->getMimeType()); - - return array( - 'Content-Type' => $type, - 'Content-Length' => $file->getSize(), - 'Cache-Control' => 'private', - ); -} - -/** * Implements hook_theme(). */ function file_theme() { @@ -599,48 +579,6 @@ function file_theme() { } /** - * Implements hook_file_download(). - */ -function file_file_download($uri) { - // Get the file record based on the URI. If not in the database just return. - /** @var \Drupal\file\FileInterface[] $files */ - $files = entity_load_multiple_by_properties('file', array('uri' => $uri)); - if (count($files)) { - foreach ($files as $item) { - // Since some database servers sometimes use a case-insensitive comparison - // by default, double check that the filename is an exact match. - if ($item->getFileUri() === $uri) { - $file = $item; - break; - } - } - } - if (!isset($file)) { - return; - } - - // Find out which (if any) fields of this type contain the file. - $references = file_get_file_references($file, NULL, EntityStorageInterface::FIELD_LOAD_CURRENT, NULL); - - // Stop processing if there are no references in order to avoid returning - // headers for files controlled by other modules. Make an exception for - // temporary files where the host entity has not yet been saved (for example, - // an image preview on a node/add form) in which case, allow download by the - // file's owner. - if (empty($references) && ($file->isPermanent() || $file->getOwnerId() != \Drupal::currentUser()->id())) { - return; - } - - if (!$file->access('download')) { - return -1; - } - - // Access is granted. - $headers = file_get_content_headers($file); - return $headers; -} - -/** * Implements file_cron() */ function file_cron() { diff --git a/core/modules/file/src/Entity/File.php b/core/modules/file/src/Entity/File.php index b21a182..b09fa29 100644 --- a/core/modules/file/src/Entity/File.php +++ b/core/modules/file/src/Entity/File.php @@ -7,6 +7,7 @@ namespace Drupal\file\Entity; +use Drupal\Component\Utility\Unicode; use Drupal\Core\Entity\ContentEntityBase; use Drupal\Core\Entity\EntityStorageInterface; use Drupal\Core\Entity\EntityTypeInterface; @@ -129,6 +130,19 @@ public function getChangedTime() { /** * {@inheritdoc} */ + public function getContentHeaders() { + $type = Unicode::mimeHeaderEncode($this->getMimeType()); + + return array( + 'Content-Type' => $type, + 'Content-Length' => $this->getSize(), + 'Cache-Control' => 'private', + ); + } + + /** + * {@inheritdoc} + */ public function getOwner() { return $this->get('uid')->entity; } diff --git a/core/modules/file/src/FileAccessControlHandler.php b/core/modules/file/src/FileAccessControlHandler.php index c6183da..3f9623b 100644 --- a/core/modules/file/src/FileAccessControlHandler.php +++ b/core/modules/file/src/FileAccessControlHandler.php @@ -24,6 +24,13 @@ class FileAccessControlHandler extends EntityAccessControlHandler { protected function checkAccess(EntityInterface $entity, $operation, $langcode, AccountInterface $account) { if ($operation == 'download') { + + // Allow access to non permanent files only to the file owner. I.e. to the + // preview of a just uploaded image. + if (!$entity->isPermanent() && $entity->getOwnerId() != $account->id()) { + return FALSE; + } + foreach ($this->getFileReferences($entity) as $field_name => $entity_map) { foreach ($entity_map as $referencing_entity_type => $referencing_entities) { /** @var \Drupal\Core\Entity\EntityInterface $referencing_entity */ diff --git a/core/modules/file/src/FileInterface.php b/core/modules/file/src/FileInterface.php index be94aaa..d4b956d 100644 --- a/core/modules/file/src/FileInterface.php +++ b/core/modules/file/src/FileInterface.php @@ -119,4 +119,13 @@ public function setTemporary(); * Creation timestamp of the node. */ public function getCreatedTime(); + + /** + * Returns the file headers for download. + * + * @return array + * An associative array of headers, as expected by + * \Symfony\Component\HttpFoundation\StreamedResponse. + */ + public function getContentHeaders(); } diff --git a/core/modules/file/src/Tests/DownloadTest.php b/core/modules/file/src/Tests/DownloadTest.php index ac81651..393684b 100644 --- a/core/modules/file/src/Tests/DownloadTest.php +++ b/core/modules/file/src/Tests/DownloadTest.php @@ -85,10 +85,10 @@ protected function doPrivateFileTransferTest() { // Test that the file transferred correctly. $this->assertEqual($contents, $this->content, 'Contents of the file are correct.'); - // Deny access to all downloads via a -1 header. - file_test_set_return('download', -1); + // Deny access to all downloads without header. + file_test_set_return('download', NULL); $this->drupalHead($url); - $this->assertResponse(403, 'Correctly denied access to a file when file_test sets the header to -1.'); + $this->assertResponse(403, 'Correctly denied access to a file when no headers are sent.'); // Try non-existent file. $url = file_create_url('private://' . $this->randomMachineName()); diff --git a/core/modules/file/tests/file_test/file_test.module b/core/modules/file/tests/file_test/file_test.module index b1bdeb3..4fed89f 100644 --- a/core/modules/file/tests/file_test/file_test.module +++ b/core/modules/file/tests/file_test/file_test.module @@ -170,9 +170,9 @@ function file_test_file_validate(File $file) { } /** - * Implements hook_file_download(). + * Implements hook_unmanaged_file_download_headers(). */ -function file_test_file_download($uri) { +function file_test_unmanaged_file_download_headers($uri) { _file_test_log_call('download', array($uri)); return _file_test_get_return('download'); } diff --git a/core/modules/image/image.module b/core/modules/image/image.module index f1faacb..9a64edb 100644 --- a/core/modules/image/image.module +++ b/core/modules/image/image.module @@ -146,45 +146,6 @@ function image_theme() { } /** - * Implements hook_file_download(). - * - * Control the access to files underneath the styles directory. - */ -function image_file_download($uri) { - $path = file_uri_target($uri); - - // Private file access for image style derivatives. - if (strpos($path, 'styles/') === 0) { - $args = explode('/', $path); - - // Discard "styles", style name, and scheme from the path - $args = array_slice($args, 3); - - // Then the remaining parts are the path to the image. - $original_uri = file_uri_scheme($uri) . '://' . implode('/', $args); - - // Check that the file exists and is an image. - $image = \Drupal::service('image.factory')->get($uri); - if ($image->isValid()) { - // Check the permissions of the original to grant access to this image. - $headers = \Drupal::moduleHandler()->invokeAll('file_download', array($original_uri)); - // Confirm there's at least one module granting access and none denying access. - if (!empty($headers) && !in_array(-1, $headers)) { - return array( - // Send headers describing the image's size, and MIME-type... - 'Content-Type' => $image->getMimeType(), - 'Content-Length' => $image->getFileSize(), - // By not explicitly setting them here, this uses normal Drupal - // Expires, Cache-Control and ETag headers to prevent proxy or - // browser caching of private images. - ); - } - } - return -1; - } -} - -/** * Implements hook_file_move(). */ function image_file_move(File $file, File $source) { diff --git a/core/modules/image/src/Controller/ImageStyleDownloadController.php b/core/modules/image/src/Controller/ImageStyleDownloadController.php index 2b2d89f..59494ee 100644 --- a/core/modules/image/src/Controller/ImageStyleDownloadController.php +++ b/core/modules/image/src/Controller/ImageStyleDownloadController.php @@ -10,6 +10,7 @@ use Drupal\Component\Utility\Crypt; use Drupal\Core\Image\ImageFactory; use Drupal\Core\Lock\LockBackendInterface; +use Drupal\file\Entity\File; use Drupal\image\ImageStyleInterface; use Drupal\system\FileDownloadController; use Psr\Log\LoggerInterface; @@ -18,6 +19,7 @@ use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException; +use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; use Symfony\Component\HttpKernel\Exception\ServiceUnavailableHttpException; /** @@ -120,11 +122,12 @@ public function deliver(Request $request, $scheme, ImageStyleInterface $image_st if (file_exists($derivative_uri)) { return parent::download($request, $scheme); } - else { - $headers = $this->moduleHandler()->invokeAll('file_download', array($image_uri)); - if (in_array(-1, $headers) || empty($headers)) { - throw new AccessDeniedHttpException(); - } + + $headers = $this->getContentHeaders($image_uri); + // If we failed to get valid headers it means the access has not been + // granted. + if (empty($headers)) { + throw new AccessDeniedHttpException(); } } diff --git a/core/modules/image/tests/modules/image_module_test/image_module_test.module b/core/modules/image/tests/modules/image_module_test/image_module_test.module index df71375..d42945e 100644 --- a/core/modules/image/tests/modules/image_module_test/image_module_test.module +++ b/core/modules/image/tests/modules/image_module_test/image_module_test.module @@ -7,8 +7,11 @@ use Drupal\image\ImageStyleInterface; -function image_module_test_file_download($uri) { - $default_uri = \Drupal::state()->get('image.test_file_download') ?: FALSE; +/** + * Implements hook_file_download_headers(). + */ +function image_module_test_unmanaged_file_download_headers($uri) { + $default_uri = \Drupal::state()->get('image.test_file_download') ? : FALSE; if ($default_uri == $uri) { return array('X-Image-Owned-By' => 'image_module_test'); } diff --git a/core/modules/responsive_image/src/Tests/ResponsiveImageFieldDisplayTest.php b/core/modules/responsive_image/src/Tests/ResponsiveImageFieldDisplayTest.php index 9759544..ae2f239 100644 --- a/core/modules/responsive_image/src/Tests/ResponsiveImageFieldDisplayTest.php +++ b/core/modules/responsive_image/src/Tests/ResponsiveImageFieldDisplayTest.php @@ -83,6 +83,7 @@ public function _testResponsiveImageFieldFormatters($scheme) { $this->createImageField($field_name, 'article', array('uri_scheme' => $scheme)); // Create a new node with an image attached. $test_image = current($this->drupalGetTestFiles('image')); + debug($test_image); $nid = $this->uploadNodeImage($test_image, $field_name, 'article'); $node = node_load($nid, TRUE); diff --git a/core/modules/system/src/FileDownloadController.php b/core/modules/system/src/FileDownloadController.php index a65342d..e1357c5 100644 --- a/core/modules/system/src/FileDownloadController.php +++ b/core/modules/system/src/FileDownloadController.php @@ -49,16 +49,7 @@ public function download(Request $request, $scheme = 'private') { $uri = $scheme . '://' . $target; if (file_stream_wrapper_valid_scheme($scheme) && file_exists($uri)) { - // Let other modules provide headers and controls access to the file. - $headers = $this->moduleHandler()->invokeAll('file_download', array($uri)); - - foreach ($headers as $result) { - if ($result == -1) { - throw new AccessDeniedHttpException(); - } - } - - if (count($headers)) { + if ($headers = $this->getContentHeaders($uri)) { return new BinaryFileResponse($uri, 200, $headers); } @@ -68,4 +59,36 @@ public function download(Request $request, $scheme = 'private') { throw new NotFoundHttpException(); } + /** + * Gets headers for the provided file uri. + * + * It first tries to load managed file based on the uri, if not it will invoke + * the hook_unmanaged_file_download_headers(). + * + * @param string $file_uri + * The file uri for which to get the headers. + * + * @return array|NULL + * If access to the file is granted an associative array of headers as + * expected by \Symfony\Component\HttpFoundation\StreamedResponse, NULL + * otherwise. + */ + protected function getContentHeaders($file_uri) { + if ($this->moduleHandler()->moduleExists('file')) { + $files = $this->entityManager() + ->getStorage('file') + ->loadByProperties(array('uri' => $file_uri)); + /** @var \Drupal\file\FileInterface $file */ + $file = reset($files); + + if (!empty($file) && $file->access('download')) { + return $file->getContentHeaders(); + } + } + + if (empty($headers)) { + return $this->moduleHandler()->invokeAll('unmanaged_file_download_headers', array($file_uri)); + } + } + } diff --git a/core/modules/system/system.api.php b/core/modules/system/system.api.php index cd2e6cf..b2e34d7 100644 --- a/core/modules/system/system.api.php +++ b/core/modules/system/system.api.php @@ -1409,24 +1409,20 @@ function hook_stream_wrappers_alter(&$wrappers) { } /** - * Control access to private file downloads and specify HTTP headers. + * Specify HTTP headers for a unmanaged file's download. * - * This hook allows modules to enforce permissions on file downloads whenever - * Drupal is handling file download, as opposed to the web server bypassing - * Drupal and returning the file from a public directory. Modules can also - * provide headers to specify information like the file's name or MIME type. + * Modules can provide headers to specify information like the file's name or + * MIME type. * - * @param $uri - * The URI of the file. - * @return - * If the user does not have permission to access the file, return -1. If the - * user has permission, return an array with the appropriate headers. If the - * file is not controlled by the current module, the return value should be - * NULL. + * @param string $uri + * The file uri being downloaded. + * @return array|null + * An array with the appropriate headers. If the file is not controlled by the + * current module, the return value should be NULL. * - * @see file_download() + * @see FileDownloadController::download() */ -function hook_file_download($uri) { +function hook_unmanaged_file_download_headers($uri) { // Check to see if this is a config download. $scheme = file_uri_scheme($uri); $target = file_uri_target($uri);