Problem/Motivation

When downloading the thumbnail for an oEmbed resource, the media system needs to figure out what the thumbnail's file extension will be, so that it (the thumbnail) can be used by the image system.

Most of the time, the file extension is right there in the thumbnail URL. But other times, the thumbnail URL has no file extension. In such a case, we make a HEAD request to the thumbnail URL and try to use the Content-Type header it gives us, if any, to determine a file extension. That, however, has two drawbacks:

  1. Downloading the thumbnail can result in two HTTP requests (one HEAD and one GET), rather than just one.
  2. Some providers, like TikTok, block HEAD requests. This results in the downloaded thumbnail having no file extension, and therefore being unusable by the image system.

Steps to reproduce

You'll need custom code, but create a media source plugin that extends the oEmbed source, and supports TikTok as a provider. You'll see that any media you add with that source will not get a proper thumbnail, since the thumbnail URLs don't have a file extension, and TikTok blocks HEAD requests. This was revealed by #3225184: oEmbed system cannot retrieve thumbnail extensions from TikTok / Vimeo.

(A test module was written for this; see #2.)

Proposed resolution

I think we can kill both of the aforementioned birds with one stone.

If we can't tease the thumbnail file extension out from the thumbnail URL, let's examine the Content-Type header in the GET response (that actually downloads the thumbnail) and derive a file extension that way. Not only will this be more likely to succeed, but it completely removes any need to make a HEAD request.

Remaining tasks

Implement it with test coverage.

User interface changes

None.

API changes

TBD, but an existing protected method may receive a new parameter and/or a deprecation notice.

Data model changes

None.

Release notes snippet

TBD

Issue fork drupal-3231731

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

phenaproxima created an issue. See original summary.

Bhanu951’s picture

Just to add If any one want to test the issue, Here is the Module Which extends oEmbed Class. I have created a sandbox project for easier testing.

Steps to test :

1.Enable the Module.
2.Create Media entity by navigating to Content > Media > Add Media > TikTok . You can see the Thumbnail is being Rendered.
3. Now attach the TikTok Media to any Node Bundle and try to embed the TikTok media entity (New or already created one). You will observe that Media item is attached and rendered, but it wont display thumbnail.

phenaproxima’s picture

Issue summary: View changes
Anybody’s picture

Same problem for vimeo as I posted in the other issue (#3225184):

Same problem for vimeo, which also doesn't return a file extension for thumbnail_url.

Example: "https://i.vimeocdn.com/video/1245577294-fa829fe2e1656b5f8c82dd2c81f2f806..."
So even if it works as designed, providers seem to do this... -.-

Perhaps we should close one of them as duplicate?

As result no Vimeo Thumbails are created (for public videos) while it works for YouTube.

phenaproxima’s picture

Status: Active » Needs review

Took a shot at implementing the solution outlined in the issue summary.

phenaproxima’s picture

Issue tags: +Performance

Since this change would reduce the need to make an extra HTTP request, it can be considered a performance improvement. Tagging accordingly.

larowlan’s picture

Looking nice, could only find a fault in the deprecation message

phenaproxima’s picture

Change record written. I've updated the code accordingly and, to be sure, grepped contrib for getThumbnailFileExtensionFromUrl, which turned up no results (see http://grep.xnddx.ru/search?text=getThumbnailFileExtensionFromUrl&filename=).

seanB’s picture

Status: Needs review » Reviewed & tested by the community

Thanks for the quick change @phenaproxima, I think this should cover it. Nice improvement!
RTBC assuming the test will be green.

Bhanu951’s picture

Hi @phenaproxima Thank you for Quick change. I tried to test this fix. I tested it on Drupal core 9.2.6 with patch applied. I still see the same issue that was present earlier for TikTok.

TikTok Video Thumbnails are still not being rendered similar to that what its present earlier. But it worked for Vimeo without any issues in my testing.

phenaproxima’s picture

I still see the same issue that was present earlier for TikTok.

Ah, sorry to hear it. However, since TikTok is not officially supported in core and this change is a performance improvement, I think we should commit it as-is. If you contact me in Slack, I'd happy to help figure out why it's failing for TikTok.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

This looks like a really nice improvement. Less requests is nice.

Committed 8597def and pushed to 9.3.x. Thanks!

  • alexpott committed 8597def on 9.3.x
    Issue #3231731 by phenaproxima, larowlan, seanB: Use Content-Type header...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.