Caching does not properly respect protocol (a problem when dealing with https)

To clarify the problem here's an example - the filter cache may return content a rendered IMG tag (rendered, for example, by media module) that references a non-https URL if it was cached while begin viewed over http. This would lead to mixed content warnings for the page.

For the media example, I think a good solution is to avoid the problem and use a schemeless URI. I haven't seen a lot of info about them, I encountered them at #1180646: Linking to google_service.js with https protocol when SSL enabled. Seems like they are good to go, except for referencing style sheets, http://stackoverflow.com/questions/2181207/is-it-safe-to-use-schemeless-....

HTTPS isn't the only case where this can rear its head. If for example you want to route authenticated users through a separate domain for added security (via HTTP authentication, VPN, or something else), then you are also likely to see cached data containing the private domain. This can lead to random HTTP authentication popups, or just plain missing content.

Confirming #8 Images uploaded via SSL are NOT visible on the Non-SSL site, link to the images is https://site/image.png while the request is done over http://site/image.png. (and we run on a self-signed certificate... boem... error) So, let's do some heavy url rewriting for now...

I'm running into this both for the http/https issue

But also because I'm have a cluster of servers, and depending on whether I want to access a single server or a load balancer I use different hostnames - but then I'm seeing these stored in the cache

I really want to just have relative urls in the page

Marked #1032210: drupal_render_cid_parts() should add http vs. https protocol to cache keys a duplicate, so bringing over its tags to here.

I think we have these questions to figure out:
- For D8, should we switch to protocol relative URLs for everything except CSS?
- If yes to above, do we want to by default add protocol to the keys for caches mentioned in the issue summary anyway, so that we work with protocol absolute URLs added by contrib/custom code?
- If yes to above, do we want a way to easily toggle that behavior so that sites that stick to protocol relative URLs can get more cache reuse?

As stated in #5:

Going schemaless would be nice and maybe something we should try getting into Drupal 8 and Drupal 7 contrib.

is definitely the right way to do it.

#6 is spot on that this is an issue in file_create_url(). So it isn't a cache system issue but more a file system issue.

I have a patch made for D7 (see #1633558: file_create_url() changes relative URL to absolute URL for shipped files) which I am rewriting for D8 at the moment (at DrupalCon Munich).

This patch change file_create_url() so schemeless urls are kept schemeless by expanding a path relative to the Drupal installation to absolute path of the web server.

Updated patch to fix the 3 failed tests by using $GLOBALS['base_path'] instead of $GLOBALS['base_url'] . '/' and creating an absolute url for curl from a path (relative url) to reflect change of file_create_url() which no longer will add scheme to schemeless urls.

+++ b/core/includes/file.inc
@@ -460,9 +460,12 @@ function file_create_url($uri) {
+      // with the base path prepended. Base path is used because there should be
+      // no need to make a relative URI absolute which the use of base URL
+      // would do.
+      return $GLOBALS['base_path'] . drupal_encode_path($uri);

I'm not sure about this. This would fail to include the host name, so wouldn't be usable in an email or anywhere else absolute URLs are needed.

@effulgentsia you are absolutely right that the use file_create_url() wouldn't work where absolute URLs are required when the function is used on schemeless URLs (paths).

The original first part of file_create_url():

function file_create_url($uri) {
  // Allow the URI to be altered, e.g. to serve a file from a CDN or static
  // file server.
  drupal_alter('file_url', $uri);

  $scheme = file_uri_scheme($uri);

  if (!$scheme) {
    // Allow for:
    // - root-relative URIs (e.g. /foo.jpg in http://example.com/foo.jpg)
    // - protocol-relative URIs (e.g. //bar.jpg, which is expanded to
    //   http://example.com/bar.jpg by the browser when viewing a page over
    //   HTTP and to https://example.com/bar.jpg when viewing a HTTPS page)
    // Both types of relative URIs are characterized by a leading slash, hence
    // we can use a single check.
    if (drupal_substr($uri, 0, 1) == '/') {
      return $uri;
    }
    else {
      // If this is not a properly formatted stream, then it is a shipped file.
      // Therefore, return the urlencoded URI with the base URL prepended.
      return $GLOBALS['base_url'] . '/' . drupal_encode_path($uri);
    }
  }

Either all schemeless URLs passed to file_create_url() should be made absolute to ensure that they can be used in emails which would make caching very difficult to implement correctly or you should use the function url() to generate absolute URLs for emails which will give you complete control over how that resulting URLs look.

If schemeless URLs must be made absolute to make them useful in emails which protocol should added (http or https) and how to determine that—at the moment this depends on whether the action creating the email is done through https or not.

The real question is: When does Drupal Core need to generate absolute URLs for files inside the Drupal installation?

• Not for javascript files
• Not for files containing CSS
• Not for image files
• Not for emails because Core only sends emails in plain text

Updated patch with some documentation about the return value.

I am still convinced that the api of file_create_url() is broken and schemeless paths should not have a scheme added. The behavior is a reminiscence from D6 where all file path are expanded to absolute urls. Further more the documentation of the function does not describe whether or not paths for shipped files are changed.

For a backport to D7 a variable could be used for switching between behavior. If the variable is set file_create_url() behaves as the old broken api if actually needed by any module.

Strange the same patch has been attached twice!?

I have been working on a similar patch in #1494670: References to CSS, JS, and similar files should be root-relative URLs: avoids mixed content warnings & fewer bytes to send. Mine adds a parameter to file_create_url() allowing one to control whether or not an absolute URL is generated.

Reroll.

I think in general schemaless is ok, but it won't solve the issue if you need to server https content from a different domain than you do http. As could be the case if you were using a CDN, which didn't support SSL.

Reroll.

Reroll.

28: file_create_url_does_not_add_scheme_to_schemeless_url-1377840.patch queued for re-testing.

Straight reroll.

I updated the tests added in #2276203: CSS Aggregation breaks URLs with Query String to assert a relative url.

Re-rolling the patch

Removing sprint weekend tag!!
As suggested by @YesCT

This honestly seems like a major bug for sites that are available on both HTTP and HTTPS. For example, the logo in the branding block could use an HTTP URL on HTTPS pages, which causes a mixed content warning.

I didn't yet have a chance to read thru all of the above discussion, but I think adding url.site to the required_cache_contexts would be a reasonable solution for the problem I'm seeing: required_cache_contexts: ['languages:language_interface', 'theme', 'user.permissions', 'url.site']

Interesting. I'm wondering whether we could switch to protocol relative URLs everywhere. This would allow us to not care about HTTP/HTTPs ... but yeah in reality url.site doesn't add a incredible big amount of additional cache entries, especially when in the future HTTPs will be the de facto standard

Even better than protocol-relative URLs would be server-relative. There is a patch for that in #1494670: References to CSS, JS, and similar files should be root-relative URLs: avoids mixed content warnings & fewer bytes to send.

Let's see if this passes tests.

It will certainly not pass since tons of tests are looking for cache contexts in various places.

Can't we instead make sure that we add that cache context where we don't already? I know that every absolute url generated through the UrlGenerator does add this, so those should be fine.

46 tests to be exact. That sounds like a reasonable approach, if we can easily add cache context everywhere that the base URL is used.

This got brought up in the context of #2606918: [securelogin] Secure Login so calling this a contrib project blocker.

I didn't yet have a chance to read thru all of the above discussion, but I think adding url.site to the required_cache_contexts

-1. We should only do that if absolutely necessary.

Can't we instead make sure that we add that cache context where we don't already? I know that every absolute url generated through the UrlGenerator does add this, so those should be fine.

+1.

This will also be a problem for HTTP/2 support.

This issue is equivalent with #1494670: References to CSS, JS, and similar files should be root-relative URLs: avoids mixed content warnings & fewer bytes to send. This issue is older, so strictly speaking we should be solving it here. But, #1494670: References to CSS, JS, and similar files should be root-relative URLs: avoids mixed content warnings & fewer bytes to send was much further along with a fix. So, closing this issue in favor of that one. A patch that is ready for final review is awaiting you there. Hopefully see you there!