Problem/Motivation

I am trying to use a YouTube playlist in a Remote Video Media entity. Every time I use a url like https://www.youtube.com/playlist?list=PLYqVYBTIm7u0HqkxEuemM9XqP1HN-PqMe it spits out the error: The given URL does not match any known oEmbed providers.. The scheme is in the YouTube endpoint, but the non escapped question mark is returning a false negative on the RegEx inspection.

Steps to reproduce

Use any YouTube playlist url as a Remote Video Media Entity.

Proposed resolution

Patch to follow with fix.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3222616

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

labboy0276 created an issue. See original summary.

labboy0276’s picture

Status: Active » Needs review
FileSize
653 bytes

Patch to address the issue.

hmendes’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
40.65 KB
98.14 KB

Tested patch from #2 and it fixed my problem.
I added a media field as Remote Video and tried to create a new node adding the url https://www.youtube.com/playlist?list=PLYqVYBTIm7u0HqkxEuemM9XqP1HN-PqMe on the media field and it showed the error on 3222616_before_patch.png, after applying the patch it worked and I could create the node.
Changing to RTBC.

cilefen’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs tests

There are a few potential places to add test coverage:

$ git ls-files|grep OEmbed|grep -i test
core/modules/media/tests/src/Functional/FieldFormatter/OEmbedFormatterTest.php
core/modules/media/tests/src/FunctionalJavascript/MediaSourceOEmbedVideoTest.php
core/modules/media/tests/src/Kernel/OEmbedIframeControllerTest.php
core/modules/media/tests/src/Kernel/OEmbedResourceConstraintValidatorTest.php
core/modules/media/tests/src/Kernel/OEmbedSourceTest.php
core/modules/media/tests/src/Traits/OEmbedTestTrait.php
core/modules/media_library/tests/src/FunctionalJavascript/WidgetOEmbedTest.php
xjm’s picture

Version: 9.1.x-dev » 9.2.x-dev

9.1.x is also security fixes only at this point.

Thanks for the bug report and proposed fix!

larowlan’s picture

Any reason this doesn't use preq_quote?

phenaproxima’s picture

Issue tags: -media oembed

Removing redundant tag.

vsujeetkumar’s picture

Status: Needs work » Needs review
FileSize
2.72 KB
1.91 KB

Test added, Please have a look.

phenaproxima’s picture

Status: Needs review » Needs work
Issue tags: -Needs tests

Thanks for writing that test, @vsujeetkumar!!

There is one issue with it, though: it looks like it's making an actual request for a real YouTube playlist. That's troublesome because, if YouTube went down (or that playlist disappeared), our tests would start failing. It's therefore imperative that core tests do NOT make real requests to the Internet.

I can see two ways to correct this:

  1. You could use OEmbedTestTrait and media_test_oembed, plus a new fixture representing a YouTube playlist, to make the test run offline. This way is a bit tricky to implement, but an example of it can be seen in Drupal\Tests\media\Functional\FieldFormatter::testRender().
  2. We could change this to a unit (or, if needed, kernel) test. Although a full functional test is handy, it might also be overkill in this case, given the nature of the fix. (Functional tests also take much longer to run than unit or kernel tests, and with over 28,000 tests in core, it adds up.) I think it would be acceptable to create a new unit test that just covers the Endpoint::matchUrl() method, and tests to ensure it handles the question mark correctly.

Sending back to "needs work" to, at the very least, adjust the test, which IMHO is the only thing that blocks commit here. My preference would be to convert the test to a unit test, but you can keep it as a functional test if that's what you're more comfortable with. The main requirement is that the test must be able to run, and pass, offline.

phenaproxima’s picture

On second thought, let's go with a unit test here. I think that it will be much simpler to write, it'll run orders of magnitude faster than a functional test, and it's guaranteed to pass offline. :) All the test needs to do, to be clear, is:

  • Create a fake provider ($this->createMock() is good for this)
  • Create an Endpoint object with a scheme that contains a ? character
  • Assert that a URL with a ? in it can match that endpoint

Ideally we should use some of the YouTube endpoint info from https://oembed.com/providers.json to create the fake endpoint, for added realism.

phenaproxima’s picture

Version: 9.2.x-dev » 9.3.x-dev
phenaproxima’s picture

Status: Needs work » Needs review
FileSize
878 bytes
1.5 KB

Wrote the unit test. Here's a pass/fail patch for proof, and I've committed the changes to the merge request.

The last submitted patch, 12: 3222616-12-FAIL.patch, failed testing. View results

marcoscano’s picture

Status: Needs review » Reviewed & tested by the community

Reviewed the patch and the test, and both look good to me. Thanks!

  • catch committed 5a3a0ab on 9.3.x
    Issue #3222616 by phenaproxima, vsujeetkumar, labboy0276, hmendes,...

  • catch committed c23e67c on 9.2.x
    Issue #3222616 by phenaproxima, vsujeetkumar, labboy0276, hmendes,...
catch’s picture

Version: 9.3.x-dev » 9.2.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 9.3.x and cherry-picked to 9.2.x, thanks!

Status: Fixed » Closed (fixed)

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

TomiMikola’s picture

Hi! There seems to still be a minor issue accepting the playlist URLs.

When copying the URL from the browser address bar the format is accepted since it contains "www." but when copied from the Share-button modal the URL is without the "www." and will throw "The given URL does not match any known oEmbed providers" error.

How to reproduce?

  1. Have Remote video enabled for media types
  2. Go to /media/add/remote_video
  3. Fill the Video URL field with value https://youtube.com/playlist?list=PLpeDXSh4nHjRbK4e6xsJ5-EFkDLOAPYfc
  4. The media item is not saved but "The given URL does not match any known oEmbed providers" error is shown
TomiMikola’s picture

Regarding the comment #20 I made a PR to the oembed provider list to handle the non-www playlist URLs: https://github.com/iamcal/oembed/pull/598

Nothing to do here in Drupal. Only to wait the provider list hopefully updates.

Eduardo Morales Alberti’s picture

When we try to embed using the URL "https://youtube.com/playlist?list=PLpeDXSh4nHjRbK4e6xsJ5-EFkDLOAPYfc" we get the error:
"Refused to frame 'http://mysite.docker.localhost:8000/' because an ancestor violates the following Content Security Policy directive: "frame-ancestors 'none'"."

But if we change to URL "https://www.youtube.com/embed/videoseries?list=PLpeDXSh4nHjRbK4e6xsJ5-EF..." then the video list is shown.

Any clue?