Opened 9 years ago
Last modified 21 months ago
#21080 assigned Bug
collectstatic post-processing fails for references inside comments
Reported by: | Owned by: | Tomáš Zigo | |
---|---|---|---|
Component: | contrib.staticfiles | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Joshua Smith | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | yes | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | no |
Description
"python manage.py collectstatic" is attempting to parse references inside css comments and generating errors during post-processing. I am using:
STATICFILES_STORAGE = 'django.contrib.staticfiles.storage.CachedStaticFilesStorage'
eg.
The following snippet of code in test.css:
.gfg-collapse-closed { /* background-image : url('arrow_close.gif'); */ }
produces the following error:
ValueError: The file 'stylesheets/arrow_close.gif' could not be found with <django.contrib.staticfiles.storage.CachedStaticFilesStorage object at 0x1078a3910>. collectstatic
Ideally, collectstatic should respect CSS comments and should not attempt to parse/reference files in lines that are commented out.
If the fix is too complex, a simple workaround might be to include a --ignore-error
flag that would allow the application to continue post-processing even when it sees errors
Change History (19)
comment:1 Changed 9 years ago by
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 Changed 9 years ago by
Could you recommend a path forward? Can we at least have it skip failed imports with a warning rather than break?
-Shreyas
comment:3 Changed 9 years ago by
I'm agree with the comment of @marfire, the correct way to do this is with a CSS parser, because there are many cases to be treated (multi-line comments, nested comments, broken comments, etc.)
Adding an option to skip failed imports was reported in ticket:19650 and it was marked as a duplicated of ticket:18958.
If you really need ignore these errors, you can create a subclass of CachedFilesMixin
on your own project and add a try...except
statement to handle these kind of exceptions.
comment:4 Changed 9 years ago by
This bit me today. "Dumb" workaround: replace that url
in your comments with u r l
.
comment:5 Changed 9 years ago by
Just as an additional datapoint and another thing to consider if it comes to parsing the CSS, I recently started working on a project where the designers used some CSS expressions for an IE-specific stylesheet. The JScript used in one expression puts a particularly crazy bit like .replace('url("','').replace('")','')
. As marfire said, that url
bit in a string raises an exception. Fortunately I can overcome this for my project with a simple sublcass to make a very small change to the regular expression used. But that is one other thing to consider if it is decided that the CachedStaticFilesStorage
needs to be more robust.
comment:6 Changed 5 years ago by
hi, apparently this issue is still open in django 1.11.
my deploy to heroku failed with the following error log:
... $ python manage.py collectstatic --noinput Post-processing 'jquery-ui-dist/jquery-ui.theme.css' failed! Traceback (most recent call last): File "manage.py", line 10, in <module> execute_from_command_line(sys.argv) File "/app/.heroku/python/lib/python3.6/site-packages/django/core/management/__init__.py", line 364, in execute_from_command_line utility.execute() File "/app/.heroku/python/lib/python3.6/site-packages/django/core/management/__init__.py", line 356, in execute self.fetch_command(subcommand).run_from_argv(self.argv) File "/app/.heroku/python/lib/python3.6/site-packages/django/core/management/base.py", line 283, in run_from_argv self.execute(*args, **cmd_options) File "/app/.heroku/python/lib/python3.6/site-packages/django/core/management/base.py", line 330, in execute output = self.handle(*args, **options) File "/app/.heroku/python/lib/python3.6/site-packages/django/contrib/staticfiles/management/commands/collectstatic.py", line 199, in handle collected = self.collect() File "/app/.heroku/python/lib/python3.6/site-packages/django/contrib/staticfiles/management/commands/collectstatic.py", line 145, in collect raise processed whitenoise.storage.MissingFileError: The file 'jquery-ui-dist/"images/ui-icons_555555_256x240.png"' could not be found with <whitenoise.storage.CompressedManifestStaticFilesStorage object at 0x7ff880f551d0>. The CSS file 'jquery-ui-dist/jquery-ui.theme.css' references a file which could not be found: jquery-ui-dist/"images/ui-icons_555555_256x240.png" Please check the URL references in this CSS file, particularly any relative paths which might be pointing to the wrong location. ! Error while running '$ python manage.py collectstatic --noinput'. See traceback above for details. You may need to update application code to resolve this error. Or, you can disable collectstatic for this application: $ heroku config:set DISABLE_COLLECTSTATIC=1 https://devcenter.heroku.com/articles/django-assets ! Push rejected, failed to compile Python app. ! Push failed
the "collectstatic" command fails here:
... The file 'jquery-ui-dist/"images/ui-icons_555555_256x240.png"' could not be found with ...
because it tries to parse js comments.
The file path should be
- jquery-ui-dist/images/ui-icons_555555_256x240.png
and not
- jquery-ui-dist/"images/ui-icons_555555_256x240.png"
other people on stackoverflow run inmto this issue as well:
- https://stackoverflow.com/questions/41772144/bad-filename-formed-by-collectstatic-from-jquery-ui-1-12-0-min-css
- https://stackoverflow.com/questions/40010040/django-pipeline-throwing-valueerror-the-file-could-not-be-found
would be great if this issue could be fixed. thanks ;)
comment:7 Changed 5 years ago by
Version: | 1.5 → 1.11 |
---|
comment:8 Changed 5 years ago by
Cc: | Joshua Smith added |
---|---|
Version: | 1.11 → 2.0 |
comment:9 Changed 5 years ago by
Version: | 2.0 → master |
---|
comment:10 follow-up: 12 Changed 5 years ago by
Re: comment:6 is a slightly different issue and MUCH easier to patch.
This issue is caused by generators (like http://jqueryui.com/themeroller) that urlencode stuff in a comment block.
E.g.
/*! jQuery UI - v1.12.1 - 2016-09-14 * http://jqueryui.com * Includes: core.css, accordion.css, ... * To view and modify this theme, visit http://jqueryui.com/themeroller/?...&iconsHover=url(%22images%2Fui-icons_555555_256x240.png%22)... * Copyright jQuery Foundation and other contributors; Licensed MIT */
The regular expression in storage.HashedFilesMixin matches url(...)
in the comment, but it does NOT unquote the urlencoded quotation mark: %22
The last step of converter()
unquotes the transformed url: return template % unquote(transformed_url)
, converting the urlencoded quotation marks back, and thus messing up the filename, as can be seen in the posted stack trace:
The file 'jquery-ui-dist/"images/ui-icons_555555_256x240.png"' could not be found...
(Notice the excess quotation marks in the path.)
Patch:
storage.py line 61
- r"""(url\(['"]{0,1}\s*(.*?)["']{0,1}\))""",
+ r"""(url\((?:['"]|%22|%27){0,1}\s*(.*?)(?:['"]|%22|%27){0,1}\))""",
This matches url encoded single- or double-quotes and fixed several such issues with 3rd party packages that included generated CSS files like this.
IF a general fix for ignoring comments in CSS files is implemented, that would resolve this issue too.
But IF NOT, this is a simple fix that prevents collectstatic
from crashing on common 3rd party packages.
Happy to submit a patch for this if this seems worthwhile.
comment:11 Changed 5 years ago by
As long as we are using regular expressions, excluding comment contents seems out of question. So your patch might be worthfile in the short term.
A proper resolution of this ticket would be to use a JS parser to get links. This will be much more invasive though.
comment:12 Changed 5 years ago by
Replying to powderflask:
If you need to resolve this issue in the interim, override ManifestStaticFilesStorage (or CachedStaticFilesStorage) and re-define the class patterns property:
from django.contrib.staticfiles import storage class PatchedManifestStaticFilesStorage(storage.ManifestStaticFilesStorage): """ Override the replacement patterns to match URL-encoded quotations. """ patterns = ( ("*.css", ( r"""(url\((?:['"]|%22|%27){0,1}\s*(.*?)(?:['"]|%22|%27){0,1}\))""", (r"""(@import\s*["']\s*(.*?)["'])""", """@import url("%s")"""), )), )
comment:13 Changed 4 years ago by
Owner: | changed from nobody to Nathan Gaberel |
---|---|
Status: | new → assigned |
comment:14 Changed 4 years ago by
Has patch: | set |
---|
comment:15 Changed 4 years ago by
Patch needs improvement: | set |
---|
comment:16 Changed 3 years ago by
Just in case anyone is looking for a _really_ simple hotfix, you can simply strip the quotes from the name
in hashed_name()
.
Specifically, somewhere at the top of this method: https://github.com/django/django/blob/efc3e32d6d7fb9bb41be73b80c8607b653c1fbd6/django/contrib/staticfiles/storage.py#L79-L111
Or... put this monkeypatch somewhere in your code (settings file for example):
from django.contrib.staticfiles import storage import functools original_hashed_name = storage.HashedFilesMixin.hashed_name @functools.wraps(original_hashed_name) def hashed_name(self, name, *args, **kwargs): return original_hashed_name(self, name.strip('"'), *args, **kwargs) storage.HashedFilesMixin.hashed_name = hashed_name
comment:17 Changed 2 years ago by
Owner: | changed from Nathan Gaberel to Tomáš Zigo |
---|---|
Patch needs improvement: | unset |
Resetting flags for more recent, unreviewed PR.
comment:18 Changed 2 years ago by
Needs tests: | set |
---|---|
Patch needs improvement: | set |
comment:19 Changed 21 months ago by
FWIW, the current approach of parsing CSS with regexes also doesn't work with SVG data inlined as data: URLs when they reference for example <linearGradient id='someid' ...>..</..> <use fill='url(%23someid)' ...></use>
. This is changed to something like fill='url("#someid#someid")'
.
Two shortcomings here are:
- double quotes are inserted that weren't there before, breaking the quoting of the data:-URL that the svg data is in
- the doubling of the id fragment.
Concerning the second problem: I didn't look too far into it, but that seems to happen because the font-face branch is triggered https://github.com/django/django/blob/96f55ccf798c7592a1203f798a4dffaf173a9263/django/contrib/staticfiles/storage.py#L131-L142
It's also at least curious that after line 131 final_url already has any fragment hash marks escaped (# -> %23), so urlsplit in line 137 doesn't catch it as a fragment.
The relevant extract of our CSS looks like this:
.sc_modal .content.branded-bg { background-image: url("data:image/svg+xml,%3csvg width='375' [...] %3cuse fill='url(%23c)' [...]"); }
CachedFilesMixin
is using simple regular expressions to matchurl
and@import
statements, and so is picking up the ones inside comments. It would also have a problem if the text was inside a string.If it's possible to rewrite the regex to cover all the corner cases, that would be the easy fix. But I doubt it. The robust way would be to parse the css files properly, but that would mean rewriting a lot of code...