From 36be97b99d4de786746a8d80bbcd41de03752df9 Mon Sep 17 00:00:00 2001 From: James Bligh Date: Wed, 4 Mar 2026 14:12:06 +0000 Subject: [PATCH] Fixed #21080 -- Ignored urls inside comments during collectstatic. Thanks Mariusz Felisiak for the review. Co-authored-by: Nathan Gaberel --- django/contrib/staticfiles/storage.py | 35 +++++++++++++++++-- docs/ref/contrib/staticfiles.txt | 6 ---- .../project/documents/cached/css/ignored.css | 21 +++++++++++ .../project/documents/cached/module.js | 11 ++++++ tests/staticfiles_tests/test_storage.py | 31 ++++++++++++++-- 5 files changed, 93 insertions(+), 11 deletions(-) diff --git a/django/contrib/staticfiles/storage.py b/django/contrib/staticfiles/storage.py index b16c77757cd6..c889bcb4a495 100644 --- a/django/contrib/staticfiles/storage.py +++ b/django/contrib/staticfiles/storage.py @@ -11,6 +11,12 @@ from django.core.files.base import ContentFile from django.core.files.storage import FileSystemStorage, storages from django.utils.functional import LazyObject +from django.utils.regex_helper import _lazy_re_compile + +comment_re = _lazy_re_compile(r"\/\*[^*]*\*+([^/*][^*]*\*+)*\/", re.DOTALL) +line_comment_re = _lazy_re_compile( + r"\/\*[^*]*\*+([^/*][^*]*\*+)*\/|\/\/[^\n]*", re.DOTALL +) class StaticFilesStorage(FileSystemStorage): @@ -204,7 +210,22 @@ def url(self, name, force=False): """ return self._url(self.stored_name, name, force) - def url_converter(self, name, hashed_files, template=None): + def get_comment_blocks(self, content, include_line_comments=False): + """ + Return a list of (start, end) tuples for each comment block. + """ + pattern = line_comment_re if include_line_comments else comment_re + return [(match.start(), match.end()) for match in re.finditer(pattern, content)] + + def is_in_comment(self, pos, comments): + for start, end in comments: + if start < pos and pos < end: + return True + if pos < start: + return False + return False + + def url_converter(self, name, hashed_files, template=None, comment_blocks=None): """ Return the custom URL converter for the given file name. """ @@ -222,6 +243,10 @@ def converter(matchobj): matched = matches["matched"] url = matches["url"] + # Ignore URLs in comments. + if comment_blocks and self.is_in_comment(matchobj.start(), comment_blocks): + return matched + # Ignore absolute/protocol-relative and data-uri URLs. if re.match(r"^[a-z]+:", url) or url.startswith("//"): return matched @@ -375,7 +400,13 @@ def path_level(name): if matches_patterns(path, (extension,)): for pattern, template in patterns: converter = self.url_converter( - name, hashed_files, template + name, + hashed_files, + template, + self.get_comment_blocks( + content, + include_line_comments=path.endswith(".js"), + ), ) try: content = pattern.sub(converter, content) diff --git a/docs/ref/contrib/staticfiles.txt b/docs/ref/contrib/staticfiles.txt index f5f078a2390f..8bb226696694 100644 --- a/docs/ref/contrib/staticfiles.txt +++ b/docs/ref/contrib/staticfiles.txt @@ -349,12 +349,6 @@ argument. For example:: manifest_storage = StaticFilesStorage(location=settings.BASE_DIR) super().__init__(*args, manifest_storage=manifest_storage, **kwargs) -.. admonition:: References in comments - - ``ManifestStaticFilesStorage`` doesn't ignore paths in statements that are - commented out. This :ticket:`may crash on the nonexistent paths <21080>`. - You should check and eventually strip comments. - .. attribute:: storage.ManifestStaticFilesStorage.manifest_hash This attribute provides a single hash that changes whenever a file in the diff --git a/tests/staticfiles_tests/project/documents/cached/css/ignored.css b/tests/staticfiles_tests/project/documents/cached/css/ignored.css index 70a8cb918a5d..c6c004e911db 100644 --- a/tests/staticfiles_tests/project/documents/cached/css/ignored.css +++ b/tests/staticfiles_tests/project/documents/cached/css/ignored.css @@ -8,3 +8,24 @@ body { background: url(); } +/* @import url("non_exist.css") */ + +/* url("non_exist.png") */ + +/* + +@import url("non_exist.css") + +url("non_exist.png") + +@import url(other.css) + +*/ + +body { + background: #d3d6d8 /*url("does.not.exist.png")*/ url(/static/cached/img/relative.png); +} + +body { + background: #d3d6d8 /* url("does.not.exist.png") */ url(/static/cached/img/relative.png) /*url("does.not.exist.either.png")*/; +} diff --git a/tests/staticfiles_tests/project/documents/cached/module.js b/tests/staticfiles_tests/project/documents/cached/module.js index c56530aea6d2..e7e1419c5a6d 100644 --- a/tests/staticfiles_tests/project/documents/cached/module.js +++ b/tests/staticfiles_tests/project/documents/cached/module.js @@ -24,3 +24,14 @@ export { firstVar as firstVarAlias, secondVar as secondVarAlias } from "./module_test.js"; + +// ignore block comments +/* export * from "./module_test_missing.js"; */ +/* +import rootConst from "/static/absolute_root_missing.js"; +const dynamicModule = import("./module_test_missing.js"); +*/ + +// ignore line comments +// import testConst from "./module_test_missing.js"; +// const dynamicModule = import("./module_test_missing.js"); diff --git a/tests/staticfiles_tests/test_storage.py b/tests/staticfiles_tests/test_storage.py index e09f9eda1c90..cdb6fd3c7e2e 100644 --- a/tests/staticfiles_tests/test_storage.py +++ b/tests/staticfiles_tests/test_storage.py @@ -65,7 +65,7 @@ def test_template_tag_simple_content(self): def test_path_ignored_completely(self): relpath = self.hashed_file_path("cached/css/ignored.css") - self.assertEqual(relpath, "cached/css/ignored.55e7c226dda1.css") + self.assertEqual(relpath, "cached/css/ignored.0e15ac4a4fb4.css") with storage.staticfiles_storage.open(relpath) as relfile: content = relfile.read() self.assertIn(b"#foobar", content) @@ -75,6 +75,22 @@ def test_path_ignored_completely(self): self.assertIn(b"chrome:foobar", content) self.assertIn(b"//foobar", content) self.assertIn(b"url()", content) + self.assertIn(b'/* @import url("non_exist.css") */', content) + self.assertIn(b'/* url("non_exist.png") */', content) + self.assertIn(b'@import url("non_exist.css")', content) + self.assertIn(b'url("non_exist.png")', content) + self.assertIn(b"@import url(other.css)", content) + self.assertIn( + b'background: #d3d6d8 /*url("does.not.exist.png")*/ ' + b'url("/static/cached/img/relative.acae32e4532b.png");', + content, + ) + self.assertIn( + b'background: #d3d6d8 /* url("does.not.exist.png") */ ' + b'url("/static/cached/img/relative.acae32e4532b.png") ' + b'/*url("does.not.exist.either.png")*/', + content, + ) self.assertPostCondition() def test_path_with_querystring(self): @@ -698,7 +714,7 @@ class TestCollectionJSModuleImportAggregationManifestStorage(CollectionTestCase) def test_module_import(self): relpath = self.hashed_file_path("cached/module.js") - self.assertEqual(relpath, "cached/module.4326210cf0bd.js") + self.assertEqual(relpath, "cached/module.eaa407b94311.js") tests = [ # Relative imports. b'import testConst from "./module_test.477bbebe77f0.js";', @@ -721,6 +737,15 @@ def test_module_import(self): b" firstVar1 as firstVarAlias,\n" b" $second_var_2 as secondVarAlias\n" b'} from "./module_test.477bbebe77f0.js";', + # Ignore block comments + b'/* export * from "./module_test_missing.js"; */', + b"/*\n" + b'import rootConst from "/static/absolute_root_missing.js";\n' + b'const dynamicModule = import("./module_test_missing.js");\n' + b"*/", + # Ignore line comments + b'// import testConst from "./module_test_missing.js";', + b'// const dynamicModule = import("./module_test_missing.js");', ] with storage.staticfiles_storage.open(relpath) as relfile: content = relfile.read() @@ -730,7 +755,7 @@ def test_module_import(self): def test_aggregating_modules(self): relpath = self.hashed_file_path("cached/module.js") - self.assertEqual(relpath, "cached/module.4326210cf0bd.js") + self.assertEqual(relpath, "cached/module.eaa407b94311.js") tests = [ b'export * from "./module_test.477bbebe77f0.js";', b'export { testConst } from "./module_test.477bbebe77f0.js";',