[Django] #34496: ManifestStaticFilesStorage.patterns for sourceMappingURL does not retrieve matched line to return for for example data URI
Django
matched line to return for for example data URI
-------------------------------------+-------------------------------------
Reporter: Hielke | Owner: nobody
Walinga |
Type: Bug | Status: new
Component: | Version: 4.2
contrib.staticfiles | Keywords:
Severity: Normal | ManifestStaticFilesStorage
Triage Stage: | sourceMappingURL regex
Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
This is a regression from
https://github.com/django/django/commit/781b44240a06f0c868254f40f36ce46c927f56d1
that has been attempted to be fixed in
https://code.djangoproject.com/ticket/33237, but has not completely fixed
this. The bug is in django/contrib/staticfiles/storage.py.
Sometimes, for a sourceMappingURL a data URI is provided instead. The
ManifestStaticFilesStorage accounted for this. It will then not change
that URI, but instead return matched pattern back. This pattern is
captured in the "matched" group.
However, after the change to using named capturing groups in the regex,
the "matched" group has been misplaced, as it was confused with the (?m)
multiline indicator. This has unfortunately not been completely fixed
after this was noted in https://code.djangoproject.com/ticket/33237 as
that only added the (?m) back, but still misplaced the matched group.
Let's compare the code:
Old:
{{{
('*.js', (
(r'(?m)^(//# (?-i:sourceMappingURL)=(.*))$', '//#
sourceMappingURL=%s'),
)),
}}}
Current:
{{{
"*.js",
(
(
r"(?m)(?P<matched>)^(//#
(?-i:sourceMappingURL)=(?P<url>.*))$",
"//# sourceMappingURL=%(url)s",
),
),
}}}
Fix:
{{{
"*.js",
(
(
r"(?m)^(?P<matched>//#
(?-i:sourceMappingURL)=(?P<url>.*))$",
"//# sourceMappingURL=%(url)s",
),
),
}}}
This fix must also be applied to the *css sourceMappingURL pattern.
== An alternative
**''However''**, let's make this better instead. Because the matched
pattern is the complete matched pattern and Python already saves the
complete pattern in the "0" group. So, instead, let's make things simple.
An alternative fix would be to remove all "matched" groupings and instead
use the "0" group in the url_converter.
Now, let's even make this backwards compatible for people that added their
own patterns prior to 4.0 and thus did not use these named captures.
Current:
{{{
def converter(matchobj):
matches = matchobj.groupdict()
matched = matches["matched"]
url = matches["url"]
}}}
Proposed Change:
{{{
def converter(matchobj):
matched = matchobj[0]
try:
url = matchobj["url"]
except IndexError:
# If "url" does not exists,
# take the second captured group for backwards compatibility
prior to changing to named capturing groups.
url = matchobj[2]
}}}
And remove all (?<matched> capturing groups from regex, which makes the
regex a little bit more legible, which seemed to have causing a lot of
confusing during changing these!
== PR
For whatever change you prefer, I can make a PR. Thank you. I hope the fix
can also be applied in 4.2.
--
Ticket URL: <https://code.djangoproject.com/ticket/34496>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
matched line to return for for example data URI
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: contrib.staticfiles | Version: 4.2
Severity: Normal | Resolution:
Keywords: | Triage Stage:
ManifestStaticFilesStorage | Unreviewed
sourceMappingURL regex |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Old description:
New description:
Old:
Current:
Fix:
== An alternative
}}}
== PR
== Post Scriptum
I noticed, for complete backwards compatibility prior to named patterns,
the template should also have a fallback for this.
Current:
{{{
matches["url"] = unquote(transformed_url)
return template % matches
}}}
Proposed change:
{{{
try:
# Remain backwards compatible prior to changing to named
template strings
return template % unquote(transformed_url)
except TypeError:
# Template string is thus a named template string
return template % {"url": unquote(transformed_url)}
}}}
--
--
Ticket URL: <https://code.djangoproject.com/ticket/34496#comment:1>
Django
matched line to return for for example data URI
-------------------------------------+-------------------------------------
Reporter: Hielke Walinga | Owner: nobody
Component: contrib.staticfiles | Version: 4.2
Severity: Normal | Resolution: needsinfo
Keywords: | Triage Stage:
ManifestStaticFilesStorage | Unreviewed
sourceMappingURL regex |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => needsinfo
Comment:
Please provide a specific example where the current regexp doesn't work.
I'd also prefer to update the regexp instead of removing the named groups.
--
Ticket URL: <https://code.djangoproject.com/ticket/34496#comment:2>
Django
matched line to return for for example data URI
-------------------------------------+-------------------------------------
Reporter: Hielke Walinga | Owner: nobody
Type: Bug | Status: closed
Component: contrib.staticfiles | Version: 4.2
Severity: Normal | Resolution: needsinfo
Keywords: | Triage Stage:
ManifestStaticFilesStorage | Unreviewed
sourceMappingURL regex |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak):
* cc: Adam Johnson, gilmrjc (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/34496#comment:3>
Django
matched line to return for for example data URI
-------------------------------------+-------------------------------------
Reporter: Hielke Walinga | Owner: nobody
Type: Bug | Status: closed
Component: contrib.staticfiles | Version: 4.2
Severity: Normal | Resolution: needsinfo
Keywords: | Triage Stage:
ManifestStaticFilesStorage | Unreviewed
sourceMappingURL regex |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Hielke Walinga):
I created a test case for the Django test suite with an example. I also
provided a fix that I assured fixed that.
https://github.com/django/django/compare/main...hwalinga:django:fix-set-
matched-regex-group-correctly
The example sourceMappingURL line is "//#
sourceMappingURL=data:application/json;charset=utf8;base64,eyJ2ZXJzaW9uIjozLCJzb3VyY2VzIjpbIl9zcmMv",
which is currently removed, but should be untouched.
--
Ticket URL: <https://code.djangoproject.com/ticket/34496#comment:4>