#23149 (YouTube Embedding is incorrect for https:// URLs) – WordPress Trac

@Otto42
12 years ago

Attachment 23149.diff added

#1 @nacin
12 years ago

Type changed from defect (bug) to enhancement

Incorrect? Eye of the beholder. A bug? No, this looks like an enhancement.

The oEmbed spec is oblivious when it comes to https content.

Mixed content would still occur when someone uses an https link but doesn't actually want or expect https content. This, I imagine, is far more common than someone with frontend https who does want https content.

Core does not currently have any frontend https logic baked in.

#2 @Otto42
12 years ago

I disagree. One would expect an obvious behavior, the code does not have that expected behavior. Therefore, it's a bug.

I agree that the spec sucks. YouTube has specifically created a scheme parameter workaround because of this. There is no reason to not to use it.

It is perfectly acceptable to put https content into an http page, and this is not typically considered to be "mixed content". The reverse of putting http into https pages, however, generates mixed content warnings, and that is indeed a problem. So basically, you're saying here that we should not support the case which is an actual problem because the totally-not-a-real-problem case is more common?

The core does not need any https front end logic baked in for it to do the right thing for oembed when it is explicitly given an https URL.

#3 @TobiasBg
12 years ago

I'm with Otto here.

This fix does prevent annoying mixed content warnings on https pages, that basically make the YouTube oEmbed unusable on https sites at this time.

There's no problem to embed a video as https on an http page, so this doesn't do harm. No one will notice. Someone who enters a https URL but then doesn't want or expect https to be embedded is doing it wrong.

I don't care, if it's labeled bug or enhancement, but I'd also like to see this fixed.

#4 @Asif2BD
12 years ago

I also need https return if I am using https link. I have posted about this in a support thread.

Either fix it in core, or give us a function or switch, so we could force WordPress to return with https, maybe via condition in function.php

#5 @JayCC
12 years ago

FWIW, we've tested this patch in our development environment with much success, and will be deploying it to production soon. +1 to have it added to core (don't care much if it's classified as a bug or feature).

#6 @JayCC
12 years ago

Cc JayCC added

#7 @rmccue
12 years ago

+1 on this in core, and +1 on it being a bug. It doesn't matter much whether it's extra on top of the oEmbed protocol, since it causes broken behaviour on the frontend regardless. Given how trivial the patch is, it's irrelevant anyway.

@JayCC
11 years ago

Attachment 23149a.diff added

modified patch to work on youtu.be links

#8 @JayCC
11 years ago

We've been using this patch in production for a couple of months now, without issue, however we had some users add youtube videos using the secure version of the shortened URL (https://youtu.be/xxxxxxx) which this patch didn't catch (so the videos were served to visitors on non-secure, causing mixed content warnings). I attached a modified version of the patch that addresses this.

@JayCC
11 years ago

Attachment 23149b.diff added

Updating patch to work with wordpress 3.6

#9 @markus.magnuson
11 years ago

Cc markus.magnuson@… added

@JayCC
11 years ago

Attachment 23149c.diff added

Updated patch for Wordpress 3.7

#10 @martyn_
11 years ago

Also hitting this on my https only site.

I disagree with this

The core does not need any https front end logic baked in for it to do the right thing for oembed when it is explicitly given an https URL.

though. If the site is https, then even if the user enters a http link, as is extremely common because they've just copy pasta from another window without any second thought, then it should be converted to https. Why not just have a protocol-relative URL ( //youtube.com/ ) generated, which will then follow the current protocol of the site?

Last edited 11 years ago by SergeyBiryukov (previous) (diff)

#11 @Otto42
11 years ago

YouTube's oEmbed doesn't do protocol relative URLs. You have to explicitly tell it the scheme to use, with scheme=https for it to use https links in the returned HTML from their embed endpoint.

#12 @dbarlett
11 years ago

Cc dylan.barlett@… added

#13 @rachelbaker
11 years ago

Cc rachel@… added

@adamsilverstein
11 years ago

Attachment 23149.2.diff added

refresh

#14 @adamsilverstein
11 years ago

23149.2.diff : Refresh against trunk

#15 @adamsilverstein
11 years ago

Keywords has-patch dev-feedback added

#16 follow-up: ↓ 18 @nacin
11 years ago

Milestone changed from Awaiting Review to 3.9

Okay, let's do it. Maybe we should always do scheme=https, regardless of video.

@Otto42
11 years ago

Attachment 23149.3.diff added

Patch to always use scheme https, regardless of URL

#17 follow-up: ↓ 21 @Otto42
11 years ago

23149.3.diff is a patch to always use the https scheme, regardless of the URL given.

Whether this is desirable or not is up for debate.

Version 0, edited 11 years ago by Otto42 (next)

#18 in reply to: ↑ 16 @adamsilverstein
11 years ago

Replying to nacin:

Okay, let's do it. Maybe we should always do scheme=https, regardless of video.

Yea, lets do it - keep running into this issue over and over again! I'm not sure about the scheme, or whether https for non https pages would be detrimental in any way.

I would appreciate your tips re: unit testing. I tried to write a unit test for this issue and failed. I tried do_shortcode and also global $wp_embed; = $wp_embed->run_shortcode( '[embed]http://youtube.com/watch?v=AbcDeFg123[/embed]' ); all return null or empty strings.

Any tips on getting embeds to fire in tests? I scoured existing tests for an example without luck. Point me in the right direction and I will whip up a test... thanks

Thanks!

#19 follow-up: ↓ 20 @Otto42
11 years ago

@adamsilverstein: The shortcode function won't work for testing here because you're not in a normal Loop and so there is no global $post context setup. The shortcode expects there to be a $post because it uses the post_meta to do caching of the oembed result. This is to prevent it from retrieving the info from the remote URL every time. Additionally, you have to use a real working YouTube URL for it to actually get anything back, a fake URL won't have valid response data.

echo wp_oembed_get( 'http://www.youtube.com/watch?v=oHg5SJYRHA0' );

returns:

<iframe width="500" height="375" src="http://www.youtube.com/embed/oHg5SJYRHA0?feature=oembed" frameborder="0" allowfullscreen></iframe>

Last edited 11 years ago by Otto42 (previous) (diff)

#20 in reply to: ↑ 19 @adamsilverstein
11 years ago

Replying to Otto42:

@adamsilverstein: The shortcode function won't work for testing here because you're not in a normal Loop and so there is no global $post context setup. The shortcode expects there to be a $post because it uses the post_meta to do caching of the oembed result. This is to prevent it from retrieving the info from the remote URL every time. Additionally, you have to use a real working YouTube URL for it to actually get anything back, a fake URL won't have valid response data.

echo wp_oembed_get( 'http://www.youtube.com/watch?v=oHg5SJYRHA0' );

returns:

<iframe width="500" height="375" src="http://www.youtube.com/embed/oHg5SJYRHA0?feature=oembed" frameborder="0" allowfullscreen></iframe>

Great! Thank you for the detailed response. I think I tried wp_oembed_get along the way, maybe missed the piece about requiring a valid URL. I will give it another shot...

#21 in reply to: ↑ 17 ; follow-up: ↓ 28 @adamsilverstein
11 years ago

Replying to Otto42:

23149.3.diff is a patch to always use the https scheme, regardless of the URL given.

Whether this is desirable or not is up for debate.

The arguments I would have against going all https are: 1. some networks may block https and that would mean users wouldn't see videos and 2. its unexpected behavior - do we do that anywhere else? (serve up https when http has been embedded)? and 3. it's a waste of resources (serving over https vs. http)

@adamsilverstein
11 years ago

Attachment 23149-test.diff added

unit test for #23149

#22 @adamsilverstein
11 years ago

23149-test.diff: a simple unit test verifying that youtube embeds using https (either youtube.com or youtu.be urls) result in an https embedded element. Passes after applying 23149.2.diff.

#23 follow-up: ↓ 24 @Otto42
11 years ago

You can't put in the width and height and such, because the return values there will depend on what theme you're running. Check for the url in the iframe only.

@adamsilverstein
11 years ago

Attachment 23149-test.2.diff added

just look for expected https url somewhere in assertion

#24 in reply to: ↑ 23 @adamsilverstein
11 years ago

Replying to Otto42:

You can't put in the width and height and such, because the return values there will depend on what theme you're running. Check for the url in the iframe only.

Doh! Fixed in 23149-test.2.diff, hope that looks better :)

This ticket was mentioned in IRC in #wordpress-dev by adamsilverstein. View the logs.

11 years ago

#27 @nacin
11 years ago

Owner set to nacin
Resolution set to fixed
Status changed from new to closed

In 26978:

Ensure that SSL YouTube URLs receive SSL embeds.

props adamsilverstein, Otto42, JayCC.
fixes #23149.

#28 in reply to: ↑ 21 @westonruter
11 years ago

Replying to adamsilverstein:

Replying to Otto42:

23149.3.diff is a patch to always use the https scheme, regardless of the URL given.

Whether this is desirable or not is up for debate.

The arguments I would have against going all https are: 1. some networks may block https and that would mean users wouldn't see videos and 2. its unexpected behavior - do we do that anywhere else? (serve up https when http has been embedded)? and 3. it's a waste of resources (serving over https vs. http)

I just ran into a problem here with a site that is served entirely over HTTPS. If a user uses the URL that YouTube provides, this URL uses HTTP. If you then embed this URL into a site that is served over HTTPS, then it will fail to embed (at least in Chrome) and give the error:

[blocked] The page at 'https://example.com/' was loaded over HTTPS, but ran insecure content from 'http://www.youtube.com/embed/DYu_bGbZiiQ?feature=oembed': this content should also be loaded over HTTPS.

So if we can't always default to using HTTPS, then what about forcing all URLs to be HTTPS if it is determined that the siteurl is HTTPS? Otherwise, I suppose a content filter can be added which checks if is_ssl and replaces all HTTP oEmbeds with HTTPS ones. This would address the situation where some pages of the site are served over HTTP whereas others over HTTPS.

Should this be re-opened or should a new ticket be made?

#29 @Otto42
11 years ago

That would be a new ticket, however I think Nacin's original reply holds here for that:

Core does not currently have any frontend https logic baked in.

This is still the case, more or less, because the logic on the https embed here is based on the URL given, not based on whether or not the page is_ssl or not.

If you embed an image with http in a post and view the page over https, then you'll have the same basic error.

Content is kind of the user's domain. Until we have a more generalized approach for munging stuff into https for the general case, there's not really any reason to munge it specifically for the embed case.