diff --git a/core/modules/image/src/Controller/ImageStyleDownloadController.php b/core/modules/image/src/Controller/ImageStyleDownloadController.php index f66c269656..fb10f6767b 100644 --- a/core/modules/image/src/Controller/ImageStyleDownloadController.php +++ b/core/modules/image/src/Controller/ImageStyleDownloadController.php @@ -123,22 +123,48 @@ public static function create(ContainerInterface $container) { * Thrown when the file is still being generated. */ public function deliver(Request $request, $scheme, ImageStyleInterface $image_style) { - $target = $request->query->get('file'); - $image_uri = $scheme . '://' . $target; - // Check that the style is defined, return a 404 (Page Not Found) if // missing. if (empty($image_style)) { throw new NotFoundHttpException(); } + // Check that the URI scheme is valid, return a 404 (Page Not Found) if + // invalid. + if (!$this->streamWrapperManager->isValidScheme($scheme)) { + throw new NotFoundHttpException(); + } + + // Check that the source image file exists. If the image style converts + // the image format, the new format extension has been added to the original + // filename, resulting in filenames like image.png.jpeg. So to find the real + // source image, we remove the extension and check if that image exists. + $target = $request->query->get('file'); + $request_image_uri = $scheme . '://' . $target; + if (file_exists($request_image_uri)) { + $image_uri = $request_image_uri; + } + else { + $path_info = pathinfo(StreamWrapperManager::getTarget($request_image_uri)); + $dir_name = $path_info['dirname'] !== '.' ? $path_info['dirname'] . DIRECTORY_SEPARATOR : ''; + $original_image_uri = sprintf('%s://%s%s', $scheme, $dir_name, $path_info['filename']); + if (file_exists($original_image_uri)) { + $image_uri = $original_image_uri; + } + } + // Don't try to generate file if source is missing. + if (!isset($image_uri)) { + $this->logger->notice('Source image at %source_image_path not found while trying to generate derivative image.', ['%source_image_path' => $request_image_uri]); + return new Response($this->t('Error generating image, missing source file.'), 404); + } + // Create an image process pipeline. $pipeline = $this->imageProcessor->createInstance('derivative'); $pipeline ->setImageStyle($image_style) ->setSourceImageUri($image_uri); - // Check that the scheme is valid, and the image derivative token is valid. + // Check that the the image derivative token is valid. // Sites which require image derivatives to be generated without a token // can set the 'image.settings:allow_insecure_derivatives' configuration to // TRUE to bypass the latter check, but this will increase the site's @@ -148,7 +174,7 @@ public function deliver(Request $request, $scheme, ImageStyleInterface $image_st // The $target variable for a derivative of a style has // styles//... as structure, so we check if the $target variable // starts with styles/. - $valid = $this->streamWrapperManager->isValidScheme($scheme); + $valid = TRUE; if (!$this->config('image.settings')->get('allow_insecure_derivatives') || strpos(ltrim($target, '\/'), 'styles/') === 0) { $valid &= hash_equals($pipeline->getDerivativeImageUrlSecurityToken(), $request->query->get(IMAGE_DERIVATIVE_TOKEN, '')); } @@ -172,24 +198,6 @@ public function deliver(Request $request, $scheme, ImageStyleInterface $image_st } } - // Don't try to generate file if source is missing. - if (!file_exists($image_uri)) { - // 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 (!file_exists($converted_image_uri)) { - $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)) { diff --git a/core/modules/image/tests/src/Functional/ImageDimensionsTest.php b/core/modules/image/tests/src/Functional/ImageDimensionsTest.php index 1260d6ba97..396136b186 100644 --- a/core/modules/image/tests/src/Functional/ImageDimensionsTest.php +++ b/core/modules/image/tests/src/Functional/ImageDimensionsTest.php @@ -344,7 +344,7 @@ public function testImageDimensions() { $style->addImageEffect($effect); $style->save(); - $this->assertSame('', $this->getImageTag($variables)); + $this->assertSame('', $this->getImageTag($variables)); $this->assertFileNotExists($generated_uri, 'Generated file does not exist.'); $this->drupalGet($this->getAbsoluteUrl($url)); $this->assertSession()->statusCodeEquals(200); @@ -365,7 +365,7 @@ public function testImageDimensions() { $style->addImageEffect($effect); $style->save(); - $this->assertSame('', $this->getImageTag($variables)); + $this->assertSame('', $this->getImageTag($variables)); $this->assertFileNotExists($generated_uri, 'Generated file does not exist.'); $this->drupalGet($this->getAbsoluteUrl($url)); $this->assertSession()->statusCodeEquals(200); @@ -387,7 +387,7 @@ public function testImageDimensions() { $style->addImageEffect($effect); $style->save(); - $this->assertSame('', $this->getImageTag($variables)); + $this->assertSame('', $this->getImageTag($variables)); $this->assertFileNotExists($generated_uri, 'Generated file does not exist.'); $this->drupalGet($this->getAbsoluteUrl($url)); $this->assertSession()->statusCodeEquals(200); @@ -409,7 +409,7 @@ public function testImageDimensions() { $style->addImageEffect($effect); $style->save(); - $this->assertSame('', $this->getImageTag($variables)); + $this->assertSame('', $this->getImageTag($variables)); $this->assertFileNotExists($generated_uri, 'Generated file does not exist.'); $this->drupalGet($this->getAbsoluteUrl($url)); $this->assertSession()->statusCodeEquals(200); @@ -427,7 +427,7 @@ public function testImageDimensions() { $style->addImageEffect($effect); $style->save(); - $this->assertSame('', $this->getImageTag($variables)); + $this->assertSame('', $this->getImageTag($variables)); $this->assertFileNotExists($generated_uri, 'Generated file does not exist.'); $this->drupalGet($this->getAbsoluteUrl($url)); $this->assertSession()->statusCodeEquals(200); @@ -467,7 +467,7 @@ public function testImageDimensions() { $style->addImageEffect($effect); $style->save(); - $this->assertEqual('', $this->getImageTag($variables)); + $this->assertEqual('', $this->getImageTag($variables)); $this->assertFileNotExists($generated_uri, 'Generated file does not exist.'); $this->drupalGet($this->getAbsoluteUrl($url)); $this->assertSession()->statusCodeEquals(200); @@ -538,7 +538,7 @@ public function testImageDimensions() { $generated_uri = 'public://styles/test_uri/public/' . $file_system->basename($original_uri); $url = file_url_transform_relative($pipeline->getDerivativeImageUrl()->toString()); - $this->assertSame('', $this->getImageTag($variables)); + $this->assertSame('', $this->getImageTag($variables)); $this->assertFileNotExists($generated_uri, 'Generated file does not exist.'); $this->drupalGet($this->getAbsoluteUrl($url)); $this->assertSession()->statusCodeEquals(200); @@ -555,7 +555,7 @@ public function testImageDimensions() { $generated_uri = 'public://styles/test_uri/public/' . $file_system->basename($original_uri); $url = file_url_transform_relative($pipeline->getDerivativeImageUrl()->toString()); $variables['#uri'] = $original_uri; - $this->assertSame('', $this->getImageTag($variables)); + $this->assertSame('', $this->getImageTag($variables)); $this->assertFileNotExists($generated_uri, 'Generated file does not exist.'); $this->drupalGet($this->getAbsoluteUrl($url)); $this->assertSession()->statusCodeEquals(200); diff --git a/core/modules/image/tests/src/Functional/ImageEffect/ConvertTest.php b/core/modules/image/tests/src/Functional/ImageEffect/ConvertTest.php index 5b9d8bc4c1..29865dde20 100644 --- a/core/modules/image/tests/src/Functional/ImageEffect/ConvertTest.php +++ b/core/modules/image/tests/src/Functional/ImageEffect/ConvertTest.php @@ -48,11 +48,12 @@ public function testConvertFileInRoot() { // Execute the image style on the test image via a GET request. $pipeline = \Drupal::service('image.processor')->createInstance('derivative') - ->setImageStyle(ImageStyle::load('image_effect_test')) + ->setImageStyle($image_style) ->setSourceImageUri($test_uri); $derivative_uri = 'public://styles/image_effect_test/public/image-test-do.png.jpeg'; $this->assertFileNotExists($derivative_uri); - $this->drupalGet($pipeline->getDerivativeImageUrl()); + $url = file_url_transform_relative($pipeline->getDerivativeImageUrl()->toString()); + $this->drupalGet($this->getAbsoluteUrl($url)); $this->assertSession()->statusCodeEquals(200); $this->assertFileExists($derivative_uri); }