The embedded iframe filter doesn't support scheme-relative URLs such as "//youtube.com" (now used in the YouTube and Vimeo embed code)

Bug #1207140 reported by Kristina Hoeppner on 2013-08-01
12
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Mahara
High
Robert Lyon
1.6
High
Robert Lyon
1.7
High
Robert Lyon
1.8
High
Robert Lyon

Bug Description

In response to FireFox's change in not supporting iframes with a http/https protocol that doesn't match the protocol of the parent page, YouTube's embed code now lists protocol-relative URLs. These start with "//www.youtube.com", no "http://". This is a standard type of relative URL/URI, but our code doesn't support it.

Since we already store the allowed iframe domains without a protocol in front of them, we should also support these protocol-relative URLs.

To replicate:
1. Open up a YouTube video on youtube.com
2. Click "Share" and then "Embed"
3. Make sure you have NOT ticked the "Use old embed code" box
4. You should get an iframe embed code, like this: <iframe width="420" height="315" src="//www.youtube.com/embed/NkyEOrQiGMQ" frameborder="0" allowfullscreen></iframe>
5. Paste this into an external media block or a text block in Mahara

Expected result: You should be able to view the video once you've saved the block
Actual result: The block will not display the video

Robert Lyon (robertl-9) wrote :

Patch submitted
https://reviews.mahara.org/#/c/2384/

This checks if the iframe code is from youtube and inserts the correct protocol needed.

Changed in mahara:
assignee: nobody → Robert Lyon (robertl-9)
status: Confirmed → In Progress

Reviewed: https://reviews.mahara.org/2385
Committed: http://gitorious.org/mahara/mahara/commit/dac25c9b90c1de8815db6acbe18242fe2789b406
Submitter: Son Nguyen (<email address hidden>)
Branch: 1.7_STABLE

commit dac25c9b90c1de8815db6acbe18242fe2789b406
Author: Robert Lyon <email address hidden>
Date: Fri Aug 2 10:11:23 2013 +1200

Add protocol to Youtube iframe code (Bug #1207140)

The youtube iframe code lacks the http: or https: protocol
- so we add it in based on mahara site protocol

Change-Id: Id1c8e026c9bad3baa07643dea5bd30477412902e
Signed-off-by: Robert Lyon <email address hidden>

Mahara Bot (dev-mahara) wrote :

Reviewed: https://reviews.mahara.org/2384
Committed: http://gitorious.org/mahara/mahara/commit/66674d0c2190354b9d24a5632af60650b05ceea8
Submitter: Son Nguyen (<email address hidden>)
Branch: master

commit 66674d0c2190354b9d24a5632af60650b05ceea8
Author: Robert Lyon <email address hidden>
Date: Fri Aug 2 10:11:23 2013 +1200

Add protocol to Youtube iframe code (Bug #1207140)

The youtube iframe code lacks the http: or https: protocol
- so we add it in based on mahara site protocol

Change-Id: Id1c8e026c9bad3baa07643dea5bd30477412902e
Signed-off-by: Robert Lyon <email address hidden>

Son Nguyen (ngson2000) on 2013-08-12
Changed in mahara:
status: In Progress → Fix Committed

I just had a discussion with Robert about this. The "//" URL is a standard (though somewhat obscure) way to write a URL. It's usually referred to as a "protocol-relative URL", it's part of the RFC definition for URLs, and it's supported by all the modern browsers including IE7+ (although apparently IE7 & 8 will attempt to fetch the http and https version a protocol-relative URL resource http://billpatrianakos.me/blog/2013/04/18/protocol-relative-urls/ )

Since Firefox 23 has just introduced a new default setting that blocks iframes which have a different http/https protocol than the parent page, I bet we're going to see many more vendors start adopting protocol-relative URLs in their embed code. So what we ought to do is adapt Mahara to handle these properly.

Although, for backwards-compatibility, we're also going to need to do a find/replace on all the existing external content blocks, to make sure their iframe URLs match up with the site's URL. That could either be a DB upgrade script, or it could be a last-minute find/replace in the external content block's rendering function.

Reviewed: https://reviews.mahara.org/2403
Committed: http://gitorious.org/mahara/mahara/commit/1bc5bd1a0a064fc0e99d2b87d02852f23ce1c461
Submitter: Aaron Wells (<email address hidden>)
Branch: master

commit 1bc5bd1a0a064fc0e99d2b87d02852f23ce1c461
Author: Aaron Wells <email address hidden>
Date: Tue Aug 13 18:44:16 2013 +1200

Revert "Add protocol to Youtube iframe code (Bug #1207140)"

This reverts commit 66674d0c2190354b9d24a5632af60650b05ceea8.

This patch adds an additional "http://" to the beginning of each URL each time you save an existing external content block! "http://" becomes "http://http://"

Change-Id: I37f21d30abad8c0b80bf2793614793f087d346fc

Reviewed: https://reviews.mahara.org/2404
Committed: http://gitorious.org/mahara/mahara/commit/c7f5694c57bd84f46caf32facbdc5b6619847cef
Submitter: Aaron Wells (<email address hidden>)
Branch: 1.7_STABLE

commit c7f5694c57bd84f46caf32facbdc5b6619847cef
Author: Aaron Wells <email address hidden>
Date: Tue Aug 13 18:46:04 2013 +1200

Revert "Add protocol to Youtube iframe code (Bug #1207140)"

This reverts commit dac25c9b90c1de8815db6acbe18242fe2789b406.

This patch adds an additional "http://" to the beginning of each URL each time you save an existing external content block! "http://" becomes "http://http://"

Change-Id: I82bc964c58406efeb3df73073e59965a7c527280

I reverted https://reviews.mahara.org/#/c/2384/ and https://reviews.mahara.org/#/c/2385/ because the patch had a rather dramatic regression. Each time you saved an existing external content block, it would add an additional "http://" to the front of each URL in it. On the second save, the URLs would be prefaced with "http://http://", on the third, "http://http://http://" and so forth.

I'm actually not finding *any* problem with the existing external content block handling the new YouTube iframes that have "//" in the URL. They seem to work fine, in HTTP and HTTPS sites, in Firefox and in Chrome (on Ubuntu). See for example: https://mahara.org/user/aaronw/nz-has-seals

I'm going to mark this issue "Invalid" for now. If someone can verify that the YouTube iframes with "//" URLs actually cause problems, feel free to reopen.

Changed in mahara:
status: Fix Committed → Invalid
status: Invalid → Incomplete
Aaron Wells (u-aaronw) wrote :

I also tested it by putting a YouTube embed with protocol-relative "//" URLs, in a text box, and that worked fine as well.

If anything, it's the *existing* YouTube embeds with the older non-protocol-relative URLs, which are likely to cause problems. But we've opened a separate bug for those: https://bugs.launchpad.net/mahara/+bug/1211583

Robert Lyon (robertl-9) wrote :

Ok there was a bug in my patch that slipped through.

However there still is a problem with embedding code into the externalmedia block.

If I go to a YouTube video and select the 'Embed' option I get to copy code that looks like this:
<iframe width="420" height="315" src="//www.youtube.com/embed/WM5Gwzk3Vfc" frameborder="0" allowfullscreen></iframe>

Adding that to the externalmedia block gives me no video.

However if I go to a YouTube video and select the 'Embed' option and also tick the 'use old embed code' I get to copy code that looks like this:
<object width="420" height="315"><param name="movie" value="//www.youtube.com/v/WM5Gwzk3Vfc?version=3&amp;hl=en_US"></param><param name="allowFullScreen" value="true"></param><param name="allowscriptaccess" value="always"></param><embed src="//www.youtube.com/v/WM5Gwzk3Vfc?version=3&amp;hl=en_US" type="application/x-shockwave-flash" width="420" height="315" allowscriptaccess="always" allowfullscreen="true"></embed></object>

That code does work for me.

So the question is - is it easier to fix the problem where the iframe code doesn't work or to tell everyone to click the 'use old embed code' option. I think having both work is a better choice.

Aaron Wells (u-aaronw) wrote :

Okay, that must have been why I didn't notice this. I was getting the old embed code -- perhaps YouTube had stored this as my preference at some point.

Re-opening. We should support the new and old embed code, by supporting scheme-relative URLs for iframes. This will probably require us rewriting the $cfg variable that stores the regular expression that checks for valid iframe URLs, and/or changing how we use htmlpurifier.

Changed in mahara:
status: Incomplete → In Progress
summary: - YouTube filter requires updates
+ YouTube iframe filter doesn't support the //youtube.com URLs YouTube now
+ provides in embed code
description: updated
Aaron Wells (u-aaronw) on 2013-08-13
description: updated
description: updated

https://reviews.mahara.org/#/c/2405/

Have submitted a new patch for this.

This patch updates the $cfg->iframeregexp to handle
- http://
- https://
- //

in iframe src code

Reviewed: https://reviews.mahara.org/2405
Committed: http://gitorious.org/mahara/mahara/commit/e1d79eceef5ff7ff938f5153fca895ef8e5c6d1f
Submitter: Aaron Wells (<email address hidden>)
Branch: master

commit e1d79eceef5ff7ff938f5153fca895ef8e5c6d1f
Author: Robert Lyon <email address hidden>
Date: Wed Aug 14 12:17:05 2013 +1200

New fix for the YouTube issue (bug #1207140)

This time around instead of changing the protocol-relative URLs
to be matching the protocol of what Mahara is using I've changed
how html purifier checks iframes to allow protocol-relative URLs.

Change-Id: I2a9436ecf3f6046acdefce8ac7751c12ad2bbf9d
Signed-off-by: Robert Lyon <email address hidden>
Signed-off-by: Aaron Wells <email address hidden>

Mahara Bot (dev-mahara) wrote :

Reviewed: https://reviews.mahara.org/2423
Committed: http://gitorious.org/mahara/mahara/commit/50cdcb130c147ec8eeb79551a470de0b0d40b0c9
Submitter: Aaron Wells (<email address hidden>)
Branch: 1.5_STABLE

commit 50cdcb130c147ec8eeb79551a470de0b0d40b0c9
Author: Robert Lyon <email address hidden>
Date: Wed Aug 21 11:34:43 2013 +1200

New fix for the YouTube issue (bug #1207140)

This time around instead of changing the protocol-relative URLs
to be matching the protocol of what Mahara is using I've changed
how html purifier checks iframes to allow protocol-relative URLs.

Change-Id: I913b189644d57cb8d77924d39da42ec0b328f3a6
Signed-off-by: Robert Lyon <email address hidden>

Robert Lyon (robertl-9) on 2013-08-21
Changed in mahara:
status: In Progress → Fix Committed
Mahara Bot (dev-mahara) wrote :

Reviewed: https://reviews.mahara.org/2421
Committed: http://gitorious.org/mahara/mahara/commit/e53161a32361900de3820e0d7601ea17d997c78f
Submitter: Aaron Wells (<email address hidden>)
Branch: 1.6_STABLE

commit e53161a32361900de3820e0d7601ea17d997c78f
Author: Robert Lyon <email address hidden>
Date: Wed Aug 14 12:17:05 2013 +1200

New fix for the YouTube issue (bug #1207140)

This time around instead of changing the protocol-relative URLs
to be matching the protocol of what Mahara is using I've changed
how html purifier checks iframes to allow protocol-relative URLs.

Change-Id: I50c848e7ae6c123b2b88de2021cecb1711a07520
Signed-off-by: Robert Lyon <email address hidden>

Mahara Bot (dev-mahara) wrote :

Reviewed: https://reviews.mahara.org/2420
Committed: http://gitorious.org/mahara/mahara/commit/17f2567244e73774dd0738036088fa4a55eaa1f4
Submitter: Aaron Wells (<email address hidden>)
Branch: 1.7_STABLE

commit 17f2567244e73774dd0738036088fa4a55eaa1f4
Author: Robert Lyon <email address hidden>
Date: Wed Aug 14 12:17:05 2013 +1200

New fix for the YouTube issue (bug #1207140)

This time around instead of changing the protocol-relative URLs
to be matching the protocol of what Mahara is using I've changed
how html purifier checks iframes to allow protocol-relative URLs.

Change-Id: Ia928616f47f99ea1859e5ab3c7942b7d0ff67e10
Signed-off-by: Robert Lyon <email address hidden>

Mahara Bot (dev-mahara) wrote :

Reviewed: https://reviews.mahara.org/2429
Committed: http://gitorious.org/mahara/mahara/commit/cf3deb956110b7026ec3b1af4abd5ec4338ee272
Submitter: Aaron Wells (<email address hidden>)
Branch: 1.6_STABLE

commit cf3deb956110b7026ec3b1af4abd5ec4338ee272
Author: Aaron Wells <email address hidden>
Date: Thu Aug 22 15:45:19 2013 +1200

Correct Mahara 1.6 version number

The patch for Bug #1207140 erroneously raised the Mahara 1.6 version number too high.
Mahara versioning policy says that version numbers within a stable branch should only
increment by 1.

Change-Id: Iedcca7281494f4826df60621fe48df0cba4f36e1

Mahara Bot (dev-mahara) wrote :

Reviewed: https://reviews.mahara.org/2428
Committed: http://gitorious.org/mahara/mahara/commit/443f1078debf756379e94d395e3d0021b69827cc
Submitter: Aaron Wells (<email address hidden>)
Branch: 1.7_STABLE

commit 443f1078debf756379e94d395e3d0021b69827cc
Author: Aaron Wells <email address hidden>
Date: Thu Aug 22 15:45:19 2013 +1200

Correct Mahara 1.7 version number

The patch for Bug #1207140 erroneously raised the Mahara 1.7 version number too high.
Mahara versioning policy says that version numbers within a stable branch should only
increment by 1.

Change-Id: Iedcca7281494f4826df60621fe48df0cba4f36e1

The patches for 1.6 and 1.7 incremented the date portion of the database version number, instead of just incrementing the last two digits. I pushed through a couple of quick-fix patches to revert the database version number back to what it should be (see the two previous comments by MaharaBot), and posted an announcement on the Mahara.org developer forums for anyone unfortunate enough to have deployed the HEAD of 1.6_STABLE or 1.7_STABLE to their site in the past 26 hours: https://mahara.org/interaction/forum/topic.php?id=5631

Aaron Wells (u-aaronw) on 2013-09-22
summary: - YouTube iframe filter doesn't support the //youtube.com URLs YouTube now
- provides in embed code
+ The embedded iframe filter doesn't support scheme-relative URLs such as
+ "//youtube.com" (now used in the YouTube and Vimeo embed code)
Aaron Wells (u-aaronw) on 2013-09-30
Changed in mahara:
milestone: 1.5.12 → 1.8.0
Aaron Wells (u-aaronw) on 2013-10-24
Changed in mahara:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Duplicates of this bug

Other bug subscribers