Revert "Issue #3080666 by phenaproxima, Upchuk, ieguskiza, ravi.shankar,...

Revert "Issue #3080666 by phenaproxima, Upchuk, ieguskiza, ravi.shankar, nikitagupta, zipymonkey, larowlan, Bladedu, pameeela, Gauravmahlawat, Spokje, mukesh.dev, seanB: oEmbed system doesn't work if thumbnail url does not have a file extension"

This reverts commit 8388b9b7.

core/modules/media/src/Plugin/media/Source/OEmbed.php

@@ -26,8 +26,6 @@
 use GuzzleHttp\Exception\TransferException;
 use Psr\Log\LoggerInterface;
 use Symfony\Component\DependencyInjection\ContainerInterface;
-use Symfony\Component\Finder\Finder;
-use Symfony\Component\Mime\MimeTypes;
 
 /**
  * Provides a media source plugin for oEmbed resources.
@@ -390,11 +388,21 @@ protected function getLocalThumbnailUri(Resource $resource) {
     if (!$remote_thumbnail_url) {
       return NULL;
     }
+    $remote_thumbnail_url = $remote_thumbnail_url->toString();
+
+    // Remove the query string, since we do not want to include it in the local
+    // thumbnail URI.
+    $local_thumbnail_url = parse_url($remote_thumbnail_url, PHP_URL_PATH);
 
-    // Ensure that we can write to the local directory where thumbnails are
-    // stored.
+    // Compute the local thumbnail URI, regardless of whether or not it exists.
     $configuration = $this->getConfiguration();
     $directory = $configuration['thumbnails_directory'];
+    $local_thumbnail_uri = "$directory/" . Crypt::hashBase64($local_thumbnail_url) . '.' . pathinfo($local_thumbnail_url, PATHINFO_EXTENSION);
+
+    // If the local thumbnail already exists, return its URI.
+    if (file_exists($local_thumbnail_uri)) {
+      return $local_thumbnail_uri;
+    }
 
     // The local thumbnail doesn't exist yet, so try to download it. First,
     // ensure that the destination directory is writable, and if it's not,
@@ -406,25 +414,8 @@ protected function getLocalThumbnailUri(Resource $resource) {
       return NULL;
     }
 
-    // The local filename of the thumbnail is always a hash of its remote URL.
-    // If a file with that name already exists in the thumbnails directory,
-    // regardless of its extension, return its URI.
-    $remote_thumbnail_url = $remote_thumbnail_url->toString();
-    $hash = Crypt::hashBase64($remote_thumbnail_url);
-    $finder = Finder::create()
-      ->files()
-      ->in($directory)
-      ->name("$hash.*");
-    if (count($finder) > 0) {
-      /** @var \Symfony\Component\Finder\SplFileInfo[] $files */
-      $files = iterator_to_array($finder);
-      return reset($files)->getPathname();
-    }
-
-    // The local thumbnail doesn't exist yet, so we need to download it.
-    $local_thumbnail_uri = $directory . DIRECTORY_SEPARATOR . $hash . '.' . $this->getThumbnailFileExtensionFromUrl($remote_thumbnail_url);
     try {
-      $response = $this->httpClient->request('GET', $remote_thumbnail_url);
+      $response = $this->httpClient->get($remote_thumbnail_url);
       if ($response->getStatusCode() === 200) {
         $this->fileSystem->saveData((string) $response->getBody(), $local_thumbnail_uri, FileSystemInterface::EXISTS_REPLACE);
         return $local_thumbnail_uri;
@@ -441,49 +432,6 @@ protected function getLocalThumbnailUri(Resource $resource) {
     return NULL;
   }
 
-  /**
-   * Tries to determine the file extension of a thumbnail.
-   *
-   * @param string $thumbnail_url
-   *   The remote URL of the thumbnail.
-   *
-   * @return string|null
-   *   The file extension, or NULL if it could not be determined.
-   */
-  protected function getThumbnailFileExtensionFromUrl(string $thumbnail_url): ?string {
-    // First, try to glean the extension from the URL path.
-    $path = parse_url($thumbnail_url, PHP_URL_PATH);
-    if ($path) {
-      $extension = strtolower(pathinfo($path, PATHINFO_EXTENSION));
-      if ($extension) {
-        return $extension;
-      }
-    }
-
-    // If the URL didn't give us any clues about the file extension, make a HEAD
-    // request to the thumbnail URL and see if the headers will give us a MIME
-    // type.
-    try {
-      $content_type = $this->httpClient->request('HEAD', $thumbnail_url)
-        ->getHeader('Content-Type');
-    }
-    catch (TransferException $e) {
-      $this->logger->warning($e->getMessage());
-    }
-
-    // If there was no Content-Type header, there's nothing else we can do.
-    if (empty($content_type)) {
-      return NULL;
-    }
-    $extensions = MimeTypes::getDefault()->getExtensions(reset($content_type));
-    if ($extensions) {
-      return reset($extensions);
-    }
-    // If no file extension could be determined from the Content-Type header,
-    // we're stumped.
-    return NULL;
-  }
-
   /**
    * {@inheritdoc}
    */

core/modules/media/tests/fixtures/oembed/video_collegehumor.xml

@@ -12,10 +12,7 @@
   <height>343</height>
   <html><h1>By the power of Grayskull, CollegeHumor works!</h1>
   </html>
-  <!-- The thumbnail URL does not contain a file extension, so we use
-  this to test the oEmbed source plugin's thumbnail handling;
-  see \Drupal\Tests\media\FunctionalJavascript\MediaSourceOEmbedVideoTest. -->
-  <thumbnail_url>internal:/media_test_oembed/thumbnail</thumbnail_url>
+  <thumbnail_url>internal:/core/misc/druplicon.png</thumbnail_url>
   <thumbnail_width>88</thumbnail_width>
   <thumbnail_height>100</thumbnail_height>
 </oembed>

core/modules/media/tests/modules/media_test_oembed/media_test_oembed.routing.yml

@@ -4,9 +4,3 @@ media_test_oembed.resource.get:
     _controller: '\Drupal\media_test_oembed\Controller\ResourceController::get'
   requirements:
     _access: 'TRUE'
-media_test_oembed.resource.thumbnail:
-  path: '/media_test_oembed/thumbnail'
-  defaults:
-    _controller: '\Drupal\media_test_oembed\Controller\ResourceController::getThumbnailWithNoExtension'
-  requirements:
-    _access: 'TRUE'

core/modules/media/tests/modules/media_test_oembed/src/Controller/ResourceController.php

@@ -2,7 +2,6 @@
 
 namespace Drupal\media_test_oembed\Controller;
 
-use Symfony\Component\HttpFoundation\BinaryFileResponse;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
 
@@ -31,24 +30,12 @@ public function get(Request $request) {
     else {
       $content = file_get_contents($resources[$asset_url]);
       $response = new Response($content);
-      $response->headers->set('Content-Type', 'application/' . pathinfo($resources[$asset_url], PATHINFO_EXTENSION));
+      $response->headers->set('Content-Type', 'application/json');
     }
 
     return $response;
   }
 
-  /**
-   * Returns an example thumbnail file without an extension.
-   *
-   * @return \Symfony\Component\HttpFoundation\BinaryFileResponse
-   *   The response.
-   */
-  public function getThumbnailWithNoExtension() {
-    $response = new BinaryFileResponse('core/misc/druplicon.png');
-    $response->headers->set('Content-Type', 'image/png');
-    return $response;
-  }
-
   /**
    * Maps an asset URL to a local fixture representing its oEmbed resource.
    *

core/modules/media/tests/src/FunctionalJavascript/MediaSourceOEmbedVideoTest.php

@@ -4,7 +4,6 @@
 
 use Drupal\Core\Session\AccountInterface;
 use Drupal\media\Entity\Media;
-use Drupal\media\Entity\MediaType;
 use Drupal\media_test_oembed\Controller\ResourceController;
 use Drupal\Tests\media\Traits\OEmbedTestTrait;
 use Drupal\user\Entity\Role;
@@ -165,29 +164,6 @@ public function testMediaOEmbedVideoSource() {
 
     $assert_session->pageTextContains('The CollegeHumor provider is not allowed.');
 
-    // Register a CollegeHumor video as a second oEmbed resource. Note that its
-    // thumbnail URL does not have a file extension.
-    $media_type = MediaType::load($media_type_id);
-    $source_configuration = $media_type->getSource()->getConfiguration();
-    $source_configuration['providers'][] = 'CollegeHumor';
-    $media_type->getSource()->setConfiguration($source_configuration);
-    $media_type->save();
-    $video_url = 'http://www.collegehumor.com/video/40003213/let-not-get-a-drink-sometime';
-    ResourceController::setResourceUrl($video_url, $this->getFixturesDirectory() . '/video_collegehumor.xml');
-
-    // Create a new media item using a CollegeHumor video.
-    $this->drupalGet("media/add/$media_type_id");
-    $assert_session->fieldExists('Remote video URL')->setValue($video_url);
-    $assert_session->buttonExists('Save')->press();
-
-    /** @var \Drupal\media\MediaInterface $media */
-    $media = Media::load(2);
-    $thumbnail = $media->getSource()->getMetadata($media, 'thumbnail_uri');
-    $this->assertFileExists($thumbnail);
-    // Although the resource's thumbnail URL doesn't have a file extension, we
-    // should have deduced the correct one.
-    $this->assertStringEndsWith('.png', $thumbnail);
-
     // Test anonymous access to media via iframe.
     $this->drupalLogout();
 

core/modules/media/tests/src/Kernel/OEmbedSourceTest.php

@@ -3,7 +3,7 @@
 namespace Drupal\Tests\media\Kernel;
 
 use Drupal\Component\Utility\Crypt;
-use Drupal\Core\Logger\RfcLogLevel;
+use Drupal\Core\Url;
 use Drupal\media\Entity\Media;
 use Drupal\media\OEmbed\Resource;
 use Drupal\media\OEmbed\ResourceFetcherInterface;
@@ -11,7 +11,6 @@
 use Drupal\media\Plugin\media\Source\OEmbed;
 use GuzzleHttp\Client;
 use GuzzleHttp\Psr7\Response;
-use GuzzleHttp\Psr7\Utils;
 use Prophecy\Argument;
 
 /**
@@ -44,62 +43,9 @@ public function testGetMetadata() {
   }
 
   /**
-   * Data provider for ::testThumbnailUri().
-   *
-   * @return array
-   *   Sets of arguments to pass to the test method.
-   */
-  public function providerThumbnailUri(): array {
-    return [
-      'no query string, file extension is known' => [
-        'internal:/core/misc/druplicon.png',
-      ],
-      'with query string and file extension' => [
-        'internal:/core/misc/druplicon.png?foo=bar',
-      ],
-      'no query string, unknown file extension' => [
-        'internal:/core/misc/druplicon',
-        [
-          'Content-Type' => ['image/png'],
-        ],
-      ],
-      'query string, unknown file extension' => [
-        'internal:/core/misc/druplicon?pasta=ravioli',
-        [
-          'Content-Type' => ['image/png'],
-        ],
-      ],
-      'no query string, unknown file extension, exception' => [
-        'internal:/core/misc/druplicon',
-        '\GuzzleHttp\Exception\TransferException',
-      ],
-    ];
-  }
-
-  /**
-   * Tests that remote thumbnails are downloaded correctly.
-   *
-   * @param string $remote_thumbnail_url
-   *   The URL of the remote thumbnail. This will be wired up to a mocked
-   *   response containing the data from core/misc/druplicon.png.
-   * @param array[]|string $thumbnail_headers
-   *   (optional) If the thumbnail's file extension cannot be determined from
-   *   its URL, a HEAD request will be made in an attempt to derive its
-   *   extension from its Content-Type header. If this is an array, it contains
-   *   headers that should be returned by the HEAD request, where the keys are
-   *   header names and the values are arrays of strings. If it's the name of a
-   *   throwable class, it is the exception class that should be thrown by the
-   *   HTTP client.
-   *
    * @covers ::getLocalThumbnailUri
-   *
-   * @dataProvider providerThumbnailUri
    */
-  public function testThumbnailUri(string $remote_thumbnail_url, $thumbnail_headers = NULL): void {
-    // Create a fake resource with the given thumbnail URL.
-    $resource = Resource::rich('<html></html>', 16, 16, NULL, 'Test resource', NULL, NULL, NULL, $remote_thumbnail_url, 16, 16);
-    $thumbnail_url = $resource->getThumbnailUrl()->toString();
-
+  public function testLocalThumbnailUriQueryStringIsIgnored() {
     // There's no need to resolve the resource URL in this test; we just need
     // to fetch the resource.
     $this->container->set(
@@ -107,55 +53,24 @@ public function testThumbnailUri(string $remote_thumbnail_url, $thumbnail_header
       $this->prophesize(UrlResolverInterface::class)->reveal()
     );
 
-    // Mock the resource fetcher so that it will return our fake resource.
-    $resource_fetcher = $this->prophesize(ResourceFetcherInterface::class);
-    $resource_fetcher->fetchResource(Argument::any())
-      ->willReturn($resource);
-    $this->container->set('media.oembed.resource_fetcher', $resource_fetcher->reveal());
+    $thumbnail_url = Url::fromUri('internal:/core/misc/druplicon.png?foo=bar');
+
+    // Create a mocked resource whose thumbnail URL contains a query string.
+    $resource = $this->prophesize(Resource::class);
+    $resource->getTitle()->willReturn('Test resource');
+    $resource->getThumbnailUrl()->willReturn($thumbnail_url);
 
     // The source plugin will try to fetch the remote thumbnail, so mock the
-    // HTTP client to ensure that request returns a response with some valid
-    // image data.
-    $data = Utils::tryFopen($this->getDrupalRoot() . '/core/misc/druplicon.png', 'r');
-    $response = new Response(200, [], Utils::streamFor($data));
+    // HTTP client to ensure that request returns an empty "OK" response.
     $http_client = $this->prophesize(Client::class);
-    // The thumbnail should only be downloaded once.
-    $http_client->request('GET', $thumbnail_url)->willReturn($response)
-      ->shouldBeCalledOnce();
-    // The extension we expect the downloaded thumbnail to have.
-    $expected_extension = 'png';
-
-    // If the file extension cannot be derived from the URL, a single HEAD
-    // request should be made to try and determine its type from the
-    // Content-Type HTTP header.
-    if (is_array($thumbnail_headers)) {
-      $response = new Response(200, $thumbnail_headers);
-      $http_client->request('HEAD', $thumbnail_url)
-        ->willReturn($response)
-        ->shouldBeCalledOnce();
-    }
-    elseif (is_a($thumbnail_headers, 'Throwable', TRUE)) {
-      $e = new $thumbnail_headers('Nope!');
-
-      $http_client->request('HEAD', $thumbnail_url)
-        ->willThrow($e)
-        ->shouldBeCalledOnce();
-
-      // Ensure that the exception is logged.
-      $logger = $this->prophesize('\Psr\Log\LoggerInterface');
-      $logger->log(RfcLogLevel::WARNING, $e->getMessage(), Argument::cetera())
-        ->shouldBeCalled();
-      $this->container->get('logger.factory')->addLogger($logger->reveal());
-
-      // If the request fails, we won't be able to determine the thumbnail's
-      // extension.
-      $expected_extension = '';
-    }
-    else {
-      $http_client->request('HEAD', $thumbnail_url)->shouldNotBeCalled();
-    }
+    $http_client->get(Argument::type('string'))->willReturn(new Response());
     $this->container->set('http_client', $http_client->reveal());
 
+    // Mock the resource fetcher so that it will return our mocked resource.
+    $resource_fetcher = $this->prophesize(ResourceFetcherInterface::class);
+    $resource_fetcher->fetchResource(NULL)->willReturn($resource->reveal());
+    $this->container->set('media.oembed.resource_fetcher', $resource_fetcher->reveal());
+
     $media_type = $this->createMediaType('oembed:video');
     $source = $media_type->getSource();
 
@@ -165,18 +80,11 @@ public function testThumbnailUri(string $remote_thumbnail_url, $thumbnail_header
     ]);
     $media->save();
 
-    // The thumbnail should have a file extension, even if it wasn't in the URL.
-    $expected_uri = 'public://oembed_thumbnails/' . Crypt::hashBase64($thumbnail_url) . ".$expected_extension";
-    $this->assertSame($expected_uri, $source->getMetadata($media, 'thumbnail_uri'));
-    // Even if we get the thumbnail_uri more than once, it should only be
-    // downloaded once (this is verified by the shouldBeCalledOnce() checks
-    // in the mocked HTTP client).
-    $source->getMetadata($media, 'thumbnail_uri');
-    // The downloaded thumbnail should be usable by the image toolkit.
-    $this->assertFileExists($expected_uri);
-    /** @var \Drupal\Core\Image\Image $image */
-    $image = $this->container->get('image.factory')->get($expected_uri);
-    $this->assertTrue($image->isValid());
+    // Get the local thumbnail URI and ensure that it does not contain any
+    // query string.
+    $local_thumbnail_uri = $media_type->getSource()->getMetadata($media, 'thumbnail_uri');
+    $expected_uri = 'public://oembed_thumbnails/' . Crypt::hashBase64('/core/misc/druplicon.png') . '.png';
+    $this->assertSame($expected_uri, $local_thumbnail_uri);
   }
 
 }