media: Take advantage of HTTP & HTTPS media being on the same domain.

Since we have an HTTPS-capable CDN in front of our S3 static domains now, it's far faster for clients to use the CDN on HTTPS as well rather than going straight to (high-latency) S3. This patch makes it so that we continue to store URLs with explicit HTTP schemes but instead of conditionally converting to HTTPS, we render protocol-relative URLs. This should be safe for systems using the filesystem media provider as we've installed an SSL cert there all along.
reddit-archive · Dec 1, 2014 · d24a8eb · d24a8eb
1 parent 155342f
commit d24a8eb
Show file tree

Hide file tree

Showing 10 changed files with 22 additions and 50 deletions.
diff --git a/install-reddit.sh b/install-reddit.sh
@@ -319,7 +319,6 @@ plugins = $plugin_str
 media_provider = filesystem
 media_fs_root = /srv/www/media
 media_fs_base_url_http = http://%(domain)s/media/
-media_fs_base_url_https = https://%(domain)s/media/
 
 [server:main]
 port = 8001

diff --git a/r2/example.ini b/r2/example.ini
@@ -233,7 +233,6 @@ s3_media_domain = s3.amazonaws.com
 # filesystem provider configuration
 media_fs_root =
 media_fs_base_url_http =
-media_fs_base_url_https =
 
 thumbnail_size = 70, 70
 thumbnail_hidpi_scaling = 2

diff --git a/r2/r2/controllers/api.py b/r2/r2/controllers/api.py
@@ -95,7 +95,7 @@
 from r2.controllers.api_docs import api_doc, api_section
 from r2.lib.search import SearchQuery
 from r2.controllers.oauth2 import require_oauth2_scope, allow_oauth2_access
-from r2.lib.template_helpers import add_sr, get_domain
+from r2.lib.template_helpers import add_sr, get_domain, make_url_protocol_relative
 from r2.lib.system_messages import notify_user_added
 from r2.controllers.ipn import generate_blob
 from r2.lib.lock import TimeoutExpired
@@ -4098,7 +4098,7 @@ def POST_setappicon(self, form, jquery, client, icon_file):
                 client._commit()
                 form.set_text('.status', 'uploaded')
                 jquery('#developed-app-%s .app-icon img'
-                       % client._id).attr('src', g.media_provider.convert_to_https(client.icon_url))
+                       % client._id).attr('src', make_url_protocol_relative(client.icon_url))
                 jquery('#developed-app-%s .ajax-upload-form'
                        % client._id).hide()
                 jquery('#developed-app-%s .edit-app-icon-button'

diff --git a/r2/r2/lib/filters.py b/r2/r2/lib/filters.py
@@ -270,7 +270,7 @@ def safemarkdown(text, nofollow=False, wrap=True, **kwargs):
         return SC_OFF + text + SC_ON
 
 def wikimarkdown(text, include_toc=True, target=None):
-    from r2.lib.template_helpers import media_https_if_secure
+    from r2.lib.template_helpers import make_url_protocol_relative
 
     # this hard codes the stylesheet page for now, but should be parameterized
     # in the future to allow per-page images.
@@ -285,7 +285,7 @@ def img_swap(tag):
         name = name and name.group(1)
         if name and name in page_images:
             url = page_images[name]
-            url = media_https_if_secure(url)
+            url = make_url_protocol_relative(url)
             tag['src'] = url
         else:
             tag.extract()

diff --git a/r2/r2/lib/pages/pages.py b/r2/r2/lib/pages/pages.py
@@ -120,10 +120,10 @@
 from r2.lib.utils import trunc_time, timesince, timeuntil, weighted_lottery
 from r2.lib.template_helpers import (
     add_sr,
-    get_domain,
-    format_number,
-    media_https_if_secure,
     comment_label,
+    format_number,
+    get_domain,
+    make_url_protocol_relative,
     static,
 )
 from r2.lib.subreddit_search import popular_searches
@@ -2397,7 +2397,7 @@ class SubredditStylesheet(Templated):
     def __init__(self, site = None,
                  stylesheet_contents = ''):
         raw_images = ImagesByWikiPage.get_images(c.site, "config/stylesheet")
-        images = {name: media_https_if_secure(url)
+        images = {name: make_url_protocol_relative(url)
                   for name, url in raw_images.iteritems()}
 
         Templated.__init__(self, site = site, images=images,

diff --git a/r2/r2/lib/providers/media/__init__.py b/r2/r2/lib/providers/media/__init__.py
@@ -35,11 +35,8 @@ def put(self, name, contents):
 
         `contents` is a byte string of the contents of the file.
 
-        The return value should be an absolute URL with the `http` scheme.
+        The return value should be an absolute URL with the `http` scheme but
+        should also work if accessed with `https`.
 
         """
         raise NotImplementedError
-
-    def convert_to_https(self, http_url):
-        """Return an HTTPS url for a given HTTP media URL."""
-        raise NotImplementedError
diff --git a/r2/r2/lib/providers/media/filesystem.py b/r2/r2/lib/providers/media/filesystem.py
@@ -38,16 +38,15 @@ class FileSystemMediaProvider(MediaProvider):
     `media_fs_root` is the root directory on the filesystem to write the objects
     into.
 
-    `media_fs_base_url_http` and `media_fs_base_url_https` are the base URLs on
-    which to find the media objects. They should be an absolute URL to the root
-    directory of the media object server.
+    `media_fs_base_url_http` is the base URL on which to find the media
+    objects. It should be an absolute URL to the root directory of the media
+    object server that is accessible via both HTTP and HTTPS.
 
     """
     config = {
         ConfigValue.str: [
             "media_fs_root",
             "media_fs_base_url_http",
-            "media_fs_base_url_https",
         ],
     }
 
@@ -57,8 +56,3 @@ def put(self, name, contents):
         with open(path, "w") as f:
             f.write(contents)
         return urlparse.urljoin(g.media_fs_base_url_http, name)
-
-    def convert_to_https(self, http_url):
-        # http://whatever.com/whatever/filename.jpg -> filename.jpg
-        name = http_url[http_url.rfind("/") + 1:]
-        return urlparse.urljoin(g.media_fs_base_url_https, name)
diff --git a/r2/r2/lib/providers/media/s3.py b/r2/r2/lib/providers/media/s3.py
@@ -94,17 +94,3 @@ def put(self, name, contents):
             return "http://%s/%s/%s" % (g.s3_media_domain, bucket_name, name)
         else:
             return "http://%s/%s" % (bucket_name, name)
-
-    def convert_to_https(self, http_url):
-        """Convert an HTTP URL on S3 to an HTTPS URL.
-
-        This currently assumes that no HTTPS-configured CDN is present, so
-        HTTPS URLs must be direct-S3 URLs so that we can use Amazon's certs.
-
-        """
-        if http_url.startswith("http://%s" % g.s3_media_domain):
-            # it's already a direct url, just change scheme
-            return http_url.replace("http://", "https://")
-        else:
-            # an indirect url, put the s3 domain in there too
-            return http_url.replace("http://", "https://%s/" % g.s3_media_domain)
diff --git a/r2/r2/lib/template_helpers.py b/r2/r2/lib/template_helpers.py
@@ -103,24 +103,18 @@ def static(path):
 
 
 def make_url_protocol_relative(url):
-    if not url:
+    if not url or url.startswith("//"):
         return url
 
-    assert url.startswith("http://"), "make_url_protocol_relative: not http"
-    return url[len("http:"):]
-
-
-def media_https_if_secure(url):
-    if not c.secure:
-        return url
-    return g.media_provider.convert_to_https(url)
+    scheme, netloc, path, query, fragment = urlparse.urlsplit(url)
+    return urlparse.urlunsplit((None, netloc, path, query, fragment))
 
 
 def header_url(url):
     if url == g.default_header_url:
         return static(url)
     else:
-        return media_https_if_secure(url)
+        return make_url_protocol_relative(url)
 
 
 def js_config(extra_config=None):

diff --git a/r2/r2/models/subreddit.py b/r2/r2/models/subreddit.py
@@ -568,6 +568,8 @@ def can_change_stylesheet(self, user):
 
     def parse_css(self, content, verify=True):
         from r2.lib import cssfilter
+        from r2.lib.template_helpers import make_url_protocol_relative
+
         if g.css_killswitch or (verify and not self.can_change_stylesheet(c.user)):
             return (None, None)
 
@@ -582,7 +584,7 @@ def parse_css(self, content, verify=True):
         )
 
         # parse and resolve images with https-safe urls
-        https_images = {name: g.media_provider.convert_to_https(url)
+        https_images = {name: make_url_protocol_relative(url)
                         for name, url in http_images.iteritems()}
         parsed_https, errors_https = cssfilter.validate_css(
             content,
@@ -595,6 +597,7 @@ def parse_css(self, content, verify=True):
     def change_css(self, content, parsed, prev=None, reason=None, author=None, force=False):
         from r2.models import ModAction
         from r2.lib.media import upload_stylesheet
+        from r2.lib.template_helpers import make_url_protocol_relative
 
         author = author if author else c.user._id36
         if content is None:
@@ -608,7 +611,7 @@ def change_css(self, content, parsed, prev=None, reason=None, author=None, force
         minified_http, minified_https = parsed
         if minified_http or minified_https:
             self.stylesheet_url_http = upload_stylesheet(minified_http)
-            self.stylesheet_url_https = g.media_provider.convert_to_https(
+            self.stylesheet_url_https = make_url_protocol_relative(
                 upload_stylesheet(minified_https))
         else:
             self.stylesheet_url_http = ""