Problem

file_create_url() currently does not allow protocol-relative (//files/image.jpg or even ://files/image.jpg) nor root-relative (/files/image.jpg) file URLs to be created.

Protocol-relative URLs

Protocol-relative URLs are a necessity to be able to transparently switch between the HTTP and HTTPs version of a site, i.e. without generating different HTML.

What is gained:

Root-relative URLs

I personally don't use root-relative URLs, but there's no good reason that I know of to prevent these URLs from being created.

What is gained:

  • This would allow Drupal 7 ports of modules such as Pathologic to continue to offer full flexibility in configuring preferred URL styles.

Solution

Patch attached that makes this possible. Comes with updated unit tests, that pass.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Status: Needs review » Needs work

The last submitted patch, file_create_url-root-relative-and-protocol-relative-urls.patch, failed testing.

Wim Leers’s picture

Version: 7.0-alpha3 » 7.x-dev
Status: Needs work » Needs review

Well, I can't reproduce the error. At least, not with HEAD. Apparently tests are running against alpha 3. Must be because I had set 7.0-alpha3 as the version.

Now I also installed D7 alpha 3, ran the test that is claimed to be failing, and found that it again fully passed. So there must be something wrong with the testing bot?

Marking as needs review and changed version to 7.x-dev. Hopefully testbot won't fail this time.

Wim Leers’s picture

aaron’s picture

subscribe

aaron’s picture

chatted briefly w/ Wim in irc: the patch looks good after a review. looks like it would largely handle unmanaged shipped files, and since it's not actually touching streams, I don't think it would introduce anything that would negatively impact any existing functionality. thus, it looks to be beneficial, especially as its tests are thorough.

the only minor bit is that

if (drupal_substr($uri, 0, 1) == '/' || drupal_substr($uri, 0, 2) == '//') {

really only needs to be

if (drupal_substr($uri, 0, 1) == '/') {

from the standpoint of the computer (with maybe a quick comment for human evaluators).

Wim Leers’s picture

FileSize
10.02 KB

Thanks again for your time Aaron! :)

I fully agree with your concerns. Patch rerolled and with hopefully sufficiently clear explanation in the comment.

Wim Leers’s picture

Also, a quick note: I cite ://files/image.jpg as an example of a protocol-relative URI in the first post. While that is true, it's not properly supported in all browsers: at least Webkit-based browsers don't support this, and possibly others. And since //files/image.jpg is equivalent and most definitely more widespread, only the latter are supported by this core patch. But again: this does not reduce the spectrum of possibilities.

aaron’s picture

Status: Needs review » Reviewed & tested by the community

this looks excellent, thanks Wim!

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks.

Wim Leers’s picture

Tagging for better findability in the future

Status: Fixed » Closed (fixed)

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

hass’s picture

@Wim: Do I need secure pages module to get urls absolute to the root in source code (/drupal7/foo/bar.png) - otherwise they are still kept full qualified?