From 31f9e8a9c642b861161401c9de005a752bf6f201 Mon Sep 17 00:00:00 2001
From: Alex Pott <alex.a.pott@googlemail.com>
Date: Tue, 9 Apr 2024 23:33:15 +0100
Subject: [PATCH] =?UTF-8?q?Issue=20#2786735=20by=20recrit,=20eiriksm,=20ra?=
=?UTF-8?q?njith=5Fkumar=5Fk=5Fu,=20immaculatexavier,=20ravi.shankar,=20za?=
=?UTF-8?q?porylie,=20smustgrave,=20TrevorBradley,=20johnle,=20Ga=C3=ABlG,?=
=?UTF-8?q?=20jefuri,=20alexpott,=20poker10:=20Image=20derivative=20genera?=
=?UTF-8?q?tion=20does=20not=20work=20if=20effect=20"Convert"=20in=20use?=
=?UTF-8?q?=20and=20file=20stored=20in=20private=20filesystem?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
---
core/modules/image/image.module | 13 ++++
.../ImageStyleDownloadController.php | 65 ++++++++++++++-----
.../Functional/ImageStylesPathAndUrlTest.php | 16 +++++
3 files changed, 76 insertions(+), 18 deletions(-)
diff --git a/core/modules/image/image.module b/core/modules/image/image.module
index 4203045b5952..0d8bd4b1f265 100644
--- a/core/modules/image/image.module
+++ b/core/modules/image/image.module
@@ -13,6 +13,7 @@
use Drupal\field\FieldConfigInterface;
use Drupal\field\FieldStorageConfigInterface;
use Drupal\file\FileInterface;
+use Drupal\image\Controller\ImageStyleDownloadController;
use Drupal\image\Entity\ImageStyle;
/**
@@ -151,6 +152,18 @@ function image_file_download($uri) {
// Check that the file exists and is an image.
$image = \Drupal::service('image.factory')->get($uri);
if ($image->isValid()) {
+ // If the image style converted the extension, it has been added to the
+ // original file, resulting in filenames like image.png.jpeg. So to find
+ // the actual source image, we remove the extension and check if that
+ // image exists.
+ if (!file_exists($original_uri)) {
+ $converted_original_uri = ImageStyleDownloadController::getUriWithoutConvertedExtension($original_uri);
+ if ($converted_original_uri !== $original_uri && file_exists($converted_original_uri)) {
+ // The converted file does exist, use it as the source.
+ $original_uri = $converted_original_uri;
+ }
+ }
+
// Check the permissions of the original to grant access to this image.
$headers = \Drupal::moduleHandler()->invokeAll('file_download', [$original_uri]);
// Confirm there's at least one module granting access and none denying access.
diff --git a/core/modules/image/src/Controller/ImageStyleDownloadController.php b/core/modules/image/src/Controller/ImageStyleDownloadController.php
index 543700115ee0..bfc5c985d8dc 100644
--- a/core/modules/image/src/Controller/ImageStyleDownloadController.php
+++ b/core/modules/image/src/Controller/ImageStyleDownloadController.php
@@ -173,6 +173,24 @@ public function deliver(Request $request, $scheme, ImageStyleInterface $image_st
$headers = [];
+ // Don't try to generate file if source is missing.
+ if ($image_uri !== $sample_image_uri && !$this->sourceImageExists($image_uri, $token_is_valid)) {
+ // If the image style converted the extension, it has been added to the
+ // original file, resulting in filenames like image.png.jpeg. So to find
+ // the actual source image, we remove the extension and check if that
+ // image exists.
+ $converted_image_uri = static::getUriWithoutConvertedExtension($image_uri);
+ if ($converted_image_uri !== $image_uri &&
+ $this->sourceImageExists($converted_image_uri, $token_is_valid)) {
+ // The converted file does exist, use it as the source.
+ $image_uri = $converted_image_uri;
+ }
+ else {
+ $this->logger->notice('Source image at %source_image_path not found while trying to generate derivative image at %derivative_path.', ['%source_image_path' => $image_uri, '%derivative_path' => $derivative_uri]);
+ return new Response($this->t('Error generating image, missing source file.'), 404);
+ }
+ }
+
// If not using a public scheme, let other modules provide headers and
// control access to the file.
if (!$is_public) {
@@ -183,28 +201,12 @@ public function deliver(Request $request, $scheme, ImageStyleInterface $image_st
}
// If it is default sample.png, ignore scheme.
+ // This value swap must be done after hook_file_download is called since
+ // the hooks are expecting a URI, not a file path.
if ($image_uri === $sample_image_uri) {
$image_uri = $target;
}
- // Don't try to generate file if source is missing.
- if (!$this->sourceImageExists($image_uri, $token_is_valid)) {
- // If the image style converted the extension, it has been added to the
- // original file, resulting in filenames like image.png.jpeg. So to find
- // the actual source image, we remove the extension and check if that
- // image exists.
- $path_info = pathinfo(StreamWrapperManager::getTarget($image_uri));
- $converted_image_uri = sprintf('%s://%s%s%s', $this->streamWrapperManager->getScheme($derivative_uri), $path_info['dirname'], DIRECTORY_SEPARATOR, $path_info['filename']);
- if (!$this->sourceImageExists($converted_image_uri, $token_is_valid)) {
- $this->logger->notice('Source image at %source_image_path not found while trying to generate derivative image at %derivative_path.', ['%source_image_path' => $image_uri, '%derivative_path' => $derivative_uri]);
- return new Response($this->t('Error generating image, missing source file.'), 404);
- }
- else {
- // The converted file does exist, use it as the source.
- $image_uri = $converted_image_uri;
- }
- }
-
// Don't start generating the image if the derivative already exists or if
// generation is in progress in another thread.
if (!file_exists($derivative_uri)) {
@@ -283,4 +285,31 @@ private function sourceImageExists(string $image_uri, bool $token_is_valid): boo
return TRUE;
}
+ /**
+ * Get the file URI without the extension from any conversion image style.
+ *
+ * If the image style converted the image, then an extension has been added
+ * to the original file, resulting in filenames like image.png.jpeg.
+ *
+ * @param string $uri
+ * The file URI.
+ *
+ * @return string
+ * The file URI without the extension from any conversion image style.
+ */
+ public static function getUriWithoutConvertedExtension(string $uri): string {
+ $original_uri = $uri;
+ $path_info = pathinfo(StreamWrapperManager::getTarget($uri));
+ // Only convert the URI when the filename still has an extension.
+ if (!empty($path_info['filename']) && pathinfo($path_info['filename'], PATHINFO_EXTENSION)) {
+ $original_uri = StreamWrapperManager::getScheme($uri) . '://';
+ if (!empty($path_info['dirname']) && $path_info['dirname'] !== '.') {
+ $original_uri .= $path_info['dirname'] . DIRECTORY_SEPARATOR;
+ }
+ $original_uri .= $path_info['filename'];
+ }
+
+ return $original_uri;
+ }
+
}
diff --git a/core/modules/image/tests/src/Functional/ImageStylesPathAndUrlTest.php b/core/modules/image/tests/src/Functional/ImageStylesPathAndUrlTest.php
index 306684fbe684..f82e52e39608 100644
--- a/core/modules/image/tests/src/Functional/ImageStylesPathAndUrlTest.php
+++ b/core/modules/image/tests/src/Functional/ImageStylesPathAndUrlTest.php
@@ -122,6 +122,22 @@ public function testImageStyleUrlExtraSlash() {
$this->doImageStyleUrlAndPathTests('public', TRUE, TRUE);
}
+ /**
+ * Test an image style URL with a private file that also gets converted.
+ */
+ public function testImageStylePrivateWithConversion(): void {
+ // Add the "convert" image style effect to our style.
+ $this->style->addImageEffect([
+ 'uuid' => '',
+ 'id' => 'image_convert',
+ 'weight' => 1,
+ 'data' => [
+ 'extension' => 'jpeg',
+ ],
+ ]);
+ $this->doImageStyleUrlAndPathTests('private');
+ }
+
/**
* Tests that an invalid source image returns a 404.
*/
--
GitLab