diff --git a/core/modules/media/src/Plugin/media/Source/OEmbed.php b/core/modules/media/src/Plugin/media/Source/OEmbed.php
index fd1d1ebaea727256f40223f3d0a331628b1a91ab..e406903df609673671f89a0e99fefc40f548144c 100644
--- a/core/modules/media/src/Plugin/media/Source/OEmbed.php
+++ b/core/modules/media/src/Plugin/media/Source/OEmbed.php
@@ -26,6 +26,8 @@
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.
@@ -388,21 +390,11 @@ 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);
- // Compute the local thumbnail URI, regardless of whether or not it exists.
+ // Ensure that we can write to the local directory where thumbnails are
+ // stored.
$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,
@@ -414,8 +406,25 @@ 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->get($remote_thumbnail_url);
+ $response = $this->httpClient->request('GET', $remote_thumbnail_url);
if ($response->getStatusCode() === 200) {
$this->fileSystem->saveData((string) $response->getBody(), $local_thumbnail_uri, FileSystemInterface::EXISTS_REPLACE);
return $local_thumbnail_uri;
@@ -432,6 +441,49 @@ 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}
*/
diff --git a/core/modules/media/tests/fixtures/oembed/video_collegehumor.xml b/core/modules/media/tests/fixtures/oembed/video_collegehumor.xml
index 76815ed1387de471cebe627dc2a47076e9c845f4..0894cafcf97e34f6789f0a6a6f63ddd426146bed 100644
--- a/core/modules/media/tests/fixtures/oembed/video_collegehumor.xml
+++ b/core/modules/media/tests/fixtures/oembed/video_collegehumor.xml
@@ -12,7 +12,10 @@
<height>343</height>
<html><h1>By the power of Grayskull, CollegeHumor works!</h1>
</html>
- <thumbnail_url>internal:/core/misc/druplicon.png</thumbnail_url>
+ <!-- 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_width>88</thumbnail_width>
<thumbnail_height>100</thumbnail_height>
</oembed>
diff --git a/core/modules/media/tests/modules/media_test_oembed/media_test_oembed.routing.yml b/core/modules/media/tests/modules/media_test_oembed/media_test_oembed.routing.yml
index 75ce685af710c04f88feb378ab3f216afd0af555..dca946e6564d7c1a8937a717e7db72257c220f8f 100644
--- a/core/modules/media/tests/modules/media_test_oembed/media_test_oembed.routing.yml
+++ b/core/modules/media/tests/modules/media_test_oembed/media_test_oembed.routing.yml
@@ -4,3 +4,9 @@ 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'
diff --git a/core/modules/media/tests/modules/media_test_oembed/src/Controller/ResourceController.php b/core/modules/media/tests/modules/media_test_oembed/src/Controller/ResourceController.php
index ab58e2f9629c503312ed77df38a24f356de8c824..6ea9a1e5354b5cfe7c33125d06fbe8189f4c3481 100644
--- a/core/modules/media/tests/modules/media_test_oembed/src/Controller/ResourceController.php
+++ b/core/modules/media/tests/modules/media_test_oembed/src/Controller/ResourceController.php
@@ -2,6 +2,7 @@
namespace Drupal\media_test_oembed\Controller;
+use Symfony\Component\HttpFoundation\BinaryFileResponse;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
@@ -30,12 +31,24 @@ public function get(Request $request) {
else {
$content = file_get_contents($resources[$asset_url]);
$response = new Response($content);
- $response->headers->set('Content-Type', 'application/json');
+ $response->headers->set('Content-Type', 'application/' . pathinfo($resources[$asset_url], PATHINFO_EXTENSION));
}
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.
*
diff --git a/core/modules/media/tests/src/FunctionalJavascript/MediaSourceOEmbedVideoTest.php b/core/modules/media/tests/src/FunctionalJavascript/MediaSourceOEmbedVideoTest.php
index a5c19ad0c7f67ed76f86a5aa5f3b2c8904a47ccc..a1dbe6e8b153b7db9768fd3a3cddbb75712ece2e 100644
--- a/core/modules/media/tests/src/FunctionalJavascript/MediaSourceOEmbedVideoTest.php
+++ b/core/modules/media/tests/src/FunctionalJavascript/MediaSourceOEmbedVideoTest.php
@@ -4,6 +4,7 @@
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;
@@ -164,6 +165,29 @@ 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();
diff --git a/core/modules/media/tests/src/Kernel/OEmbedSourceTest.php b/core/modules/media/tests/src/Kernel/OEmbedSourceTest.php
index 923a9cc00f1c88a3f1a613837d53c70465125597..314b2796e8b6157f397324264d1d3fa9fc0a7bad 100644
--- a/core/modules/media/tests/src/Kernel/OEmbedSourceTest.php
+++ b/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\Url;
+use Drupal\Core\Logger\RfcLogLevel;
use Drupal\media\Entity\Media;
use Drupal\media\OEmbed\Resource;
use Drupal\media\OEmbed\ResourceFetcherInterface;
@@ -11,6 +11,7 @@
use Drupal\media\Plugin\media\Source\OEmbed;
use GuzzleHttp\Client;
use GuzzleHttp\Psr7\Response;
+use GuzzleHttp\Psr7\Utils;
use Prophecy\Argument;
/**
@@ -43,9 +44,62 @@ 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 testLocalThumbnailUriQueryStringIsIgnored() {
+ 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();
+
// There's no need to resolve the resource URL in this test; we just need
// to fetch the resource.
$this->container->set(
@@ -53,23 +107,54 @@ public function testLocalThumbnailUriQueryStringIsIgnored() {
$this->prophesize(UrlResolverInterface::class)->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);
+ // 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());
// The source plugin will try to fetch the remote thumbnail, so mock the
- // HTTP client to ensure that request returns an empty "OK" response.
+ // 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 = $this->prophesize(Client::class);
- $http_client->get(Argument::type('string'))->willReturn(new Response());
- $this->container->set('http_client', $http_client->reveal());
+ // 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';
- // 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());
+ // 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();
+ }
+ $this->container->set('http_client', $http_client->reveal());
$media_type = $this->createMediaType('oembed:video');
$source = $media_type->getSource();
@@ -80,11 +165,18 @@ public function testLocalThumbnailUriQueryStringIsIgnored() {
]);
$media->save();
- // 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);
+ // 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());
}
}