From 1183f6cec93f02f6be542500fb68c20837c3e6e0 Mon Sep 17 00:00:00 2001 From: Toby Lawrence Date: Mon, 1 Feb 2016 21:44:35 -0500 Subject: [PATCH 1/2] Add the ability to exclude files from URL canonicalization. To be able to more easily deal with same-origin issues, due to some course assets being loaded in an iframe, we're adding the ability to specify file extensions that should be excluded from URL canonicalization. The default value is simply 'html', which means we won't rewrite asset links that point to files ending in .html. Thus, they'll be loaded from the same origin as the parent page, and voila, no SOP issues. --- common/djangoapps/static_replace/__init__.py | 5 +- .../0002_assetexcludedextensionsconfig.py | 27 ++ common/djangoapps/static_replace/models.py | 24 + .../test/test_static_replace.py | 413 +++++++++++++----- .../xmodule/xmodule/contentstore/content.py | 19 +- 5 files changed, 376 insertions(+), 112 deletions(-) create mode 100644 common/djangoapps/static_replace/migrations/0002_assetexcludedextensionsconfig.py diff --git a/common/djangoapps/static_replace/__init__.py b/common/djangoapps/static_replace/__init__.py index 64145302ffc3..84c81bd18779 100644 --- a/common/djangoapps/static_replace/__init__.py +++ b/common/djangoapps/static_replace/__init__.py @@ -5,7 +5,7 @@ from django.contrib.staticfiles import finders from django.conf import settings -from static_replace.models import AssetBaseUrlConfig +from static_replace.models import AssetBaseUrlConfig, AssetExcludedExtensionsConfig from xmodule.modulestore.django import modulestore from xmodule.modulestore import ModuleStoreEnum from xmodule.contentstore.content import StaticContent @@ -182,7 +182,8 @@ def replace_static_url(original, prefix, quote, rest): # if not, then assume it's courseware specific content and then look in the # Mongo-backed database base_url = AssetBaseUrlConfig.get_base_url() - url = StaticContent.get_canonicalized_asset_path(course_id, rest, base_url) + excluded_exts = AssetExcludedExtensionsConfig.get_excluded_extensions() + url = StaticContent.get_canonicalized_asset_path(course_id, rest, base_url, excluded_exts) if AssetLocator.CANONICAL_NAMESPACE in url: url = url.replace('block@', 'block/', 1) diff --git a/common/djangoapps/static_replace/migrations/0002_assetexcludedextensionsconfig.py b/common/djangoapps/static_replace/migrations/0002_assetexcludedextensionsconfig.py new file mode 100644 index 000000000000..54a6ca4a4853 --- /dev/null +++ b/common/djangoapps/static_replace/migrations/0002_assetexcludedextensionsconfig.py @@ -0,0 +1,27 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations, models +import django.db.models.deletion +from django.conf import settings + + +class Migration(migrations.Migration): + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ('static_replace', '0001_initial'), + ] + + operations = [ + migrations.CreateModel( + name='AssetExcludedExtensionsConfig', + fields=[ + ('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)), + ('change_date', models.DateTimeField(auto_now_add=True, verbose_name='Change date')), + ('enabled', models.BooleanField(default=False, verbose_name='Enabled')), + ('excluded_extensions', models.TextField(default=b'html', help_text=b'The file extensions to exclude from canonicalization. No leading period required. Values should be space separated i.e. "html svg css"')), + ('changed_by', models.ForeignKey(on_delete=django.db.models.deletion.PROTECT, editable=False, to=settings.AUTH_USER_MODEL, null=True, verbose_name='Changed by')), + ], + ), + ] diff --git a/common/djangoapps/static_replace/models.py b/common/djangoapps/static_replace/models.py index 7521d885bbd7..8c73889c54ba 100644 --- a/common/djangoapps/static_replace/models.py +++ b/common/djangoapps/static_replace/models.py @@ -27,3 +27,27 @@ def __repr__(self): def __unicode__(self): return unicode(repr(self)) + + +class AssetExcludedExtensionsConfig(ConfigurationModel): + """Configuration for the the excluded file extensions when canonicalizing static asset paths.""" + + class Meta(object): + app_label = 'static_replace' + + excluded_extensions = TextField( + default='html', + help_text='The file extensions to exclude from canonicalization. No leading period required. ' + + 'Values should be space separated i.e. "html svg css"' + ) + + @classmethod + def get_excluded_extensions(cls): + """Gets the excluded file extensions when canonicalizing static asset paths""" + return cls.current().excluded_extensions.split() + + def __repr__(self): + return ''.format(self.get_excluded_extensions().split()) + + def __unicode__(self): + return unicode(repr(self)) diff --git a/common/djangoapps/static_replace/test/test_static_replace.py b/common/djangoapps/static_replace/test/test_static_replace.py index 56f90bd0be1a..37b2fc48db16 100644 --- a/common/djangoapps/static_replace/test/test_static_replace.py +++ b/common/djangoapps/static_replace/test/test_static_replace.py @@ -98,11 +98,13 @@ def test_storage_url_not_exists(mock_storage): @patch('static_replace.StaticContent', autospec=True) @patch('static_replace.modulestore', autospec=True) @patch('static_replace.AssetBaseUrlConfig.get_base_url') -def test_mongo_filestore(mock_get_base_url, mock_modulestore, mock_static_content): +@patch('static_replace.AssetExcludedExtensionsConfig.get_excluded_extensions') +def test_mongo_filestore(mock_get_excluded_extensions, mock_get_base_url, mock_modulestore, mock_static_content): mock_modulestore.return_value = Mock(MongoModuleStore) mock_static_content.get_canonicalized_asset_path.return_value = "c4x://mock_url" mock_get_base_url.return_value = u'' + mock_get_excluded_extensions.return_value = ['foobar'] # No namespace => no change to path assert_equals('"/static/data_dir/file.png"', replace_static_urls(STATIC_SOURCE, DATA_DIRECTORY)) @@ -113,7 +115,7 @@ def test_mongo_filestore(mock_get_base_url, mock_modulestore, mock_static_conten replace_static_urls(STATIC_SOURCE, DATA_DIRECTORY, course_id=COURSE_KEY) ) - mock_static_content.get_canonicalized_asset_path.assert_called_once_with(COURSE_KEY, 'file.png', u'') + mock_static_content.get_canonicalized_asset_path.assert_called_once_with(COURSE_KEY, 'file.png', u'', ['foobar']) @patch('static_replace.settings', autospec=True) @@ -217,6 +219,13 @@ def setUpClass(cls): # Create an unlocked image with funky characters in the name. cls.create_image(prefix, (1, 1), 'black', 'weird {}_unlock.png') + cls.create_image(prefix, (1, 1), 'black', 'special/weird {}_unlock.png') + + # Create an HTML file to test extension exclusion, and create a control file. + cls.create_arbitrary_content(prefix, '{}_not_excluded.htm') + cls.create_arbitrary_content(prefix, '{}_excluded.html') + cls.create_arbitrary_content(prefix, 'special/{}_not_excluded.htm') + cls.create_arbitrary_content(prefix, 'special/{}_excluded.html') @classmethod def create_image(cls, prefix, dimensions, color, name, locked=False): @@ -244,83 +253,195 @@ def create_image(cls, prefix, dimensions, color, name, locked=False): return new_content + @classmethod + def create_arbitrary_content(cls, prefix, name, locked=False): + """ + Creates an arbitrary piece of content with a fixed body, for when content doesn't matter. + + Args: + prefix: the prefix to use e.g. split vs mongo + name: the name of the content; can be a format string + locked: whether or not the asset should be locked + + Returns: + StaticContent: the StaticContent object for the created content + + """ + new_buf = StringIO('testingggggggggggg') + new_name = name.format(prefix) + new_key = StaticContent.compute_location(cls.courses[prefix].id, new_name) + new_content = StaticContent(new_key, new_name, 'application/octet-stream', new_buf.getvalue(), locked=locked) + contentstore().save(new_content) + + return new_content + @ddt.data( # No leading slash. - (u'', u'{prefix}_unlock.png', u'/{asset_key}@{prefix}_unlock.png', 1), - (u'', u'{prefix}_lock.png', u'/{asset_key}@{prefix}_lock.png', 1), - (u'', u'weird {prefix}_unlock.png', u'/{asset_key}@weird_{prefix}_unlock.png', 1), - (u'dev', u'{prefix}_unlock.png', u'//dev/{asset_key}@{prefix}_unlock.png', 1), - (u'dev', u'{prefix}_lock.png', u'/{asset_key}@{prefix}_lock.png', 1), - (u'dev', u'weird {prefix}_unlock.png', u'//dev/{asset_key}@weird_{prefix}_unlock.png', 1), + (u'', u'{prfx}_unlock.png', u'/{asset}@{prfx}_unlock.png', 1), + (u'', u'{prfx}_lock.png', u'/{asset}@{prfx}_lock.png', 1), + (u'', u'weird {prfx}_unlock.png', u'/{asset}@weird_{prfx}_unlock.png', 1), + (u'', u'{prfx}_excluded.html', u'/{asset}@{prfx}_excluded.html', 1), + (u'', u'{prfx}_not_excluded.htm', u'/{asset}@{prfx}_not_excluded.htm', 1), + (u'dev', u'{prfx}_unlock.png', u'//dev/{asset}@{prfx}_unlock.png', 1), + (u'dev', u'{prfx}_lock.png', u'/{asset}@{prfx}_lock.png', 1), + (u'dev', u'weird {prfx}_unlock.png', u'//dev/{asset}@weird_{prfx}_unlock.png', 1), + (u'dev', u'{prfx}_excluded.html', u'/{asset}@{prfx}_excluded.html', 1), + (u'dev', u'{prfx}_not_excluded.htm', u'//dev/{asset}@{prfx}_not_excluded.htm', 1), # No leading slash with subdirectory. This ensures we properly substitute slashes. - (u'', u'special/{prefix}_unlock.png', u'/{asset_key}@special_{prefix}_unlock.png', 1), - (u'', u'special/{prefix}_lock.png', u'/{asset_key}@special_{prefix}_lock.png', 1), - (u'dev', u'special/{prefix}_unlock.png', u'//dev/{asset_key}@special_{prefix}_unlock.png', 1), - (u'dev', u'special/{prefix}_lock.png', u'/{asset_key}@special_{prefix}_lock.png', 1), + (u'', u'special/{prfx}_unlock.png', u'/{asset}@special_{prfx}_unlock.png', 1), + (u'', u'special/{prfx}_lock.png', u'/{asset}@special_{prfx}_lock.png', 1), + (u'', u'special/weird {prfx}_unlock.png', u'/{asset}@special_weird_{prfx}_unlock.png', 1), + (u'', u'special/{prfx}_excluded.html', u'/{asset}@special_{prfx}_excluded.html', 1), + (u'', u'special/{prfx}_not_excluded.htm', u'/{asset}@special_{prfx}_not_excluded.htm', 1), + (u'dev', u'special/{prfx}_unlock.png', u'//dev/{asset}@special_{prfx}_unlock.png', 1), + (u'dev', u'special/{prfx}_lock.png', u'/{asset}@special_{prfx}_lock.png', 1), + (u'dev', u'special/weird {prfx}_unlock.png', u'//dev/{asset}@special_weird_{prfx}_unlock.png', 1), + (u'dev', u'special/{prfx}_excluded.html', u'/{asset}@special_{prfx}_excluded.html', 1), + (u'dev', u'special/{prfx}_not_excluded.htm', u'//dev/{asset}@special_{prfx}_not_excluded.htm', 1), # Leading slash. - (u'', u'/{prefix}_unlock.png', u'/{asset_key}@{prefix}_unlock.png', 1), - (u'', u'/{prefix}_lock.png', u'/{asset_key}@{prefix}_lock.png', 1), - (u'dev', u'/{prefix}_unlock.png', u'//dev/{asset_key}@{prefix}_unlock.png', 1), - (u'dev', u'/{prefix}_lock.png', u'/{asset_key}@{prefix}_lock.png', 1), + (u'', u'/{prfx}_unlock.png', u'/{asset}@{prfx}_unlock.png', 1), + (u'', u'/{prfx}_lock.png', u'/{asset}@{prfx}_lock.png', 1), + (u'', u'/weird {prfx}_unlock.png', u'/{asset}@weird_{prfx}_unlock.png', 1), + (u'', u'/{prfx}_excluded.html', u'/{asset}@{prfx}_excluded.html', 1), + (u'', u'/{prfx}_not_excluded.htm', u'/{asset}@{prfx}_not_excluded.htm', 1), + (u'dev', u'/{prfx}_unlock.png', u'//dev/{asset}@{prfx}_unlock.png', 1), + (u'dev', u'/{prfx}_lock.png', u'/{asset}@{prfx}_lock.png', 1), + (u'dev', u'/weird {prfx}_unlock.png', u'//dev/{asset}@weird_{prfx}_unlock.png', 1), + (u'dev', u'/{prfx}_excluded.html', u'/{asset}@{prfx}_excluded.html', 1), + (u'dev', u'/{prfx}_not_excluded.htm', u'//dev/{asset}@{prfx}_not_excluded.htm', 1), # Leading slash with subdirectory. This ensures we properly substitute slashes. - (u'', u'/special/{prefix}_unlock.png', u'/{asset_key}@special_{prefix}_unlock.png', 1), - (u'', u'/special/{prefix}_lock.png', u'/{asset_key}@special_{prefix}_lock.png', 1), - (u'dev', u'/special/{prefix}_unlock.png', u'//dev/{asset_key}@special_{prefix}_unlock.png', 1), - (u'dev', u'/special/{prefix}_lock.png', u'/{asset_key}@special_{prefix}_lock.png', 1), + (u'', u'/special/{prfx}_unlock.png', u'/{asset}@special_{prfx}_unlock.png', 1), + (u'', u'/special/{prfx}_lock.png', u'/{asset}@special_{prfx}_lock.png', 1), + (u'', u'/special/weird {prfx}_unlock.png', u'/{asset}@special_weird_{prfx}_unlock.png', 1), + (u'', u'/special/{prfx}_excluded.html', u'/{asset}@special_{prfx}_excluded.html', 1), + (u'', u'/special/{prfx}_not_excluded.htm', u'/{asset}@special_{prfx}_not_excluded.htm', 1), + (u'dev', u'/special/{prfx}_unlock.png', u'//dev/{asset}@special_{prfx}_unlock.png', 1), + (u'dev', u'/special/{prfx}_lock.png', u'/{asset}@special_{prfx}_lock.png', 1), + (u'dev', u'/special/weird {prfx}_unlock.png', u'//dev/{asset}@special_weird_{prfx}_unlock.png', 1), + (u'dev', u'/special/{prfx}_excluded.html', u'/{asset}@special_{prfx}_excluded.html', 1), + (u'dev', u'/special/{prfx}_not_excluded.htm', u'//dev/{asset}@special_{prfx}_not_excluded.htm', 1), # Static path. - (u'', u'/static/{prefix}_unlock.png', u'/{asset_key}@{prefix}_unlock.png', 1), - (u'', u'/static/{prefix}_lock.png', u'/{asset_key}@{prefix}_lock.png', 1), - (u'', u'/static/weird {prefix}_unlock.png', u'/{asset_key}@weird_{prefix}_unlock.png', 1), - (u'dev', u'/static/{prefix}_unlock.png', u'//dev/{asset_key}@{prefix}_unlock.png', 1), - (u'dev', u'/static/{prefix}_lock.png', u'/{asset_key}@{prefix}_lock.png', 1), - (u'dev', u'/static/weird {prefix}_unlock.png', u'//dev/{asset_key}@weird_{prefix}_unlock.png', 1), + (u'', u'/static/{prfx}_unlock.png', u'/{asset}@{prfx}_unlock.png', 1), + (u'', u'/static/{prfx}_lock.png', u'/{asset}@{prfx}_lock.png', 1), + (u'', u'/static/weird {prfx}_unlock.png', u'/{asset}@weird_{prfx}_unlock.png', 1), + (u'', u'/static/{prfx}_excluded.html', u'/{asset}@{prfx}_excluded.html', 1), + (u'', u'/static/{prfx}_not_excluded.htm', u'/{asset}@{prfx}_not_excluded.htm', 1), + (u'dev', u'/static/{prfx}_unlock.png', u'//dev/{asset}@{prfx}_unlock.png', 1), + (u'dev', u'/static/{prfx}_lock.png', u'/{asset}@{prfx}_lock.png', 1), + (u'dev', u'/static/weird {prfx}_unlock.png', u'//dev/{asset}@weird_{prfx}_unlock.png', 1), + (u'dev', u'/static/{prfx}_excluded.html', u'/{asset}@{prfx}_excluded.html', 1), + (u'dev', u'/static/{prfx}_not_excluded.htm', u'//dev/{asset}@{prfx}_not_excluded.htm', 1), # Static path with subdirectory. This ensures we properly substitute slashes. - (u'', u'/static/special/{prefix}_unlock.png', u'/{asset_key}@special_{prefix}_unlock.png', 1), - (u'', u'/static/special/{prefix}_lock.png', u'/{asset_key}@special_{prefix}_lock.png', 1), - (u'dev', u'/static/special/{prefix}_unlock.png', u'//dev/{asset_key}@special_{prefix}_unlock.png', 1), - (u'dev', u'/static/special/{prefix}_lock.png', u'/{asset_key}@special_{prefix}_lock.png', 1), + (u'', u'/static/special/{prfx}_unlock.png', u'/{asset}@special_{prfx}_unlock.png', 1), + (u'', u'/static/special/{prfx}_lock.png', u'/{asset}@special_{prfx}_lock.png', 1), + (u'', u'/static/special/weird {prfx}_unlock.png', u'/{asset}@special_weird_{prfx}_unlock.png', 1), + (u'', u'/static/special/{prfx}_excluded.html', u'/{asset}@special_{prfx}_excluded.html', 1), + (u'', u'/static/special/{prfx}_not_excluded.htm', u'/{asset}@special_{prfx}_not_excluded.htm', 1), + (u'dev', u'/static/special/{prfx}_unlock.png', u'//dev/{asset}@special_{prfx}_unlock.png', 1), + (u'dev', u'/static/special/{prfx}_lock.png', u'/{asset}@special_{prfx}_lock.png', 1), + (u'dev', u'/static/special/weird {prfx}_unlock.png', u'//dev/{asset}@special_weird_{prfx}_unlock.png', 1), + (u'dev', u'/static/special/{prfx}_excluded.html', u'/{asset}@special_{prfx}_excluded.html', 1), + (u'dev', u'/static/special/{prfx}_not_excluded.htm', u'//dev/{asset}@special_{prfx}_not_excluded.htm', 1), # Static path with query parameter. ( u'', - u'/static/{prefix}_unlock.png?foo=/static/{prefix}_lock.png', - u'/{asset_key}@{prefix}_unlock.png?foo={encoded_asset_key}{prefix}_lock.png', + u'/static/{prfx}_unlock.png?foo=/static/{prfx}_lock.png', + u'/{asset}@{prfx}_unlock.png?foo={encoded_asset}{prfx}_lock.png', + 2 + ), + ( + u'', + u'/static/{prfx}_lock.png?foo=/static/{prfx}_unlock.png', + u'/{asset}@{prfx}_lock.png?foo={encoded_asset}{prfx}_unlock.png', 2 ), ( u'', - u'/static/{prefix}_lock.png?foo=/static/{prefix}_unlock.png', - u'/{asset_key}@{prefix}_lock.png?foo={encoded_asset_key}{prefix}_unlock.png', + u'/static/{prfx}_excluded.html?foo=/static/{prfx}_excluded.html', + u'/{asset}@{prfx}_excluded.html?foo={encoded_asset}{prfx}_excluded.html', + 2 + ), + ( + u'', + u'/static/{prfx}_excluded.html?foo=/static/{prfx}_not_excluded.htm', + u'/{asset}@{prfx}_excluded.html?foo={encoded_asset}{prfx}_not_excluded.htm', + 2 + ), + ( + u'', + u'/static/{prfx}_not_excluded.htm?foo=/static/{prfx}_excluded.html', + u'/{asset}@{prfx}_not_excluded.htm?foo={encoded_asset}{prfx}_excluded.html', + 2 + ), + ( + u'', + u'/static/{prfx}_not_excluded.htm?foo=/static/{prfx}_not_excluded.htm', + u'/{asset}@{prfx}_not_excluded.htm?foo={encoded_asset}{prfx}_not_excluded.htm', + 2 + ), + ( + u'dev', + u'/static/{prfx}_unlock.png?foo=/static/{prfx}_lock.png', + u'//dev/{asset}@{prfx}_unlock.png?foo={encoded_asset}{prfx}_lock.png', + 2 + ), + ( + u'dev', + u'/static/{prfx}_lock.png?foo=/static/{prfx}_unlock.png', + u'/{asset}@{prfx}_lock.png?foo={encoded_base_url}{encoded_asset}{prfx}_unlock.png', 2 ), ( u'dev', - u'/static/{prefix}_unlock.png?foo=/static/{prefix}_lock.png', - u'//dev/{asset_key}@{prefix}_unlock.png?foo={encoded_asset_key}{prefix}_lock.png', + u'/static/{prfx}_excluded.html?foo=/static/{prfx}_excluded.html', + u'/{asset}@{prfx}_excluded.html?foo={encoded_asset}{prfx}_excluded.html', 2 ), ( u'dev', - u'/static/{prefix}_lock.png?foo=/static/{prefix}_unlock.png', - u'/{asset_key}@{prefix}_lock.png?foo={encoded_base_url}{encoded_asset_key}{prefix}_unlock.png', + u'/static/{prfx}_excluded.html?foo=/static/{prfx}_not_excluded.htm', + u'/{asset}@{prfx}_excluded.html?foo={encoded_base_url}{encoded_asset}{prfx}_not_excluded.htm', + 2 + ), + ( + u'dev', + u'/static/{prfx}_not_excluded.htm?foo=/static/{prfx}_excluded.html', + u'//dev/{asset}@{prfx}_not_excluded.htm?foo={encoded_asset}{prfx}_excluded.html', + 2 + ), + ( + u'dev', + u'/static/{prfx}_not_excluded.htm?foo=/static/{prfx}_not_excluded.htm', + u'//dev/{asset}@{prfx}_not_excluded.htm?foo={encoded_base_url}{encoded_asset}{prfx}_not_excluded.htm', 2 ), # Already asset key. - (u'', u'/{asset_key}@{prefix}_unlock.png', u'/{asset_key}@{prefix}_unlock.png', 1), - (u'', u'/{asset_key}@{prefix}_lock.png', u'/{asset_key}@{prefix}_lock.png', 1), - (u'dev', u'/{asset_key}@{prefix}_unlock.png', u'//dev/{asset_key}@{prefix}_unlock.png', 1), - (u'dev', u'/{asset_key}@{prefix}_lock.png', u'/{asset_key}@{prefix}_lock.png', 1), + (u'', u'/{asset}@{prfx}_unlock.png', u'/{asset}@{prfx}_unlock.png', 1), + (u'', u'/{asset}@{prfx}_lock.png', u'/{asset}@{prfx}_lock.png', 1), + (u'', u'/{asset}@weird_{prfx}_unlock.png', u'/{asset}@weird_{prfx}_unlock.png', 1), + (u'', u'/{asset}@{prfx}_excluded.html', u'/{asset}@{prfx}_excluded.html', 1), + (u'', u'/{asset}@{prfx}_not_excluded.htm', u'/{asset}@{prfx}_not_excluded.htm', 1), + (u'dev', u'/{asset}@{prfx}_unlock.png', u'//dev/{asset}@{prfx}_unlock.png', 1), + (u'dev', u'/{asset}@{prfx}_lock.png', u'/{asset}@{prfx}_lock.png', 1), + (u'dev', u'/{asset}@weird_{prfx}_unlock.png', u'//dev/{asset}@weird_{prfx}_unlock.png', 1), + (u'dev', u'/{asset}@{prfx}_excluded.html', u'/{asset}@{prfx}_excluded.html', 1), + (u'dev', u'/{asset}@{prfx}_not_excluded.htm', u'//dev/{asset}@{prfx}_not_excluded.htm', 1), # Old, c4x-style path. - (u'', u'/{c4x}/{prefix}_unlock.png', u'/{c4x}/{prefix}_unlock.png', 1), - (u'', u'/{c4x}/{prefix}_lock.png', u'/{c4x}/{prefix}_lock.png', 1), - (u'', u'/{c4x}/weird_{prefix}_lock.png', u'/{c4x}/weird_{prefix}_lock.png', 1), - (u'dev', u'/{c4x}/{prefix}_unlock.png', u'/{c4x}/{prefix}_unlock.png', 1), - (u'dev', u'/{c4x}/{prefix}_lock.png', u'/{c4x}/{prefix}_lock.png', 1), - (u'dev', u'/{c4x}/weird_{prefix}_unlock.png', u'/{c4x}/weird_{prefix}_unlock.png', 1), + (u'', u'/{c4x}/{prfx}_unlock.png', u'/{c4x}/{prfx}_unlock.png', 1), + (u'', u'/{c4x}/{prfx}_lock.png', u'/{c4x}/{prfx}_lock.png', 1), + (u'', u'/{c4x}/weird_{prfx}_lock.png', u'/{c4x}/weird_{prfx}_lock.png', 1), + (u'', u'/{c4x}/{prfx}_excluded.html', u'/{c4x}/{prfx}_excluded.html', 1), + (u'', u'/{c4x}/{prfx}_not_excluded.htm', u'/{c4x}/{prfx}_not_excluded.htm', 1), + (u'dev', u'/{c4x}/{prfx}_unlock.png', u'/{c4x}/{prfx}_unlock.png', 1), + (u'dev', u'/{c4x}/{prfx}_lock.png', u'/{c4x}/{prfx}_lock.png', 1), + (u'dev', u'/{c4x}/weird_{prfx}_unlock.png', u'/{c4x}/weird_{prfx}_unlock.png', 1), + (u'dev', u'/{c4x}/{prfx}_excluded.html', u'/{c4x}/{prfx}_excluded.html', 1), + (u'dev', u'/{c4x}/{prfx}_not_excluded.htm', u'/{c4x}/{prfx}_not_excluded.htm', 1), # Thumbnails. - (u'', u'/{th_key}@{prefix}_unlock-{th_ext}', u'/{th_key}@{prefix}_unlock-{th_ext}', 1), - (u'', u'/{th_key}@{prefix}_lock-{th_ext}', u'/{th_key}@{prefix}_lock-{th_ext}', 1), - (u'dev', u'/{th_key}@{prefix}_unlock-{th_ext}', u'//dev/{th_key}@{prefix}_unlock-{th_ext}', 1), - (u'dev', u'/{th_key}@{prefix}_lock-{th_ext}', u'//dev/{th_key}@{prefix}_lock-{th_ext}', 1), + (u'', u'/{th_key}@{prfx}_unlock-{th_ext}', u'/{th_key}@{prfx}_unlock-{th_ext}', 1), + (u'', u'/{th_key}@{prfx}_lock-{th_ext}', u'/{th_key}@{prfx}_lock-{th_ext}', 1), + (u'dev', u'/{th_key}@{prfx}_unlock-{th_ext}', u'//dev/{th_key}@{prfx}_unlock-{th_ext}', 1), + (u'dev', u'/{th_key}@{prfx}_lock-{th_ext}', u'//dev/{th_key}@{prfx}_lock-{th_ext}', 1), ) @ddt.unpack def test_canonical_asset_path_with_new_style_assets(self, base_url, start, expected, mongo_calls): @@ -333,95 +454,179 @@ def test_canonical_asset_path_with_new_style_assets(self, base_url, start, expec th_ext = 'png-16x16.jpg' start = start.format( - prefix=prefix, + prfx=prefix, c4x=c4x, - asset_key=asset_key, + asset=asset_key, encoded_base_url=encoded_base_url, - encoded_asset_key=encoded_asset_key, + encoded_asset=encoded_asset_key, th_key=th_key, th_ext=th_ext ) expected = expected.format( - prefix=prefix, + prfx=prefix, c4x=c4x, - asset_key=asset_key, + asset=asset_key, encoded_base_url=encoded_base_url, - encoded_asset_key=encoded_asset_key, + encoded_asset=encoded_asset_key, th_key=th_key, th_ext=th_ext ) with check_mongo_calls(mongo_calls): - asset_path = StaticContent.get_canonicalized_asset_path(self.courses[prefix].id, start, base_url) + asset_path = StaticContent.get_canonicalized_asset_path(self.courses[prefix].id, start, base_url, ['html']) self.assertEqual(asset_path, expected) @ddt.data( # No leading slash. - (u'', u'{prefix}_unlock.png', u'/{c4x}/{prefix}_unlock.png', 1), - (u'', u'{prefix}_lock.png', u'/{c4x}/{prefix}_lock.png', 1), - (u'', u'weird {prefix}_unlock.png', u'/{c4x}/weird_{prefix}_unlock.png', 1), - (u'dev', u'{prefix}_unlock.png', u'//dev/{c4x}/{prefix}_unlock.png', 1), - (u'dev', u'{prefix}_lock.png', u'/{c4x}/{prefix}_lock.png', 1), - (u'dev', u'weird {prefix}_unlock.png', u'//dev/{c4x}/weird_{prefix}_unlock.png', 1), + (u'', u'{prfx}_unlock.png', u'/{c4x}/{prfx}_unlock.png', 1), + (u'', u'{prfx}_lock.png', u'/{c4x}/{prfx}_lock.png', 1), + (u'', u'weird {prfx}_unlock.png', u'/{c4x}/weird_{prfx}_unlock.png', 1), + (u'', u'{prfx}_excluded.html', u'/{c4x}/{prfx}_excluded.html', 1), + (u'', u'{prfx}_not_excluded.htm', u'/{c4x}/{prfx}_not_excluded.htm', 1), + (u'dev', u'{prfx}_unlock.png', u'//dev/{c4x}/{prfx}_unlock.png', 1), + (u'dev', u'{prfx}_lock.png', u'/{c4x}/{prfx}_lock.png', 1), + (u'dev', u'weird {prfx}_unlock.png', u'//dev/{c4x}/weird_{prfx}_unlock.png', 1), + (u'dev', u'{prfx}_excluded.html', u'/{c4x}/{prfx}_excluded.html', 1), + (u'dev', u'{prfx}_not_excluded.htm', u'//dev/{c4x}/{prfx}_not_excluded.htm', 1), # No leading slash with subdirectory. This ensures we probably substitute slashes. - (u'', u'special/{prefix}_unlock.png', u'/{c4x}/special_{prefix}_unlock.png', 1), - (u'', u'special/{prefix}_lock.png', u'/{c4x}/special_{prefix}_lock.png', 1), - (u'dev', u'special/{prefix}_unlock.png', u'//dev/{c4x}/special_{prefix}_unlock.png', 1), - (u'dev', u'special/{prefix}_lock.png', u'/{c4x}/special_{prefix}_lock.png', 1), + (u'', u'special/{prfx}_unlock.png', u'/{c4x}/special_{prfx}_unlock.png', 1), + (u'', u'special/{prfx}_lock.png', u'/{c4x}/special_{prfx}_lock.png', 1), + (u'', u'special/weird {prfx}_unlock.png', u'/{c4x}/special_weird_{prfx}_unlock.png', 1), + (u'', u'special/{prfx}_excluded.html', u'/{c4x}/special_{prfx}_excluded.html', 1), + (u'', u'special/{prfx}_not_excluded.htm', u'/{c4x}/special_{prfx}_not_excluded.htm', 1), + (u'dev', u'special/{prfx}_unlock.png', u'//dev/{c4x}/special_{prfx}_unlock.png', 1), + (u'dev', u'special/{prfx}_lock.png', u'/{c4x}/special_{prfx}_lock.png', 1), + (u'dev', u'special/weird {prfx}_unlock.png', u'//dev/{c4x}/special_weird_{prfx}_unlock.png', 1), + (u'dev', u'special/{prfx}_excluded.html', u'/{c4x}/special_{prfx}_excluded.html', 1), + (u'dev', u'special/{prfx}_not_excluded.htm', u'//dev/{c4x}/special_{prfx}_not_excluded.htm', 1), # Leading slash. - (u'', u'/{prefix}_unlock.png', u'/{c4x}/{prefix}_unlock.png', 1), - (u'', u'/{prefix}_lock.png', u'/{c4x}/{prefix}_lock.png', 1), - (u'dev', u'/{prefix}_unlock.png', u'//dev/{c4x}/{prefix}_unlock.png', 1), - (u'dev', u'/{prefix}_lock.png', u'/{c4x}/{prefix}_lock.png', 1), + (u'', u'/{prfx}_unlock.png', u'/{c4x}/{prfx}_unlock.png', 1), + (u'', u'/{prfx}_lock.png', u'/{c4x}/{prfx}_lock.png', 1), + (u'', u'/weird {prfx}_unlock.png', u'/{c4x}/weird_{prfx}_unlock.png', 1), + (u'', u'/{prfx}_excluded.html', u'/{c4x}/{prfx}_excluded.html', 1), + (u'', u'/{prfx}_not_excluded.htm', u'/{c4x}/{prfx}_not_excluded.htm', 1), + (u'dev', u'/{prfx}_unlock.png', u'//dev/{c4x}/{prfx}_unlock.png', 1), + (u'dev', u'/{prfx}_lock.png', u'/{c4x}/{prfx}_lock.png', 1), + (u'dev', u'/weird {prfx}_unlock.png', u'//dev/{c4x}/weird_{prfx}_unlock.png', 1), + (u'dev', u'/{prfx}_excluded.html', u'/{c4x}/{prfx}_excluded.html', 1), + (u'dev', u'/{prfx}_not_excluded.htm', u'//dev/{c4x}/{prfx}_not_excluded.htm', 1), # Leading slash with subdirectory. This ensures we properly substitute slashes. - (u'', u'/special/{prefix}_unlock.png', u'/{c4x}/special_{prefix}_unlock.png', 1), - (u'', u'/special/{prefix}_lock.png', u'/{c4x}/special_{prefix}_lock.png', 1), - (u'dev', u'/special/{prefix}_unlock.png', u'//dev/{c4x}/special_{prefix}_unlock.png', 1), - (u'dev', u'/special/{prefix}_lock.png', u'/{c4x}/special_{prefix}_lock.png', 1), + (u'', u'/special/{prfx}_unlock.png', u'/{c4x}/special_{prfx}_unlock.png', 1), + (u'', u'/special/{prfx}_lock.png', u'/{c4x}/special_{prfx}_lock.png', 1), + (u'', u'/special/weird {prfx}_unlock.png', u'/{c4x}/special_weird_{prfx}_unlock.png', 1), + (u'', u'/special/{prfx}_excluded.html', u'/{c4x}/special_{prfx}_excluded.html', 1), + (u'', u'/special/{prfx}_not_excluded.htm', u'/{c4x}/special_{prfx}_not_excluded.htm', 1), + (u'dev', u'/special/{prfx}_unlock.png', u'//dev/{c4x}/special_{prfx}_unlock.png', 1), + (u'dev', u'/special/{prfx}_lock.png', u'/{c4x}/special_{prfx}_lock.png', 1), + (u'dev', u'/special/weird {prfx}_unlock.png', u'//dev/{c4x}/special_weird_{prfx}_unlock.png', 1), + (u'dev', u'/special/{prfx}_excluded.html', u'/{c4x}/special_{prfx}_excluded.html', 1), + (u'dev', u'/special/{prfx}_not_excluded.htm', u'//dev/{c4x}/special_{prfx}_not_excluded.htm', 1), # Static path. - (u'', u'/static/{prefix}_unlock.png', u'/{c4x}/{prefix}_unlock.png', 1), - (u'', u'/static/{prefix}_lock.png', u'/{c4x}/{prefix}_lock.png', 1), - (u'', u'/static/weird {prefix}_unlock.png', u'/{c4x}/weird_{prefix}_unlock.png', 1), - (u'dev', u'/static/{prefix}_unlock.png', u'//dev/{c4x}/{prefix}_unlock.png', 1), - (u'dev', u'/static/{prefix}_lock.png', u'/{c4x}/{prefix}_lock.png', 1), - (u'dev', u'/static/weird {prefix}_unlock.png', u'//dev/{c4x}/weird_{prefix}_unlock.png', 1), + (u'', u'/static/{prfx}_unlock.png', u'/{c4x}/{prfx}_unlock.png', 1), + (u'', u'/static/{prfx}_lock.png', u'/{c4x}/{prfx}_lock.png', 1), + (u'', u'/static/weird {prfx}_unlock.png', u'/{c4x}/weird_{prfx}_unlock.png', 1), + (u'', u'/static/{prfx}_excluded.html', u'/{c4x}/{prfx}_excluded.html', 1), + (u'', u'/static/{prfx}_not_excluded.htm', u'/{c4x}/{prfx}_not_excluded.htm', 1), + (u'dev', u'/static/{prfx}_unlock.png', u'//dev/{c4x}/{prfx}_unlock.png', 1), + (u'dev', u'/static/{prfx}_lock.png', u'/{c4x}/{prfx}_lock.png', 1), + (u'dev', u'/static/weird {prfx}_unlock.png', u'//dev/{c4x}/weird_{prfx}_unlock.png', 1), + (u'dev', u'/static/{prfx}_excluded.html', u'/{c4x}/{prfx}_excluded.html', 1), + (u'dev', u'/static/{prfx}_not_excluded.htm', u'//dev/{c4x}/{prfx}_not_excluded.htm', 1), # Static path with subdirectory. This ensures we properly substitute slashes. - (u'', u'/static/special/{prefix}_unlock.png', u'/{c4x}/special_{prefix}_unlock.png', 1), - (u'', u'/static/special/{prefix}_lock.png', u'/{c4x}/special_{prefix}_lock.png', 1), - (u'dev', u'/static/special/{prefix}_unlock.png', u'//dev/{c4x}/special_{prefix}_unlock.png', 1), - (u'dev', u'/static/special/{prefix}_lock.png', u'/{c4x}/special_{prefix}_lock.png', 1), + (u'', u'/static/special/{prfx}_unlock.png', u'/{c4x}/special_{prfx}_unlock.png', 1), + (u'', u'/static/special/{prfx}_lock.png', u'/{c4x}/special_{prfx}_lock.png', 1), + (u'', u'/static/special/weird {prfx}_unlock.png', u'/{c4x}/special_weird_{prfx}_unlock.png', 1), + (u'', u'/static/special/{prfx}_excluded.html', u'/{c4x}/special_{prfx}_excluded.html', 1), + (u'', u'/static/special/{prfx}_not_excluded.htm', u'/{c4x}/special_{prfx}_not_excluded.htm', 1), + (u'dev', u'/static/special/{prfx}_unlock.png', u'//dev/{c4x}/special_{prfx}_unlock.png', 1), + (u'dev', u'/static/special/{prfx}_lock.png', u'/{c4x}/special_{prfx}_lock.png', 1), + (u'dev', u'/static/special/weird {prfx}_unlock.png', u'//dev/{c4x}/special_weird_{prfx}_unlock.png', 1), + (u'dev', u'/static/special/{prfx}_excluded.html', u'/{c4x}/special_{prfx}_excluded.html', 1), + (u'dev', u'/static/special/{prfx}_not_excluded.htm', u'//dev/{c4x}/special_{prfx}_not_excluded.htm', 1), # Static path with query parameter. ( u'', - u'/static/{prefix}_unlock.png?foo=/static/{prefix}_lock.png', - u'/{c4x}/{prefix}_unlock.png?foo={encoded_c4x}{prefix}_lock.png', + u'/static/{prfx}_unlock.png?foo=/static/{prfx}_lock.png', + u'/{c4x}/{prfx}_unlock.png?foo={encoded_c4x}{prfx}_lock.png', + 2 + ), + ( + u'', + u'/static/{prfx}_lock.png?foo=/static/{prfx}_unlock.png', + u'/{c4x}/{prfx}_lock.png?foo={encoded_c4x}{prfx}_unlock.png', + 2 + ), + ( + u'', + u'/static/{prfx}_excluded.html?foo=/static/{prfx}_excluded.html', + u'/{c4x}/{prfx}_excluded.html?foo={encoded_c4x}{prfx}_excluded.html', + 2 + ), + ( + u'', + u'/static/{prfx}_excluded.html?foo=/static/{prfx}_not_excluded.htm', + u'/{c4x}/{prfx}_excluded.html?foo={encoded_c4x}{prfx}_not_excluded.htm', 2 ), ( u'', - u'/static/{prefix}_lock.png?foo=/static/{prefix}_unlock.png', - u'/{c4x}/{prefix}_lock.png?foo={encoded_c4x}{prefix}_unlock.png', + u'/static/{prfx}_not_excluded.htm?foo=/static/{prfx}_excluded.html', + u'/{c4x}/{prfx}_not_excluded.htm?foo={encoded_c4x}{prfx}_excluded.html', + 2 + ), + ( + u'', + u'/static/{prfx}_not_excluded.htm?foo=/static/{prfx}_not_excluded.htm', + u'/{c4x}/{prfx}_not_excluded.htm?foo={encoded_c4x}{prfx}_not_excluded.htm', + 2 + ), + ( + u'dev', + u'/static/{prfx}_unlock.png?foo=/static/{prfx}_lock.png', + u'//dev/{c4x}/{prfx}_unlock.png?foo={encoded_c4x}{prfx}_lock.png', + 2 + ), + ( + u'dev', + u'/static/{prfx}_lock.png?foo=/static/{prfx}_unlock.png', + u'/{c4x}/{prfx}_lock.png?foo={encoded_base_url}{encoded_c4x}{prfx}_unlock.png', + 2 + ), + ( + u'dev', + u'/static/{prfx}_excluded.html?foo=/static/{prfx}_excluded.html', + u'/{c4x}/{prfx}_excluded.html?foo={encoded_c4x}{prfx}_excluded.html', + 2 + ), + ( + u'dev', + u'/static/{prfx}_excluded.html?foo=/static/{prfx}_not_excluded.htm', + u'/{c4x}/{prfx}_excluded.html?foo={encoded_base_url}{encoded_c4x}{prfx}_not_excluded.htm', 2 ), ( u'dev', - u'/static/{prefix}_unlock.png?foo=/static/{prefix}_lock.png', - u'//dev/{c4x}/{prefix}_unlock.png?foo={encoded_c4x}{prefix}_lock.png', + u'/static/{prfx}_not_excluded.htm?foo=/static/{prfx}_excluded.html', + u'//dev/{c4x}/{prfx}_not_excluded.htm?foo={encoded_c4x}{prfx}_excluded.html', 2 ), ( u'dev', - u'/static/{prefix}_lock.png?foo=/static/{prefix}_unlock.png', - u'/{c4x}/{prefix}_lock.png?foo={encoded_base_url}{encoded_c4x}{prefix}_unlock.png', + u'/static/{prfx}_not_excluded.htm?foo=/static/{prfx}_not_excluded.htm', + u'//dev/{c4x}/{prfx}_not_excluded.htm?foo={encoded_base_url}{encoded_c4x}{prfx}_not_excluded.htm', 2 ), # Old, c4x-style path. - (u'', u'/{c4x}/{prefix}_unlock.png', u'/{c4x}/{prefix}_unlock.png', 1), - (u'', u'/{c4x}/{prefix}_lock.png', u'/{c4x}/{prefix}_lock.png', 1), - (u'', u'/{c4x}/weird_{prefix}_unlock.png', u'/{c4x}/weird_{prefix}_unlock.png', 1), - (u'dev', u'/{c4x}/{prefix}_unlock.png', u'//dev/{c4x}/{prefix}_unlock.png', 1), - (u'dev', u'/{c4x}/{prefix}_lock.png', u'/{c4x}/{prefix}_lock.png', 1), - (u'dev', u'/{c4x}/weird_{prefix}_unlock.png', u'//dev/{c4x}/weird_{prefix}_unlock.png', 1), + (u'', u'/{c4x}/{prfx}_unlock.png', u'/{c4x}/{prfx}_unlock.png', 1), + (u'', u'/{c4x}/{prfx}_lock.png', u'/{c4x}/{prfx}_lock.png', 1), + (u'', u'/{c4x}/weird_{prfx}_lock.png', u'/{c4x}/weird_{prfx}_lock.png', 1), + (u'', u'/{c4x}/{prfx}_excluded.html', u'/{c4x}/{prfx}_excluded.html', 1), + (u'', u'/{c4x}/{prfx}_not_excluded.htm', u'/{c4x}/{prfx}_not_excluded.htm', 1), + (u'dev', u'/{c4x}/{prfx}_unlock.png', u'//dev/{c4x}/{prfx}_unlock.png', 1), + (u'dev', u'/{c4x}/{prfx}_lock.png', u'/{c4x}/{prfx}_lock.png', 1), + (u'dev', u'/{c4x}/weird_{prfx}_unlock.png', u'//dev/{c4x}/weird_{prfx}_unlock.png', 1), + (u'dev', u'/{c4x}/{prfx}_excluded.html', u'/{c4x}/{prfx}_excluded.html', 1), + (u'dev', u'/{c4x}/{prfx}_not_excluded.htm', u'//dev/{c4x}/{prfx}_not_excluded.htm', 1), ) @ddt.unpack def test_canonical_asset_path_with_c4x_style_assets(self, base_url, start, expected, mongo_calls): @@ -431,18 +636,18 @@ def test_canonical_asset_path_with_c4x_style_assets(self, base_url, start, expec encoded_base_url = quote_plus('//' + base_url) start = start.format( - prefix=prefix, + prfx=prefix, encoded_base_url=encoded_base_url, c4x=c4x_block, encoded_c4x=encoded_c4x_block ) expected = expected.format( - prefix=prefix, + prfx=prefix, encoded_base_url=encoded_base_url, c4x=c4x_block, encoded_c4x=encoded_c4x_block ) with check_mongo_calls(mongo_calls): - asset_path = StaticContent.get_canonicalized_asset_path(self.courses[prefix].id, start, base_url) + asset_path = StaticContent.get_canonicalized_asset_path(self.courses[prefix].id, start, base_url, ['html']) self.assertEqual(asset_path, expected) diff --git a/common/lib/xmodule/xmodule/contentstore/content.py b/common/lib/xmodule/xmodule/contentstore/content.py index ddf92eea35a3..89a269a5d049 100644 --- a/common/lib/xmodule/xmodule/contentstore/content.py +++ b/common/lib/xmodule/xmodule/contentstore/content.py @@ -168,7 +168,7 @@ def get_asset_key_from_path(course_key, path): return StaticContent.compute_location(course_key, path) @staticmethod - def get_canonicalized_asset_path(course_key, path, base_url): + def get_canonicalized_asset_path(course_key, path, base_url, excluded_exts): """ Returns a fully-qualified path to a piece of static content. @@ -199,17 +199,24 @@ def get_canonicalized_asset_path(course_key, path, base_url): # If we can't find the item, just treat it as if it's locked. serve_from_cdn = False + # See if this is an allowed file extension to serve. Some files aren't served through the + # CDN in order to avoid same-origin policy/CORS-related issues. + for excluded_ext in excluded_exts: + if relative_path.lower().endswith(excluded_ext.lower()): + serve_from_cdn = False + break + # Update any query parameter values that have asset paths in them. This is for assets that # require their own after-the-fact values, like a Flash file that needs the path of a config # file passed to it e.g. /static/visualization.swf?configFile=/static/visualization.xml query_params = parse_qsl(query_string) updated_query_params = [] - for query_name, query_value in query_params: - if query_value.startswith("/static/"): - new_query_value = StaticContent.get_canonicalized_asset_path(course_key, query_value, base_url) - updated_query_params.append((query_name, new_query_value)) + for query_name, query_val in query_params: + if query_val.startswith("/static/"): + new_val = StaticContent.get_canonicalized_asset_path(course_key, query_val, base_url, excluded_exts) + updated_query_params.append((query_name, new_val)) else: - updated_query_params.append((query_name, query_value)) + updated_query_params.append((query_name, query_val)) serialized_asset_key = StaticContent.serialize_asset_key_with_slash(asset_key) base_url = base_url if serve_from_cdn else '' From e57d1a200606321d83bdb0673324f6647101dd38 Mon Sep 17 00:00:00 2001 From: Toby Lawrence Date: Tue, 2 Feb 2016 10:07:13 -0500 Subject: [PATCH 2/2] Add leading period so we don't get partial matches on extensions. --- common/djangoapps/static_replace/models.py | 3 ++- .../djangoapps/static_replace/test/test_static_replace.py | 6 ++++-- common/lib/xmodule/xmodule/contentstore/content.py | 6 ++---- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/common/djangoapps/static_replace/models.py b/common/djangoapps/static_replace/models.py index 8c73889c54ba..514adbbe778d 100644 --- a/common/djangoapps/static_replace/models.py +++ b/common/djangoapps/static_replace/models.py @@ -44,7 +44,8 @@ class Meta(object): @classmethod def get_excluded_extensions(cls): """Gets the excluded file extensions when canonicalizing static asset paths""" - return cls.current().excluded_extensions.split() + add_period = lambda x: '.' + x + return map(add_period, cls.current().excluded_extensions.split()) def __repr__(self): return ''.format(self.get_excluded_extensions().split()) diff --git a/common/djangoapps/static_replace/test/test_static_replace.py b/common/djangoapps/static_replace/test/test_static_replace.py index 37b2fc48db16..bc203a85bceb 100644 --- a/common/djangoapps/static_replace/test/test_static_replace.py +++ b/common/djangoapps/static_replace/test/test_static_replace.py @@ -445,6 +445,7 @@ def create_arbitrary_content(cls, prefix, name, locked=False): ) @ddt.unpack def test_canonical_asset_path_with_new_style_assets(self, base_url, start, expected, mongo_calls): + exts = ['.html', '.tm'] prefix = 'split' encoded_base_url = quote_plus('//' + base_url) c4x = 'c4x/a/b/asset' @@ -473,7 +474,7 @@ def test_canonical_asset_path_with_new_style_assets(self, base_url, start, expec ) with check_mongo_calls(mongo_calls): - asset_path = StaticContent.get_canonicalized_asset_path(self.courses[prefix].id, start, base_url, ['html']) + asset_path = StaticContent.get_canonicalized_asset_path(self.courses[prefix].id, start, base_url, exts) self.assertEqual(asset_path, expected) @ddt.data( @@ -630,6 +631,7 @@ def test_canonical_asset_path_with_new_style_assets(self, base_url, start, expec ) @ddt.unpack def test_canonical_asset_path_with_c4x_style_assets(self, base_url, start, expected, mongo_calls): + exts = ['.html', '.tm'] prefix = 'old' c4x_block = 'c4x/a/b/asset' encoded_c4x_block = quote_plus('/' + c4x_block + '/') @@ -649,5 +651,5 @@ def test_canonical_asset_path_with_c4x_style_assets(self, base_url, start, expec ) with check_mongo_calls(mongo_calls): - asset_path = StaticContent.get_canonicalized_asset_path(self.courses[prefix].id, start, base_url, ['html']) + asset_path = StaticContent.get_canonicalized_asset_path(self.courses[prefix].id, start, base_url, exts) self.assertEqual(asset_path, expected) diff --git a/common/lib/xmodule/xmodule/contentstore/content.py b/common/lib/xmodule/xmodule/contentstore/content.py index 89a269a5d049..b6f11e0979cb 100644 --- a/common/lib/xmodule/xmodule/contentstore/content.py +++ b/common/lib/xmodule/xmodule/contentstore/content.py @@ -201,10 +201,8 @@ def get_canonicalized_asset_path(course_key, path, base_url, excluded_exts): # See if this is an allowed file extension to serve. Some files aren't served through the # CDN in order to avoid same-origin policy/CORS-related issues. - for excluded_ext in excluded_exts: - if relative_path.lower().endswith(excluded_ext.lower()): - serve_from_cdn = False - break + if any(relative_path.lower().endswith(excluded_ext.lower()) for excluded_ext in excluded_exts): + serve_from_cdn = False # Update any query parameter values that have asset paths in them. This is for assets that # require their own after-the-fact values, like a Flash file that needs the path of a config