diff --git a/core/modules/media/src/Plugin/media/Source/OEmbed.php b/core/modules/media/src/Plugin/media/Source/OEmbed.php index e7417c0018..d7a61984b0 100644 --- a/core/modules/media/src/Plugin/media/Source/OEmbed.php +++ b/core/modules/media/src/Plugin/media/Source/OEmbed.php @@ -12,6 +12,7 @@ use Drupal\Core\File\Exception\FileException; use Drupal\Core\File\FileSystemInterface; use Drupal\Core\Form\FormStateInterface; +use Drupal\Core\Image\ImageFactory; use Drupal\Core\Messenger\MessengerInterface; use Drupal\Core\Url; use Drupal\media\IFrameUrlHelper; @@ -26,6 +27,7 @@ use GuzzleHttp\Exception\TransferException; use Psr\Log\LoggerInterface; use Symfony\Component\DependencyInjection\ContainerInterface; +use Symfony\Component\Mime\MimeTypes; /** * Provides a media source plugin for oEmbed resources. @@ -123,6 +125,13 @@ class OEmbed extends MediaSourceBase implements OEmbedInterface { */ protected $fileSystem; + /** + * The image factory. + * + * @var \Drupal\Core\Image\ImageFactory + */ + protected $imageFactory; + /** * Constructs a new OEmbed instance. * @@ -154,8 +163,10 @@ class OEmbed extends MediaSourceBase implements OEmbedInterface { * The iFrame URL helper service. * @param \Drupal\Core\File\FileSystemInterface $file_system * The file system. + * @param \Drupal\Core\Image\ImageFactory $image_factory + * The image factory. */ - public function __construct(array $configuration, $plugin_id, $plugin_definition, EntityTypeManagerInterface $entity_type_manager, EntityFieldManagerInterface $entity_field_manager, ConfigFactoryInterface $config_factory, FieldTypePluginManagerInterface $field_type_manager, LoggerInterface $logger, MessengerInterface $messenger, ClientInterface $http_client, ResourceFetcherInterface $resource_fetcher, UrlResolverInterface $url_resolver, IFrameUrlHelper $iframe_url_helper, FileSystemInterface $file_system) { + public function __construct(array $configuration, $plugin_id, $plugin_definition, EntityTypeManagerInterface $entity_type_manager, EntityFieldManagerInterface $entity_field_manager, ConfigFactoryInterface $config_factory, FieldTypePluginManagerInterface $field_type_manager, LoggerInterface $logger, MessengerInterface $messenger, ClientInterface $http_client, ResourceFetcherInterface $resource_fetcher, UrlResolverInterface $url_resolver, IFrameUrlHelper $iframe_url_helper, FileSystemInterface $file_system, ImageFactory $image_factory = NULL) { parent::__construct($configuration, $plugin_id, $plugin_definition, $entity_type_manager, $entity_field_manager, $field_type_manager, $config_factory); $this->logger = $logger; $this->messenger = $messenger; @@ -164,6 +175,7 @@ public function __construct(array $configuration, $plugin_id, $plugin_definition $this->urlResolver = $url_resolver; $this->iFrameUrlHelper = $iframe_url_helper; $this->fileSystem = $file_system; + $this->imageFactory = $image_factory ?: \Drupal::service('image.factory'); } /** @@ -184,7 +196,8 @@ public static function create(ContainerInterface $container, array $configuratio $container->get('media.oembed.resource_fetcher'), $container->get('media.oembed.url_resolver'), $container->get('media.oembed.iframe_url_helper'), - $container->get('file_system') + $container->get('file_system'), + $container->get('image.factory') ); } @@ -390,14 +403,10 @@ protected function getLocalThumbnailUri(Resource $resource) { } $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. $configuration = $this->getConfiguration(); $directory = $configuration['thumbnails_directory']; - $local_thumbnail_uri = "$directory/" . Crypt::hashBase64($local_thumbnail_url) . '.' . pathinfo($local_thumbnail_url, PATHINFO_EXTENSION); + $local_thumbnail_uri = "$directory/" . Crypt::hashBase64($remote_thumbnail_url) . '.' . $this->getThumbnailFileExtensionFromUrl($remote_thumbnail_url); // If the local thumbnail already exists, return its URI. if (file_exists($local_thumbnail_uri)) { @@ -415,7 +424,7 @@ protected function getLocalThumbnailUri(Resource $resource) { } 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,44 @@ protected function getLocalThumbnailUri(Resource $resource) { return NULL; } + /** + * Tries to determine the file extension of a thumbnail, based on its URL. + * + * @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 by seeing if the URL path ends with one + // of the file extensions supported by the current image toolkit. + $path = parse_url($thumbnail_url, PHP_URL_PATH); + if ($path) { + $extension = strtolower(pathinfo($path, PATHINFO_EXTENSION)); + $supported_extensions = array_map('strtolower', $this->imageFactory->getSupportedExtensions()); + + if (in_array($extension, $supported_extensions, TRUE)) { + 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. + $response = $this->httpClient->request('HEAD', $thumbnail_url); + if ($response->hasHeader('Content-Type')) { + $content_type = $response->getHeader('Content-Type'); + $extensions = MimeTypes::getDefault()->getExtensions(reset($content_type)); + + if ($extensions) { + return reset($extensions); + } + } + + 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 76815ed138..0894cafcf9 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 @@ 343

By the power of Grayskull, CollegeHumor works!

- internal:/core/misc/druplicon.png + + internal:/media_test_oembed/thumbnail 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 75ce685af7..dca946e656 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 ab58e2f962..6ea9a1e535 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 a5c19ad0c7..a1dbe6e8b1 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 923a9cc00f..58a65a42bd 100644 --- a/core/modules/media/tests/src/Kernel/OEmbedSourceTest.php +++ b/core/modules/media/tests/src/Kernel/OEmbedSourceTest.php @@ -63,9 +63,11 @@ public function testLocalThumbnailUriQueryStringIsIgnored() { // 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 = $this->prophesize(Client::class); - $http_client->get(Argument::type('string'))->willReturn(new Response()); + $http_client->request('GET', Argument::type('string'))->willReturn(new Response()); $this->container->set('http_client', $http_client->reveal()); + $http_client->get($thumbnail_url->toString())->willReturn(new Response()); + // 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()); @@ -83,8 +85,103 @@ public function testLocalThumbnailUriQueryStringIsIgnored() { // 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'; + $expected_uri = 'public://oembed_thumbnails/' . Crypt::hashBase64($thumbnail_url->toString()) . '.png'; $this->assertSame($expected_uri, $local_thumbnail_uri); } + /** + * Data provider for ::testGetThumbnailUri(). + * + * @return array + * The sets of arguments to pass to the test method. + */ + public function providerGetThumbnailUri() { + return [ + 'with file extension' => [ + 'https://upload.wikimedia.org/wikipedia/commons/9/90/Dries_Buytaert_at_FOSDEM_2008_by_Wikinews.jpg', + ], + 'no file extension' => [ + 'http://placekitten.com/200/300', + ], + ]; + } + + /** + * Tests remote thumbnail URL handling. + * + * @param string $thumbnail_url + * The remote URL of the thumbnail. + * + * @dataProvider providerGetThumbnailUri + */ + public function testGetThumbnailUri($thumbnail_url) { + // We will need a media type that uses the oEmbed plugin. + $media = Media::create([ + 'bundle' => $this->createMediaType('oembed:video')->id(), + 'field_media_oembed_video' => 'test', + ]); + + // Mock the services which are used to fetch the resource and thumbnail. + $url_resolver = $this->prophesize('\Drupal\media\OEmbed\UrlResolverInterface'); + $resource_fetcher = $this->prophesize('\Drupal\media\OEmbed\ResourceFetcherInterface'); + $http_client = $this->prophesize('\GuzzleHttp\ClientInterface'); + + $resource = $this->prophesize('\Drupal\media\OEmbed\Resource'); + $resource->getThumbnailUrl()->willReturn(Url::fromUri($thumbnail_url)); + + $url_resolver->getResourceUrl('test')->willReturnArgument(0); + $resource_fetcher->fetchResource('test')->willReturn($resource->reveal()); + + // The HTTP client should make a HEAD request ONLY if the thumbnail URL + // doesn't have a file extension. + if (pathinfo($thumbnail_url, PATHINFO_EXTENSION)) { + $http_client->request('HEAD', $thumbnail_url)->shouldNotBeCalled(); + } + else { + $response = new Response(200, [ + 'Content-Type' => [ + 'image/jpeg', + ], + ]); + $http_client->request('HEAD', $thumbnail_url) + // A HEAD request should be made every time the thumbnail is requested + // when it lacks a file extension. + ->shouldBeCalledTimes(2) + ->willReturn($response); + } + // We always expect the actual thumbnail to be retrieved but a GET + // request should only be made ONCE when first downloading the file. + $http_client->request('GET', $thumbnail_url) + ->shouldBeCalledOnce() + ->willReturn(new Response()); + + $plugin = new OEmbed( + ['source_field' => 'field_media_oembed_video'], + 'oembed', + [], + $this->container->get('entity_type.manager'), + $this->container->get('entity_field.manager'), + $this->container->get('config.factory'), + $this->container->get('plugin.manager.field.field_type'), + $this->container->get('logger.factory')->get('media'), + $this->container->get('messenger'), + $http_client->reveal(), + $resource_fetcher->reveal(), + $url_resolver->reveal(), + $this->container->get('media.oembed.iframe_url_helper'), + $this->container->get('file_system') + ); + + // Get the local thumbnail URI twice, to ensure that it is only downloaded + // (i.e., with a GET request) once. If the thumbnail URL is remote, we + // expect a HEAD request to be made each time by + // getRemoteThumbnailPathExtension(). + for ($i = 0; $i < 2; $i++) { + $local_thumbnail_uri = $plugin->getMetadata($media, 'thumbnail_uri'); + $this->assertNotEmpty($local_thumbnail_uri); + $this->assertFileExists($local_thumbnail_uri); + $this->assertMatchesRegularExpression('/.jpe?g$/i', $local_thumbnail_uri); + } + } + }