Opened 6 years ago

Closed 2 months ago

#29167 closed Bug (worksforme)

HashedFilesMixin doesn't work with data URIs that include a closing parenthesis

Reported by: Clem Flory Owned by: Adam Zapletal
Component: contrib.staticfiles Version: 2.0
Severity: Normal Keywords:
Cc: Adam Zapletal Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

My team recently started using Bootstrap 4, and some of their styles get broken by the regex patterns in HashedFilesMixin (​https://github.com/django/django/blob/2.0.2/django/contrib/staticfiles/storage.py#L52-L57). This is loosely related to #21080.

For example, this is one of the rules in the generated Bootstrap 4 CSS, and the current regex incorrectly mistakes the closing parentheses in rgba(0, 0, 0, 0.5) as the end of url():

.navbar-light .navbar-toggler-icon {
  background-image: url("data:image/svg+xml;charset=utf8,%3Csvg viewBox='0 0 30 30' xmlns='http://www.w3.org/2000/svg'%3E%3Cpath stroke='rgba(0, 0, 0, 0.5)' stroke-width='2' stroke-linecap='round' stroke-miterlimit='10' d='M4 7h22M4 15h22M4 23h22'/%3E%3C/svg%3E");
}

Not an ideal solution, but we were able to work around this issue by Base64 encoding these data URIs.

Change History (8)

comment:1 by Tim Graham, 6 years ago

Summary: collectstatic breaks Bootstrap 4 styles → HashedFilesMixin doesn't work with data URIs that include a closing parenthesis
Triage Stage: Unreviewed → Accepted

comment:2 by Adam Zapletal, 2 months ago

It looks like this was fixed in mid-2022 by commit e6f36ea0a97af5c7d18bd155a6c4a937cf658ce6.

I tried the CSS above with ManifestStaticFilesStorage on Django v5.0.2, and it works fine. collectstatic didnt't crash, and the CSS was unchanged.

comment:3 by Adam Zapletal, 2 months ago

Cc: Adam Zapletal added

comment:4 by Mariusz Felisiak, 2 months ago

Adam, Can you submit PR with a regression test to prove it's fixed?

comment:5 by Adam Zapletal, 2 months ago

Owner: changed from nobody to Adam Zapletal
Status: new → assigned

comment:6 by Adam Zapletal, 2 months ago

A PR has been submitted!

Last edited 2 months ago by Adam Zapletal (previous) (diff)

comment:7 by Adam Zapletal, 2 months ago

Has patch: set

comment:8 by Mariusz Felisiak, 2 months ago

Resolution: → worksforme
Status: assigned → closed
Triage Stage: Accepted → Unreviewed

Closing as "worksforme" unless someone can provide a reproducible scenario. Thanks Adam Zapletal for the investigation:

I tested this all the way back to Django v2.0.2, which is the release the bug reporter was probably using, and Python 3.8. I wasn't able to reproduce the bug. In the process, I realized that Django has been skipping the processing of data: URLs since 46c12d1293aa90209f3c640f214c4b5a3d6cb9e6. That was well before the version specified on the ticket.

Other than adding named groups, I don't think the regex in question has changed at all since it was initially added in 1d32bdd3c9586ff10d0799264105850fa7e3f512. The ticket is correct in that the regex doesn't match to the final closing parenthesis, but it doesn't need to because data: URLs are skipped.

I'm unsure how the bug reporter was able to get unexpected behavior. I doubt that this PR is even needed since there's already an assert for data: URLs in the tests, and I wonder if the ticket is even valid.

Note: See TracTickets for help on using tickets.
Back to Top