Issue #2786735 by recrit, eiriksm, ranjith_kumar_k_u, immaculatexavier,...

Issue #2786735 by recrit, eiriksm, ranjith_kumar_k_u, immaculatexavier, ravi.shankar, zaporylie, smustgrave, TrevorBradley, johnle, GaëlG, jefuri, alexpott, poker10: Image derivative generation does not work if effect "Convert" in use and file stored in private filesystem

(cherry picked from commit 31f9e8a9)

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.

core/modules/image/src/Controller/ImageStyleDownloadController.php

@@ -167,6 +167,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) {
@@ -177,28 +195,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)) {
@@ -277,4 +279,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;
+  }
+
 }

core/modules/image/tests/src/Functional/ImageStylesPathAndUrlTest.php

@@ -119,6 +119,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.
    */