Issue #2770761 by claudiu.cristea, mondrake: Derivatives should not be created...

Issue #2770761 by claudiu.cristea, mondrake: Derivatives should not be created on read-only stream wrappers

core/modules/file/tests/file_test/src/StreamWrapper/DummyRemoteReadOnlyStreamWrapper.php

+<?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.');
+  }
+
+}

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');
+  }
+
 }

core/modules/image/tests/src/Kernel/ImageStyleCustomStreamWrappersTest.php

+<?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],
+    ];
+  }
+
+}