Opened 6 years ago

Last modified 2 months ago

#21080 assigned Bug

collectstatic post-processing fails for references inside comments

Reported by: shreyas@… Owned by: Nathan Gaberel
Component: contrib.staticfiles Version: master
Severity: Normal Keywords:
Cc: Joshua Smith Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no 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 (15)

comment:1 Changed 6 years ago by Kevin Christopher Henry

Triage Stage: UnreviewedAccepted

CachedFilesMixin is using simple regular expressions to match url 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...

comment:2 Changed 6 years ago by anonymous

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 6 years ago by Pablo SEMINARIO

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 6 years ago by German Larrain

This bit me today. "Dumb" workaround: replace that url in your comments with u r l.

comment:5 Changed 6 years ago by Ben Spaulding

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 22 months ago by Udo Schochtert

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:

would be great if this issue could be fixed. thanks ;)

comment:7 Changed 22 months ago by Udo Schochtert

Version: 1.51.11

comment:8 Changed 17 months ago by Joshua Smith

Cc: Joshua Smith added
Version: 1.112.0

comment:9 Changed 17 months ago by Joshua Smith

Version: 2.0master

comment:10 Changed 15 months ago by powderflask

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 15 months ago by Claude Paroz

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 in reply to:  10 Changed 15 months ago by powderflask

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 months ago by Nathan Gaberel

Owner: changed from nobody to Nathan Gaberel
Status: newassigned

comment:14 Changed 4 months ago by Nathan Gaberel

Has patch: set

PR

I put something together that detects block comments in css (using a regexp) and ignores those urls that overlap with a comment.
It has the benefit of not requiring additional dependencies to parse css.

I think it's ready but I'm probably missing some test cases, feedback welcome!

comment:15 Changed 2 months ago by felixxm

Patch needs improvement: set
Note: See TracTickets for help on using tickets.
Back to Top