From 85304b8b9d91d75221992069a06fbe2e24f23c63 Mon Sep 17 00:00:00 2001
From: Toby Lawrence <tlawrence@edx.org>
Date: Thu, 17 Mar 2016 11:15:57 -0400
Subject: [PATCH] Make CDNifying of course over image URLs only happen for
 relative URLs.

We don't want to blindly assemble the base CDN URL with whatever an
image URL happens to be, since it might be an absolute URL and now the
result is a broken URL.  We take a more selective approach now.
---
 .../content/course_overviews/models.py        | 30 ++++++++++++++++---
 .../content/course_overviews/tests.py         | 25 ++++++++++++++++
 2 files changed, 51 insertions(+), 4 deletions(-)

diff --git a/openedx/core/djangoapps/content/course_overviews/models.py b/openedx/core/djangoapps/content/course_overviews/models.py
index 0160dc15d5..3e1edf2080 100644
--- a/openedx/core/djangoapps/content/course_overviews/models.py
+++ b/openedx/core/djangoapps/content/course_overviews/models.py
@@ -3,7 +3,7 @@ Declaration of CourseOverview model
 """
 import json
 import logging
-from urlparse import urlunparse
+from urlparse import urlparse, urlunparse
 
 from django.db import models, transaction
 from django.db.models.fields import BooleanField, DateTimeField, DecimalField, TextField, FloatField, IntegerField
@@ -549,9 +549,9 @@ class CourseOverview(TimeStampedModel):
             urls['small'] = self.image_set.small_url or raw_image_url
             urls['large'] = self.image_set.large_url or raw_image_url
 
-        return self._apply_cdn(urls)
+        return self.apply_cdn_to_urls(urls)
 
-    def _apply_cdn(self, image_urls):
+    def apply_cdn_to_urls(self, image_urls):
         """
         Given a dict of resolutions -> urls, return a copy with CDN applied.
 
@@ -568,10 +568,32 @@ class CourseOverview(TimeStampedModel):
         base_url = cdn_config.base_url
 
         return {
-            resolution: urlunparse((None, base_url, url, None, None, None))
+            resolution: self._apply_cdn_to_url(url, base_url)
             for resolution, url in image_urls.items()
         }
 
+    def _apply_cdn_to_url(self, url, base_url):
+        """
+        Applies a new CDN/base URL to the given URL.
+
+        If a URL is absolute, we skip switching the host since it could
+        be a hostname that isn't behind our CDN, and we could unintentionally
+        break the URL overall.
+        """
+
+        # The URL can't be empty.
+        if not url:
+            return url
+
+        _, netloc, path, params, query, fragment = urlparse(url)
+
+        # If this is an absolute URL, just return it as is.  It could be a domain
+        # that isn't ours, and thus CDNing it would actually break it.
+        if netloc:
+            return url
+
+        return urlunparse((None, base_url, path, params, query, fragment))
+
     def __unicode__(self):
         """Represent ourselves with the course key."""
         return unicode(self.id)
diff --git a/openedx/core/djangoapps/content/course_overviews/tests.py b/openedx/core/djangoapps/content/course_overviews/tests.py
index 988cfda7af..c3198394df 100644
--- a/openedx/core/djangoapps/content/course_overviews/tests.py
+++ b/openedx/core/djangoapps/content/course_overviews/tests.py
@@ -669,6 +669,31 @@ class CourseOverviewImageSetTestCase(ModuleStoreTestCase):
             for url in overview.image_urls.values():
                 self.assertTrue(url.startswith(expected_cdn_url))
 
+    @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split)
+    def test_cdn_with_external_image(self, modulestore_type):
+        """
+        Test that we return CDN prefixed URLs unless they're absolute.
+        """
+        with self.store.default_store(modulestore_type):
+            course = CourseFactory.create(default_store=modulestore_type)
+            overview = CourseOverview.get_from_id(course.id)
+
+            # Now enable the CDN...
+            AssetBaseUrlConfig.objects.create(enabled=True, base_url='fakecdn.edx.org')
+            expected_cdn_url = "//fakecdn.edx.org"
+
+            start_urls = {
+                'raw': 'http://google.com/image.png',
+                'small': '/static/overview.png',
+                'large': ''
+            }
+
+            modified_urls = overview.apply_cdn_to_urls(start_urls)
+            self.assertEqual(modified_urls['raw'], start_urls['raw'])
+            self.assertNotEqual(modified_urls['small'], start_urls['small'])
+            self.assertTrue(modified_urls['small'].startswith(expected_cdn_url))
+            self.assertEqual(modified_urls['large'], start_urls['large'])
+
     @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split)
     def test_error_generating_thumbnails(self, modulestore_type):
         """