From a82488bf4b57c1bc917f8ed79eac7a99d5e26bff Mon Sep 17 00:00:00 2001 From: Jannis Leidel Date: Thu, 3 Nov 2011 10:51:02 +0000 Subject: [PATCH] Fixed #16966 -- Stopped CachedStaticFilesStorage from choking on querystrings and path fragments. git-svn-id: http://code.djangoproject.com/svn/django/trunk@17068 bcc190cf-cafb-0310-a4f2-bffc1f526a37 --- django/contrib/staticfiles/storage.py | 23 ++-- .../project/documents/cached/relative.css | 1 + .../staticfiles_tests/tests.py | 105 ++++++++++-------- 3 files changed, 77 insertions(+), 52 deletions(-) diff --git a/django/contrib/staticfiles/storage.py b/django/contrib/staticfiles/storage.py index ecb6769973..8353889e82 100644 --- a/django/contrib/staticfiles/storage.py +++ b/django/contrib/staticfiles/storage.py @@ -3,6 +3,8 @@ import hashlib import os import posixpath import re +from urllib import unquote +from urlparse import urlsplit, urlunsplit from django.conf import settings from django.core.cache import (get_cache, InvalidCacheBackendError, @@ -64,23 +66,28 @@ class CachedFilesMixin(object): self._patterns.setdefault(extension, []).append(compiled) def hashed_name(self, name, content=None): + parsed_name = urlsplit(unquote(name)) + clean_name = parsed_name.path if content is None: - if not self.exists(name): + if not self.exists(clean_name): raise ValueError("The file '%s' could not be found with %r." % - (name, self)) + (clean_name, self)) try: - content = self.open(name) + content = self.open(clean_name) except IOError: # Handle directory paths return name - path, filename = os.path.split(name) + path, filename = os.path.split(clean_name) root, ext = os.path.splitext(filename) # Get the MD5 hash of the file md5 = hashlib.md5() for chunk in content.chunks(): md5.update(chunk) md5sum = md5.hexdigest()[:12] - return os.path.join(path, u"%s.%s%s" % (root, md5sum, ext)) + hashed_name = os.path.join(path, u"%s.%s%s" % (root, md5sum, ext)) + unparsed_name = list(parsed_name) + unparsed_name[2] = hashed_name + return urlunsplit(unparsed_name) def cache_key(self, name): return u'staticfiles:cache:%s' % name @@ -98,7 +105,7 @@ class CachedFilesMixin(object): hashed_name = self.hashed_name(name) # set the cache if there was a miss (e.g. if cache server goes down) self.cache.set(cache_key, hashed_name) - return super(CachedFilesMixin, self).url(hashed_name) + return unquote(super(CachedFilesMixin, self).url(hashed_name)) def url_converter(self, name): """ @@ -132,9 +139,9 @@ class CachedFilesMixin(object): else: start, end = 1, sub_level - 1 joined_result = '/'.join(name_parts[:-start] + url_parts[end:]) - hashed_url = self.url(joined_result, force=True) + hashed_url = self.url(unquote(joined_result), force=True) # Return the hashed and normalized version to the file - return 'url("%s")' % hashed_url + return 'url("%s")' % unquote(hashed_url) return converter def post_process(self, paths, dry_run=False, **options): diff --git a/tests/regressiontests/staticfiles_tests/project/documents/cached/relative.css b/tests/regressiontests/staticfiles_tests/project/documents/cached/relative.css index afa658dbeb..95d1f4abb8 100644 --- a/tests/regressiontests/staticfiles_tests/project/documents/cached/relative.css +++ b/tests/regressiontests/staticfiles_tests/project/documents/cached/relative.css @@ -1,5 +1,6 @@ @import url("../cached/styles.css"); @import url("absolute.css"); +@import url("absolute.css#eggs"); body { background: #d3d6d8 url(img/relative.png); } \ No newline at end of file diff --git a/tests/regressiontests/staticfiles_tests/tests.py b/tests/regressiontests/staticfiles_tests/tests.py index 67977e70d1..652ddbd2b2 100644 --- a/tests/regressiontests/staticfiles_tests/tests.py +++ b/tests/regressiontests/staticfiles_tests/tests.py @@ -61,7 +61,7 @@ class BaseStaticFilesTestCase(object): self.addCleanup(os.unlink, _nonascii_filepath) def assertFileContains(self, filepath, text): - self.assertTrue(text in self._get_file(smart_unicode(filepath)), + self.assertIn(text, self._get_file(smart_unicode(filepath)), u"'%s' not in '%s'" % (text, filepath)) def assertFileNotFound(self, filepath): @@ -72,11 +72,15 @@ class BaseStaticFilesTestCase(object): template = loader.get_template_from_string(template) return template.render(Context(kwargs)).strip() - def assertTemplateRenders(self, template, result, **kwargs): + def static_template_snippet(self, path): + return "{%% load static from staticfiles %%}{%% static '%s' %%}" % path + + def assertStaticRenders(self, path, result, **kwargs): + template = self.static_template_snippet(path) self.assertEqual(self.render_template(template, **kwargs), result) - def assertTemplateRaises(self, exc, template, result, **kwargs): - self.assertRaises(exc, self.assertTemplateRenders, template, result, **kwargs) + def assertStaticRaises(self, exc, path, result, **kwargs): + self.assertRaises(exc, self.assertStaticRenders, path, result, **kwargs) class StaticFilesTestCase(BaseStaticFilesTestCase, TestCase): @@ -99,8 +103,8 @@ class BaseCollectionTestCase(BaseStaticFilesTestCase): settings.STATIC_ROOT = tempfile.mkdtemp() self.run_collectstatic() # Use our own error handler that can handle .svn dirs on Windows - self.addCleanup(shutil.rmtree, settings.STATIC_ROOT, - ignore_errors=True, onerror=rmtree_errorhandler) + #self.addCleanup(shutil.rmtree, settings.STATIC_ROOT, + # ignore_errors=True, onerror=rmtree_errorhandler) def tearDown(self): settings.STATIC_ROOT = self.old_root @@ -194,8 +198,8 @@ class TestFindStatic(CollectionTestCase, TestDefaults): finally: sys.stdout = _stdout self.assertEqual(len(lines), 3) # three because there is also the "Found here" line - self.assertTrue('project' in lines[1]) - self.assertTrue('apps' in lines[2]) + self.assertIn('project', lines[1]) + self.assertIn('apps', lines[2]) class TestCollection(CollectionTestCase, TestDefaults): @@ -284,73 +288,90 @@ class TestCollectionCachedStorage(BaseCollectionTestCase, """ Tests for the Cache busting storage """ - def cached_file_path(self, relpath): - template = "{%% load static from staticfiles %%}{%% static '%s' %%}" - fullpath = self.render_template(template % relpath) + def cached_file_path(self, path): + fullpath = self.render_template(self.static_template_snippet(path)) return fullpath.replace(settings.STATIC_URL, '') def test_template_tag_return(self): """ Test the CachedStaticFilesStorage backend. """ - self.assertTemplateRaises(ValueError, """ - {% load static from staticfiles %}{% static "does/not/exist.png" %} - """, "/static/does/not/exist.png") - self.assertTemplateRenders(""" - {% load static from staticfiles %}{% static "test/file.txt" %} - """, "/static/test/file.dad0999e4f8f.txt") - self.assertTemplateRenders(""" - {% load static from staticfiles %}{% static "cached/styles.css" %} - """, "/static/cached/styles.93b1147e8552.css") + self.assertStaticRaises(ValueError, + "does/not/exist.png", + "/static/does/not/exist.png") + self.assertStaticRenders("test/file.txt", + "/static/test/file.dad0999e4f8f.txt") + self.assertStaticRenders("cached/styles.css", + "/static/cached/styles.93b1147e8552.css") def test_template_tag_simple_content(self): relpath = self.cached_file_path("cached/styles.css") self.assertEqual(relpath, "cached/styles.93b1147e8552.css") with storage.staticfiles_storage.open(relpath) as relfile: content = relfile.read() - self.assertFalse("cached/other.css" in content, content) - self.assertTrue("/static/cached/other.d41d8cd98f00.css" in content) + self.assertNotIn("cached/other.css", content) + self.assertIn("/static/cached/other.d41d8cd98f00.css", content) + + def test_path_with_querystring(self): + relpath = self.cached_file_path("cached/styles.css?spam=eggs") + self.assertEqual(relpath, + "cached/styles.93b1147e8552.css?spam=eggs") + with storage.staticfiles_storage.open( + "cached/styles.93b1147e8552.css") as relfile: + content = relfile.read() + self.assertNotIn("cached/other.css", content) + self.assertIn("/static/cached/other.d41d8cd98f00.css", content) + + def test_path_with_fragment(self): + relpath = self.cached_file_path("cached/styles.css#eggs") + self.assertEqual(relpath, "cached/styles.93b1147e8552.css#eggs") + with storage.staticfiles_storage.open( + "cached/styles.93b1147e8552.css") as relfile: + content = relfile.read() + self.assertNotIn("cached/other.css", content) + self.assertIn("/static/cached/other.d41d8cd98f00.css", content) def test_template_tag_absolute(self): relpath = self.cached_file_path("cached/absolute.css") self.assertEqual(relpath, "cached/absolute.cc80cb5e2eb1.css") with storage.staticfiles_storage.open(relpath) as relfile: content = relfile.read() - self.assertFalse("/static/cached/styles.css" in content) - self.assertTrue("/static/cached/styles.93b1147e8552.css" in content) + self.assertNotIn("/static/cached/styles.css", content) + self.assertIn("/static/cached/styles.93b1147e8552.css", content) def test_template_tag_denorm(self): relpath = self.cached_file_path("cached/denorm.css") self.assertEqual(relpath, "cached/denorm.363de96e9b4b.css") with storage.staticfiles_storage.open(relpath) as relfile: content = relfile.read() - self.assertFalse("..//cached///styles.css" in content) - self.assertTrue("/static/cached/styles.93b1147e8552.css" in content) + self.assertNotIn("..//cached///styles.css", content) + self.assertIn("/static/cached/styles.93b1147e8552.css", content) def test_template_tag_relative(self): relpath = self.cached_file_path("cached/relative.css") - self.assertEqual(relpath, "cached/relative.8dffb45d91f5.css") + self.assertEqual(relpath, "cached/relative.2217ea7273c2.css") with storage.staticfiles_storage.open(relpath) as relfile: content = relfile.read() - self.assertFalse("../cached/styles.css" in content) - self.assertFalse('@import "styles.css"' in content) - self.assertTrue("/static/cached/styles.93b1147e8552.css" in content) - self.assertFalse("url(img/relative.png)" in content) - self.assertTrue("/static/cached/img/relative.acae32e4532b.png" in content) + self.assertNotIn("../cached/styles.css", content) + self.assertNotIn('@import "styles.css"', content) + self.assertIn("/static/cached/styles.93b1147e8552.css", content) + self.assertNotIn("url(img/relative.png)", content) + self.assertIn("/static/cached/img/relative.acae32e4532b.png", content) + self.assertIn("/static/cached/absolute.cc80cb5e2eb1.css#eggs", content) def test_template_tag_deep_relative(self): relpath = self.cached_file_path("cached/css/window.css") self.assertEqual(relpath, "cached/css/window.9db38d5169f3.css") with storage.staticfiles_storage.open(relpath) as relfile: content = relfile.read() - self.assertFalse('url(img/window.png)' in content) - self.assertTrue('url("/static/cached/css/img/window.acae32e4532b.png")' in content) + self.assertNotIn('url(img/window.png)', content) + self.assertIn('url("/static/cached/css/img/window.acae32e4532b.png")', content) def test_template_tag_url(self): relpath = self.cached_file_path("cached/url.css") self.assertEqual(relpath, "cached/url.615e21601e4b.css") with storage.staticfiles_storage.open(relpath) as relfile: - self.assertTrue("https://" in relfile.read()) + self.assertIn("https://", relfile.read()) def test_cache_invalidation(self): name = "cached/styles.css" @@ -367,7 +388,6 @@ class TestCollectionCachedStorage(BaseCollectionTestCase, cached_name = storage.staticfiles_storage.cache.get(cache_key) self.assertEqual(cached_name, hashed_name) - # we set DEBUG to False here since the template tag wouldn't work otherwise TestCollectionCachedStorage = override_settings(**dict(TEST_SETTINGS, STATICFILES_STORAGE='django.contrib.staticfiles.storage.CachedStaticFilesStorage', @@ -517,9 +537,9 @@ class TestMiscFinder(TestCase): default_storage._wrapped = empty def test_get_finder(self): - self.assertTrue(isinstance(finders.get_finder( + self.assertIsInstance(finders.get_finder( 'django.contrib.staticfiles.finders.FileSystemFinder'), - finders.FileSystemFinder)) + finders.FileSystemFinder) def test_get_finder_bad_classname(self): self.assertRaises(ImproperlyConfigured, finders.get_finder, @@ -545,9 +565,6 @@ class TestMiscFinder(TestCase): class TestTemplateTag(StaticFilesTestCase): def test_template_tag(self): - self.assertTemplateRenders(""" - {% load static from staticfiles %}{% static "does/not/exist.png" %} - """, "/static/does/not/exist.png") - self.assertTemplateRenders(""" - {% load static from staticfiles %}{% static "testfile.txt" %} - """, "/static/testfile.txt") + self.assertStaticRenders("does/not/exist.png", + "/static/does/not/exist.png") + self.assertStaticRenders("testfile.txt", "/static/testfile.txt")