Opened 10 years ago
Closed 9 years ago
#22972 closed Bug (duplicate)
HashedFilesMixin.patterns should limit URL matches to their respective filetypes
Reported by: | Owned by: | aehlke | |
---|---|---|---|
Component: | contrib.staticfiles | Version: | dev |
Severity: | Normal | Keywords: | HashedFilesMixin CachedFilesMixin staticfiles |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | no |
Description
HashedFilesMixin
contains a patterns
property which maps file extensions to regex patterns. This works great for the default case that Django ships with "*.css"
as the only filetype, but results in surprising behavior once extended with multiple filetypes. I would expect the regex patterns to only apply to the filetype they're categorized under, but once I added "*.js"
to patterns
(via subclassing), the CSS rules also applied to my JS files.
My use-case of extending this is to add URL rewriting to JS source map references, e.g. //# sourceMappingURL=foo.js.map
which can appear at the end of JS files. I need to be able to rewrite these URLs in the same way that Django's staticfiles can rewrite URLs in CSS, for e.g. adding hashes to filenames.
Once I tried extending patterns
with this:
patterns = HashedFilesMixin.patterns + ( ("*.js", ( (r"""(//# sourceMappingURL=(\s*))""", """//# sourceMappingURL=%s"""), )), )
The surprising and broken behavior was that Django tried to rewrite "URLs" for this JS (from Backbone.js): this.loadUrl(window.location.hash)
, which matched the CSS pattern for rewriting url(foo)
because the regex patterns are case-insensitive.
This also applies to the previous CachedFilesMixin
from before 1.7.
Please let me know if a fix for this would be accepted, and I'll put together a test case and patch.
Change History (14)
comment:1 by , 10 years ago
Summary: | HashedFilesMixin → HashedFilesMixin.patterns should limit URL matches to their respective filetypes |
---|
comment:2 by , 10 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:3 by , 10 years ago
comment:4 by , 10 years ago
I've submitted a pull request with test coverage (a test which fails before applying the patch) and patch for the fix. Please let me know if anything needs changing.
comment:5 by , 10 years ago
Has patch: | set |
---|
comment:6 by , 10 years ago
Type: | Uncategorized → Bug |
---|
comment:7 by , 10 years ago
Version: | 1.7-rc-1 → master |
---|
comment:8 by , 10 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
comment:9 by , 10 years ago
This patterns property is also documented in the Django docs, which I think adds priority to getting this fixed as it makes it sound like something you can extend (where in current versions you cannot actually extend without subtly broken behavior.)
comment:10 by , 10 years ago
Resolution: | fixed |
---|---|
Status: | closed → new |
A ticket shouldn't be closed until the fix is committed to Django.
comment:12 by , 10 years ago
Patch needs improvement: | set |
---|---|
Triage Stage: | Unreviewed → Accepted |
I left comments for improvement on the PR. Please uncheck "Patch needs improvement" when you update it, thanks.
comment:14 by , 9 years ago
Resolution: | → duplicate |
---|---|
Status: | new → closed |
I'm preparing a test case and patch.