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): """