Opened 11 years ago
Closed 10 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 , 11 years ago
| Summary: | HashedFilesMixin → HashedFilesMixin.patterns should limit URL matches to their respective filetypes |
|---|
comment:2 by , 11 years ago
| Owner: | changed from to |
|---|---|
| Status: | new → assigned |
comment:3 by , 11 years ago
comment:4 by , 11 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 , 11 years ago
| Has patch: | set |
|---|
comment:6 by , 11 years ago
| Type: | Uncategorized → Bug |
|---|
comment:7 by , 11 years ago
| Version: | 1.7-rc-1 → master |
|---|
comment:8 by , 11 years ago
| Resolution: | → fixed |
|---|---|
| Status: | assigned → closed |
comment:9 by , 11 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 , 11 years ago
| Resolution: | fixed |
|---|---|
| Status: | closed → new |
A ticket shouldn't be closed until the fix is committed to Django.
comment:12 by , 11 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 , 10 years ago
| Resolution: | → duplicate |
|---|---|
| Status: | new → closed |
I'm preparing a test case and patch.