[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>
Django
Reporter: TZanke | Owner: blighj
Type: Bug | Status: assigned
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Comment (by Jacob Walls):
Given that we didn't merge ​https://github.com/django/django/pull/19561
(lexer), do you think we can leave this ticket open for another
incremental, small-scoped regex fix, James?
--
Ticket URL: <https://code.djangoproject.com/ticket/32849#comment:9>
Django
Reporter: TZanke | Owner: blighj
Type: Bug | Status: assigned
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Comment (by blighj):
On reflection, I don't think this ticket is something for core django.
django's collectstatic supports css url/import statements, and the
experimental regexs for js support import/export statements.
There is nothing in the documentation saying that the patterns are
designed to be user extended. The patterns attribute is mentioned in the
docs, so maybe you could argue that it is, but I think if you are at the
level where you are subclassing and extending the patterns you can add
whatever newline handling you need to the code too at that point.
I'd vote we close this particular ticket.
--
Ticket URL: <https://code.djangoproject.com/ticket/32849#comment:10>
Django
Reporter: TZanke | Owner: blighj
Component: contrib.staticfiles | Version: 3.2
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Changes (by Jacob Walls):
* resolution: => wontfix
* status: assigned => closed
--
Ticket URL: <https://code.djangoproject.com/ticket/32849#comment:11>