From aa12b0e73fae39513745e5b0d378af45090cb67c Mon Sep 17 00:00:00 2001 From: Alex Pott <alex.a.pott@googlemail.com> Date: Mon, 5 Oct 2020 13:06:18 +0100 Subject: [PATCH] Issue #2630230 by mondrake, ayushmishra206, chr.fritsch, eiriksm, borisson_, msuthars, jjcarrion, mohit1604, marvin_B8, mikelutz, vacho, vakulrai, larowlan: Image effect convert fails when image file is in the public files root --- .../ImageStyleDownloadController.php | 26 +++++++-- .../Functional/ImageEffect/ConvertTest.php | 58 +++++++++++++++++++ 2 files changed, 80 insertions(+), 4 deletions(-) create mode 100644 core/modules/image/tests/src/Functional/ImageEffect/ConvertTest.php diff --git a/core/modules/image/src/Controller/ImageStyleDownloadController.php b/core/modules/image/src/Controller/ImageStyleDownloadController.php index c7cf65cdf457..f97358b445a3 100644 --- a/core/modules/image/src/Controller/ImageStyleDownloadController.php +++ b/core/modules/image/src/Controller/ImageStyleDownloadController.php @@ -3,8 +3,10 @@ namespace Drupal\image\Controller; use Drupal\Component\Utility\Crypt; +use Drupal\Core\File\FileSystemInterface; use Drupal\Core\Image\ImageFactory; use Drupal\Core\Lock\LockBackendInterface; +use Drupal\Core\StreamWrapper\StreamWrapperManager; use Drupal\Core\StreamWrapper\StreamWrapperManagerInterface; use Drupal\image\ImageStyleInterface; use Drupal\system\FileDownloadController; @@ -42,6 +44,13 @@ class ImageStyleDownloadController extends FileDownloadController { */ protected $logger; + /** + * File system service, + * + * @var \Drupal\Core\File\FileSystemInterface + */ + protected $fileSystem; + /** * Constructs a ImageStyleDownloadController object. * @@ -51,12 +60,20 @@ class ImageStyleDownloadController extends FileDownloadController { * The image factory. * @param \Drupal\Core\StreamWrapper\StreamWrapperManagerInterface $stream_wrapper_manager * The stream wrapper manager. + * @param \Drupal\Core\File\FileSystemInterface $file_system + * The system service. */ - public function __construct(LockBackendInterface $lock, ImageFactory $image_factory, StreamWrapperManagerInterface $stream_wrapper_manager = NULL) { + public function __construct(LockBackendInterface $lock, ImageFactory $image_factory, StreamWrapperManagerInterface $stream_wrapper_manager = NULL, FileSystemInterface $file_system = NULL) { parent::__construct($stream_wrapper_manager); $this->lock = $lock; $this->imageFactory = $image_factory; $this->logger = $this->getLogger('image'); + + if (!isset($file_system)) { + @trigger_error('Not defining the $file_system argument to ' . __METHOD__ . ' is deprecated in drupal:9.1.0 and will throw an error in drupal:10.0.0.', E_USER_DEPRECATED); + $file_system = \Drupal::service('file_system'); + } + $this->fileSystem = $file_system; } /** @@ -66,7 +83,8 @@ public static function create(ContainerInterface $container) { return new static( $container->get('lock'), $container->get('image.factory'), - $container->get('stream_wrapper_manager') + $container->get('stream_wrapper_manager'), + $container->get('file_system') ); } @@ -137,8 +155,8 @@ public function deliver(Request $request, $scheme, ImageStyleInterface $image_st // 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($image_uri); - $converted_image_uri = $path_info['dirname'] . DIRECTORY_SEPARATOR . $path_info['filename']; + $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); diff --git a/core/modules/image/tests/src/Functional/ImageEffect/ConvertTest.php b/core/modules/image/tests/src/Functional/ImageEffect/ConvertTest.php new file mode 100644 index 000000000000..e926f3cd67ff --- /dev/null +++ b/core/modules/image/tests/src/Functional/ImageEffect/ConvertTest.php @@ -0,0 +1,58 @@ +<?php + +namespace Drupal\Tests\image\Functional\ImageEffect; + +use Drupal\Core\File\FileSystemInterface; +use Drupal\image\Entity\ImageStyle; +use Drupal\Tests\BrowserTestBase; + +/** + * Tests for the Convert image effect. + * + * @group image + */ +class ConvertTest extends BrowserTestBase { + + /** + * {@inheritdoc} + */ + protected $defaultTheme = 'classy'; + + /** + * {@inheritdoc} + */ + protected static $modules = ['image']; + + /** + * Tests that files stored in the root folder are converted properly. + */ + public function testConvertFileInRoot() { + // Create the test image style with a Convert effect. + $image_style = ImageStyle::create([ + 'name' => 'image_effect_test', + 'label' => 'Image Effect Test', + ]); + $this->assertEquals(SAVED_NEW, $image_style->save()); + $image_style->addImageEffect([ + 'id' => 'image_convert', + 'data' => [ + 'extension' => 'jpeg', + ], + ]); + $this->assertEquals(SAVED_UPDATED, $image_style->save()); + + // Create a copy of a test image file in root. + $test_uri = 'public://image-test-do.png'; + \Drupal::service('file_system')->copy('core/tests/fixtures/files/image-test.png', $test_uri, FileSystemInterface::EXISTS_REPLACE); + $this->assertFileExists($test_uri); + + // Execute the image style on the test image via a GET request. + $derivative_uri = 'public://styles/image_effect_test/public/image-test-do.png.jpeg'; + $this->assertFileNotExists($derivative_uri); + $url = file_url_transform_relative($image_style->buildUrl($test_uri)); + $this->drupalGet($this->getAbsoluteUrl($url)); + $this->assertSession()->statusCodeEquals(200); + $this->assertFileExists($derivative_uri); + } + +} -- GitLab