diff --git a/core/modules/media/src/Plugin/media/Source/OEmbed.php b/core/modules/media/src/Plugin/media/Source/OEmbed.php index e406903df60..fd1d1ebaea7 100644 --- a/core/modules/media/src/Plugin/media/Source/OEmbed.php +++ b/core/modules/media/src/Plugin/media/Source/OEmbed.php @@ -26,8 +26,6 @@ use GuzzleHttp\ClientInterface; 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 @@ class OEmbed extends MediaSourceBase implements OEmbedInterface { if (!$remote_thumbnail_url) { return NULL; } + $remote_thumbnail_url = $remote_thumbnail_url->toString(); - // Ensure that we can write to the local directory where thumbnails are - // stored. + // 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. $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 @@ class OEmbed extends MediaSourceBase implements OEmbedInterface { 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 @@ class OEmbed extends MediaSourceBase implements OEmbedInterface { 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 0894cafcf97..76815ed1387 100644 --- a/core/modules/media/tests/fixtures/oembed/video_collegehumor.xml +++ b/core/modules/media/tests/fixtures/oembed/video_collegehumor.xml @@ -12,10 +12,7 @@ 343

By the power of Grayskull, CollegeHumor works!

- - internal:/media_test_oembed/thumbnail + internal:/core/misc/druplicon.png 88 100 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 dca946e6564..75ce685af71 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,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' 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 6ea9a1e5354..ab58e2f9629 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,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 @@ class ResourceController { 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. * diff --git a/core/modules/media/tests/src/FunctionalJavascript/MediaSourceOEmbedVideoTest.php b/core/modules/media/tests/src/FunctionalJavascript/MediaSourceOEmbedVideoTest.php index a1dbe6e8b15..a5c19ad0c7f 100644 --- a/core/modules/media/tests/src/FunctionalJavascript/MediaSourceOEmbedVideoTest.php +++ b/core/modules/media/tests/src/FunctionalJavascript/MediaSourceOEmbedVideoTest.php @@ -4,7 +4,6 @@ namespace Drupal\Tests\media\FunctionalJavascript; 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 @@ class MediaSourceOEmbedVideoTest extends MediaSourceTestBase { $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 314b2796e8b..923a9cc00f1 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\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\OEmbed\UrlResolverInterface; 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 @@ class OEmbedSourceTest extends MediaKernelTestBase { } /** - * 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('', 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 @@ class OEmbedSourceTest extends MediaKernelTestBase { $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 @@ class OEmbedSourceTest extends MediaKernelTestBase { ]); $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); } }