From f18da3e585b62db37f0555b589ffbd7f1a966cad Mon Sep 17 00:00:00 2001 From: Alex Pott <alex.a.pott@googlemail.com> Date: Mon, 1 Aug 2016 13:15:55 +0100 Subject: [PATCH] Issue #2770761 by claudiu.cristea, mondrake: Derivatives should not be created on read-only stream wrappers --- .../DummyRemoteReadOnlyStreamWrapper.php | 33 ++++++ core/modules/image/src/Entity/ImageStyle.php | 37 +++++- .../ImageStyleCustomStreamWrappersTest.php | 111 ++++++++++++++++++ 3 files changed, 175 insertions(+), 6 deletions(-) create mode 100644 core/modules/file/tests/file_test/src/StreamWrapper/DummyRemoteReadOnlyStreamWrapper.php create mode 100644 core/modules/image/tests/src/Kernel/ImageStyleCustomStreamWrappersTest.php diff --git a/core/modules/file/tests/file_test/src/StreamWrapper/DummyRemoteReadOnlyStreamWrapper.php b/core/modules/file/tests/file_test/src/StreamWrapper/DummyRemoteReadOnlyStreamWrapper.php new file mode 100644 index 000000000000..51913b8ce156 --- /dev/null +++ b/core/modules/file/tests/file_test/src/StreamWrapper/DummyRemoteReadOnlyStreamWrapper.php @@ -0,0 +1,33 @@ +<?php + +namespace Drupal\file_test\StreamWrapper; + +use Drupal\Core\StreamWrapper\StreamWrapperInterface; + +/** + * Dummy read-only remote stream wrapper (dummy-remote-readonly://). + */ +class DummyRemoteReadOnlyStreamWrapper extends DummyRemoteStreamWrapper { + + /** + * {@inheritdoc} + */ + public static function getType() { + return StreamWrapperInterface::READ_VISIBLE; + } + + /** + * {@inheritdoc} + */ + public function getName() { + return t('Dummy remote read-only files'); + } + + /** + * {@inheritdoc} + */ + public function getDescription() { + return t('Dummy remote read-only stream wrapper for testing.'); + } + +} diff --git a/core/modules/image/src/Entity/ImageStyle.php b/core/modules/image/src/Entity/ImageStyle.php index e5460ee352b7..e72258da6885 100644 --- a/core/modules/image/src/Entity/ImageStyle.php +++ b/core/modules/image/src/Entity/ImageStyle.php @@ -162,15 +162,28 @@ protected static function replaceImageStyle(ImageStyleInterface $style) { * {@inheritdoc} */ public function buildUri($uri) { - $scheme = $this->fileUriScheme($uri); - if ($scheme) { + $source_scheme = $scheme = $this->fileUriScheme($uri); + $default_scheme = $this->fileDefaultScheme(); + + if ($source_scheme) { $path = $this->fileUriTarget($uri); + // The scheme of derivative image files only needs to be computed for + // source files not stored in the default scheme. + if ($source_scheme != $default_scheme) { + $class = $this->getStreamWrapperManager()->getClass($source_scheme); + $is_writable = $class::getType() & StreamWrapperInterface::WRITE; + + // Compute the derivative URI scheme. Derivatives created from writable + // source stream wrappers will inherit the scheme. Derivatives created + // from read-only stream wrappers will fall-back to the default scheme. + $scheme = $is_writable ? $source_scheme : $default_scheme; + } } else { $path = $uri; - $scheme = $this->fileDefaultScheme(); + $source_scheme = $scheme = $default_scheme; } - return $scheme . '://styles/' . $this->id() . '/' . $scheme . '/' . $this->addExtension($path); + return "$scheme://styles/{$this->id()}/$source_scheme/{$this->addExtension($path)}"; } /** @@ -211,7 +224,7 @@ public function buildUrl($path, $clean_urls = NULL) { // to the actual file path, this avoids bootstrapping PHP once the files are // built. if ($clean_urls === FALSE && file_uri_scheme($uri) == 'public' && !file_exists($uri)) { - $directory_path = \Drupal::service('stream_wrapper_manager')->getViaUri($uri)->getDirectoryPath(); + $directory_path = $this->getStreamWrapperManager()->getViaUri($uri)->getDirectoryPath(); return Url::fromUri('base:' . $directory_path . '/' . file_uri_target($uri), array('absolute' => TRUE, 'query' => $token_query))->toString(); } @@ -238,7 +251,7 @@ public function flush($path = NULL) { } // Delete the style directory in each registered wrapper. - $wrappers = \Drupal::service('stream_wrapper_manager')->getWrappers(StreamWrapperInterface::WRITE_VISIBLE); + $wrappers = $this->getStreamWrapperManager()->getWrappers(StreamWrapperInterface::WRITE_VISIBLE); foreach ($wrappers as $wrapper => $wrapper_data) { if (file_exists($directory = $wrapper . '://styles/' . $this->id())) { file_unmanaged_delete_recursive($directory); @@ -495,4 +508,16 @@ protected function fileDefaultScheme() { return file_default_scheme(); } + /** + * Gets the stream wrapper manager service. + * + * @return \Drupal\Core\StreamWrapper\StreamWrapperManagerInterface + * The stream wrapper manager service + * + * @todo Properly inject this service in Drupal 9.0.x. + */ + protected function getStreamWrapperManager() { + return \Drupal::service('stream_wrapper_manager'); + } + } diff --git a/core/modules/image/tests/src/Kernel/ImageStyleCustomStreamWrappersTest.php b/core/modules/image/tests/src/Kernel/ImageStyleCustomStreamWrappersTest.php new file mode 100644 index 000000000000..6a45a98680f9 --- /dev/null +++ b/core/modules/image/tests/src/Kernel/ImageStyleCustomStreamWrappersTest.php @@ -0,0 +1,111 @@ +<?php + +namespace Drupal\Tests\image\Kernel; + +use Drupal\Core\DependencyInjection\ContainerBuilder; +use Drupal\Core\StreamWrapper\PrivateStream; +use Drupal\Core\StreamWrapper\PublicStream; +use Drupal\file_test\StreamWrapper\DummyReadOnlyStreamWrapper; +use Drupal\file_test\StreamWrapper\DummyRemoteReadOnlyStreamWrapper; +use Drupal\file_test\StreamWrapper\DummyStreamWrapper; +use Drupal\image\Entity\ImageStyle; +use Drupal\KernelTests\KernelTestBase; + +/** + * Tests derivative generation with source images using stream wrappers. + * + * @group image + */ +class ImageStyleCustomStreamWrappersTest extends KernelTestBase { + + /** + * Modules to enable. + * + * @var string[] + */ + public static $modules = ['system', 'image']; + + /** + * A testing image style entity. + * + * @var \Drupal\image\ImageStyleInterface + */ + protected $imageStyle; + + /** + * The file system service. + * + * @var \Drupal\Core\File\FileSystemInterface + */ + protected $fileSystem; + + /** + * {@inheritdoc} + */ + protected function setUp() { + parent::setUp(); + $this->fileSystem = $this->container->get('file_system'); + $this->config('system.file')->set('default_scheme', 'public')->save(); + $this->imageStyle = ImageStyle::create(['name' => 'test']); + $this->imageStyle->save(); + } + + /** + * {@inheritdoc} + */ + public function register(ContainerBuilder $container) { + parent::register($container); + foreach ($this->providerTestCustomStreamWrappers() as $stream_wrapper) { + $scheme = $stream_wrapper[0]; + $class = $stream_wrapper[2]; + $container->register("stream_wrapper.$scheme", $class) + ->addTag('stream_wrapper', ['scheme' => $scheme]); + } + } + + /** + * Tests derivative creation with several source on a local writable stream. + * + * @dataProvider providerTestCustomStreamWrappers + * + * @param string $source_scheme + * The source stream wrapper scheme. + * @param string $expected_scheme + * The derivative expected stream wrapper scheme. + */ + public function testCustomStreamWrappers($source_scheme, $expected_scheme) { + $derivative_uri = $this->imageStyle->buildUri("$source_scheme://some/path/image.png"); + $derivative_scheme = $this->fileSystem->uriScheme($derivative_uri); + + // Check that the derivative scheme is the expected scheme. + $this->assertSame($expected_scheme, $derivative_scheme); + + // Check that the derivative URI is the expected one. + $expected_uri = "$expected_scheme://styles/{$this->imageStyle->id()}/$source_scheme/some/path/image.png"; + $this->assertSame($expected_uri, $derivative_uri); + } + + /** + * Provide test cases for testCustomStreamWrappers(). + * + * Derivatives created from writable source stream wrappers will inherit the + * scheme from source. Derivatives created from read-only stream wrappers will + * fall-back to the default scheme. + * + * @return array[] + * An array having each element an array with three items: + * - The source stream wrapper scheme. + * - The derivative expected stream wrapper scheme. + * - The stream wrapper service class. + */ + public function providerTestCustomStreamWrappers() { + return [ + ['public', 'public', PublicStream::class], + ['private', 'private', PrivateStream::class], + ['dummy', 'dummy', DummyStreamWrapper::class], + ['dummy-readonly', 'public', DummyReadOnlyStreamWrapper::class], + ['dummy-remote-readonly', 'public', DummyRemoteReadOnlyStreamWrapper::class], + ]; + } + +} -- GitLab