[Django] #32849: ManifestStaticFilesStorage newline breaks regex
Django
-----------------------------------------------+------------------------
Reporter: TZanke | Owner: nobody
Type: Bug | Status: new
Component: contrib.staticfiles | Version: 3.2
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-----------------------------------------------+------------------------
Hello,
I am trying to use the `ManifestStaticFilesStorage` static file handling.
Right now i found a possible issue with line breaks.
The file in which strings should be replaced is loaded as a string with
line breaks:
https://github.com/django/django/blob/main/django/contrib/staticfiles/storage.py#L315
The default regex may not have this issue, `default_template =
"""url("%(url)s")"""` cause a minifier will not break inside this `url()`
method.
https://github.com/django/django/blob/main/django/contrib/staticfiles/storage.py#L45
But in my case a more complex string with URL has to be replaced, so
currently the string sometimes contains a line break.
The line break now is not matched against the regex which leads to a
caching problem.
Expample:
CKEditor4: https://cdn.ckeditor.com/4.16.1/standard/ckeditor.js
Search for "styles/tableselection.css"
First occurrence is split by line break.
My regex: `(appendStyleSheet\(this\.path\+"([^'"]*?)"\))`
Regex with newline support:
`(appendStyleSheet\(this\s?\.\s?path\s?\+\s?"([^'"]*?)"\))`
Question:
Who is responsible for possible line breaks?
Maybe Django should load the content without line breaks?
Or has the developer/user of ManifestStaticFilesStorage a responsibility
to care for possible line breaks in his regexes?
But: Maybe he (the developer) does not know this can happen. Later on,
someone else changes the JS code => minifyer adds linebreaks => production
=> regex mismatch => caching problem
Let me know whats your opinion.
Best regards
Tobias
--
Ticket URL: <https://code.djangoproject.com/ticket/32849>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: TZanke | Owner: nobody
Type: Bug | Status: new
Component: contrib.staticfiles | Version: 3.2
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* stage: Unreviewed => Accepted
Comment:
Hi Tobias. Thanks for the report.
I'm not 100% sure, but let's provisionally Accept to see what a PR looks
like.
If you can reduce that CKEditor example to a minimum case, that would
help.
(Can we always strip `\n` in JS without loss? — minifiers often output on
one line, no? 🤔)
--
Ticket URL: <https://code.djangoproject.com/ticket/32849#comment:1>
Django
Reporter: TZanke | Owner: nobody
Type: Bug | Status: new
Component: contrib.staticfiles | Version: 3.2
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Keryn Knight):
Replying to [comment:1 Carlton Gibson]:
> (Can we always strip `\n` in JS without loss? [...])
''Possibly'' not, due to the rules around Automatic Semi-colon Insertion
(similar to how white-space is sort-of significant in HTML). I'd presume
any/all minifiers do explicit insertion in such cases to avoid affecting
the intended output...
--
Ticket URL: <https://code.djangoproject.com/ticket/32849#comment:2>
Django
Reporter: TZanke | Owner: geordan
| noubissie
Type: Bug | Status: assigned
Component: contrib.staticfiles | Version: 3.2
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by geordan noubissie):
* owner: nobody => geordan noubissie
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/32849#comment:3>
Django
Reporter: TZanke | Owner: geordan
| noubissie
Type: Bug | Status: assigned
Keywords: | Triage Stage: Accepted
Easy pickings: 0 | UI/UX: 0
Changes (by geordan noubissie):
--
Ticket URL: <https://code.djangoproject.com/ticket/32849#comment:4>
Django
Reporter: TZanke | Owner: geordan
| noubissie
Type: Bug | Status: assigned
Keywords: | Triage Stage: Accepted
Easy pickings: 0 | UI/UX: 0
Comment (by geordan noubissie):
Patch available via PR #19322 – fixes newline handling in
`ManifestStaticFilesStorage`.
--
Ticket URL: <https://code.djangoproject.com/ticket/32849#comment:5>
Django
Reporter: TZanke | Owner: geordan
| noubissie
Type: Bug | Status: assigned
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Changes (by Sarah Boyce):
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/32849#comment:6>
Django
Reporter: TZanke | Owner: geordan
| noubissie
Type: Bug | Status: assigned
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Comment (by blighj):
This package I've been working on and hoping to get feedback on, takes
care of this ticket as it is no longer using regex's to parse
import/export statements https://github.com/blighj/django-
manifeststaticfiles-enhanced
--
Ticket URL: <https://code.djangoproject.com/ticket/32849#comment:7>
Django
Reporter: TZanke | Owner: blighj
Type: Bug | Status: assigned
Component: contrib.staticfiles | Version: 3.2
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Changes (by blighj):
* owner: geordan noubissie => blighj
Comment:
This https://github.com/django/django/pull/19561 PR for #21080 will make
this ticket obsolete.
--
Ticket URL: <https://code.djangoproject.com/ticket/32849#comment:8>