Skip to content
Snippets Groups Projects
Commit 146249c8 authored by Adam G-H's avatar Adam G-H Committed by Lee Rowlands
Browse files

Issue #3080666 by phenaproxima, Upchuk, ieguskiza, ravi.shankar, nikitagupta,...

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
parent 477095b7
No related branches found
No related tags found
20 merge requests!12227Issue #3181946 by jonmcl, mglaman,!7452Issue #1797438. HTML5 validation is preventing form submit and not fully...,!1896Issue #2940605: Can only intentionally re-render an entity with references 20 times,!1101Issue #2412669 by claudiu.cristea, Julfabre, sidharrell, catch, daffie,...,!1039Issue #2556069 by claudiu.cristea, bnjmnm, lauriii, pfrenssen, Tim Bozeman,...,!10223132456: Fix issue where views instances are emptied before an ajax request is complete,!1012Issue #3226887: Hreflang on non-canonical content pages,!872Draft: Issue #3221319: Race condition when creating menu links and editing content deletes menu links,!868Issue #3080666: oEmbed system doesn't work if thumbnail url does not have a file extension,!594Put each entity type table into a details element on admin/config/regional/content-language,!592Issue #2957953: Editing menus user-experience has regressed,!579Issue #2230909: Simple decimals fail to pass validation,!560Move callback classRemove outside of the loop,!555Issue #3202493,!512Issue #3207771: Menu UI node type form documentation points to non-existent function,!485Sets the autocomplete attribute for username/password input field on login form.,!449Issue #2784233: Allow multiple vocabularies in the taxonomy filter,!231Issue #2671162: summary text wysiwyg patch working fine on 9.2.0-dev,!43Resolve #3173180: Add UI for 'loading' html attribute to images,!30Issue #3182188: Updates composer usage to point at ./vendor/bin/composer
...@@ -26,6 +26,8 @@ ...@@ -26,6 +26,8 @@
use GuzzleHttp\Exception\TransferException; use GuzzleHttp\Exception\TransferException;
use Psr\Log\LoggerInterface; use Psr\Log\LoggerInterface;
use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\DependencyInjection\ContainerInterface;
use Symfony\Component\Finder\Finder;
use Symfony\Component\Mime\MimeTypes;
/** /**
* Provides a media source plugin for oEmbed resources. * Provides a media source plugin for oEmbed resources.
...@@ -388,21 +390,11 @@ protected function getLocalThumbnailUri(Resource $resource) { ...@@ -388,21 +390,11 @@ protected function getLocalThumbnailUri(Resource $resource) {
if (!$remote_thumbnail_url) { if (!$remote_thumbnail_url) {
return NULL; 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(); $configuration = $this->getConfiguration();
$directory = $configuration['thumbnails_directory']; $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, // 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, // ensure that the destination directory is writable, and if it's not,
...@@ -414,8 +406,25 @@ protected function getLocalThumbnailUri(Resource $resource) { ...@@ -414,8 +406,25 @@ protected function getLocalThumbnailUri(Resource $resource) {
return NULL; 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 { try {
$response = $this->httpClient->get($remote_thumbnail_url); $response = $this->httpClient->request('GET', $remote_thumbnail_url);
if ($response->getStatusCode() === 200) { if ($response->getStatusCode() === 200) {
$this->fileSystem->saveData((string) $response->getBody(), $local_thumbnail_uri, FileSystemInterface::EXISTS_REPLACE); $this->fileSystem->saveData((string) $response->getBody(), $local_thumbnail_uri, FileSystemInterface::EXISTS_REPLACE);
return $local_thumbnail_uri; return $local_thumbnail_uri;
...@@ -432,6 +441,49 @@ protected function getLocalThumbnailUri(Resource $resource) { ...@@ -432,6 +441,49 @@ protected function getLocalThumbnailUri(Resource $resource) {
return NULL; 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} * {@inheritdoc}
*/ */
......
...@@ -12,7 +12,10 @@ ...@@ -12,7 +12,10 @@
<height>343</height> <height>343</height>
<html><h1>By the power of Grayskull, CollegeHumor works!</h1> <html><h1>By the power of Grayskull, CollegeHumor works!</h1>
</html> </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_width>88</thumbnail_width>
<thumbnail_height>100</thumbnail_height> <thumbnail_height>100</thumbnail_height>
</oembed> </oembed>
...@@ -4,3 +4,9 @@ media_test_oembed.resource.get: ...@@ -4,3 +4,9 @@ media_test_oembed.resource.get:
_controller: '\Drupal\media_test_oembed\Controller\ResourceController::get' _controller: '\Drupal\media_test_oembed\Controller\ResourceController::get'
requirements: requirements:
_access: 'TRUE' _access: 'TRUE'
media_test_oembed.resource.thumbnail:
path: '/media_test_oembed/thumbnail'
defaults:
_controller: '\Drupal\media_test_oembed\Controller\ResourceController::getThumbnailWithNoExtension'
requirements:
_access: 'TRUE'
...@@ -2,6 +2,7 @@ ...@@ -2,6 +2,7 @@
namespace Drupal\media_test_oembed\Controller; namespace Drupal\media_test_oembed\Controller;
use Symfony\Component\HttpFoundation\BinaryFileResponse;
use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpFoundation\Response;
...@@ -30,12 +31,24 @@ public function get(Request $request) { ...@@ -30,12 +31,24 @@ public function get(Request $request) {
else { else {
$content = file_get_contents($resources[$asset_url]); $content = file_get_contents($resources[$asset_url]);
$response = new Response($content); $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; 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. * Maps an asset URL to a local fixture representing its oEmbed resource.
* *
......
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
use Drupal\Core\Session\AccountInterface; use Drupal\Core\Session\AccountInterface;
use Drupal\media\Entity\Media; use Drupal\media\Entity\Media;
use Drupal\media\Entity\MediaType;
use Drupal\media_test_oembed\Controller\ResourceController; use Drupal\media_test_oembed\Controller\ResourceController;
use Drupal\Tests\media\Traits\OEmbedTestTrait; use Drupal\Tests\media\Traits\OEmbedTestTrait;
use Drupal\user\Entity\Role; use Drupal\user\Entity\Role;
...@@ -164,6 +165,29 @@ public function testMediaOEmbedVideoSource() { ...@@ -164,6 +165,29 @@ public function testMediaOEmbedVideoSource() {
$assert_session->pageTextContains('The CollegeHumor provider is not allowed.'); $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. // Test anonymous access to media via iframe.
$this->drupalLogout(); $this->drupalLogout();
......
...@@ -3,7 +3,7 @@ ...@@ -3,7 +3,7 @@
namespace Drupal\Tests\media\Kernel; namespace Drupal\Tests\media\Kernel;
use Drupal\Component\Utility\Crypt; use Drupal\Component\Utility\Crypt;
use Drupal\Core\Url; use Drupal\Core\Logger\RfcLogLevel;
use Drupal\media\Entity\Media; use Drupal\media\Entity\Media;
use Drupal\media\OEmbed\Resource; use Drupal\media\OEmbed\Resource;
use Drupal\media\OEmbed\ResourceFetcherInterface; use Drupal\media\OEmbed\ResourceFetcherInterface;
...@@ -11,6 +11,7 @@ ...@@ -11,6 +11,7 @@
use Drupal\media\Plugin\media\Source\OEmbed; use Drupal\media\Plugin\media\Source\OEmbed;
use GuzzleHttp\Client; use GuzzleHttp\Client;
use GuzzleHttp\Psr7\Response; use GuzzleHttp\Psr7\Response;
use GuzzleHttp\Psr7\Utils;
use Prophecy\Argument; use Prophecy\Argument;
/** /**
...@@ -43,9 +44,62 @@ public function testGetMetadata() { ...@@ -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 * @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 // There's no need to resolve the resource URL in this test; we just need
// to fetch the resource. // to fetch the resource.
$this->container->set( $this->container->set(
...@@ -53,23 +107,54 @@ public function testLocalThumbnailUriQueryStringIsIgnored() { ...@@ -53,23 +107,54 @@ public function testLocalThumbnailUriQueryStringIsIgnored() {
$this->prophesize(UrlResolverInterface::class)->reveal() $this->prophesize(UrlResolverInterface::class)->reveal()
); );
$thumbnail_url = Url::fromUri('internal:/core/misc/druplicon.png?foo=bar'); // Mock the resource fetcher so that it will return our fake resource.
$resource_fetcher = $this->prophesize(ResourceFetcherInterface::class);
// Create a mocked resource whose thumbnail URL contains a query string. $resource_fetcher->fetchResource(Argument::any())
$resource = $this->prophesize(Resource::class); ->willReturn($resource);
$resource->getTitle()->willReturn('Test resource'); $this->container->set('media.oembed.resource_fetcher', $resource_fetcher->reveal());
$resource->getThumbnailUrl()->willReturn($thumbnail_url);
// The source plugin will try to fetch the remote thumbnail, so mock the // 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 = $this->prophesize(Client::class);
$http_client->get(Argument::type('string'))->willReturn(new Response()); // The thumbnail should only be downloaded once.
$this->container->set('http_client', $http_client->reveal()); $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. // If the file extension cannot be derived from the URL, a single HEAD
$resource_fetcher = $this->prophesize(ResourceFetcherInterface::class); // request should be made to try and determine its type from the
$resource_fetcher->fetchResource(NULL)->willReturn($resource->reveal()); // Content-Type HTTP header.
$this->container->set('media.oembed.resource_fetcher', $resource_fetcher->reveal()); 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'); $media_type = $this->createMediaType('oembed:video');
$source = $media_type->getSource(); $source = $media_type->getSource();
...@@ -80,11 +165,18 @@ public function testLocalThumbnailUriQueryStringIsIgnored() { ...@@ -80,11 +165,18 @@ public function testLocalThumbnailUriQueryStringIsIgnored() {
]); ]);
$media->save(); $media->save();
// Get the local thumbnail URI and ensure that it does not contain any // The thumbnail should have a file extension, even if it wasn't in the URL.
// query string. $expected_uri = 'public://oembed_thumbnails/' . Crypt::hashBase64($thumbnail_url) . ".$expected_extension";
$local_thumbnail_uri = $media_type->getSource()->getMetadata($media, 'thumbnail_uri'); $this->assertSame($expected_uri, $source->getMetadata($media, 'thumbnail_uri'));
$expected_uri = 'public://oembed_thumbnails/' . Crypt::hashBase64('/core/misc/druplicon.png') . '.png'; // Even if we get the thumbnail_uri more than once, it should only be
$this->assertSame($expected_uri, $local_thumbnail_uri); // 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());
} }
} }
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment