Opened 18 months ago

Last modified 17 months ago

#34496 closed Bug

ManifestStaticFilesStorage.patterns for sourceMappingURL does not retrieve matched line to return for for example data URI — at Initial Version

Reported by: Hielke Walinga Owned by: nobody
Component: contrib.staticfiles Version: 4.2
Severity: Normal Keywords: ManifestStaticFilesStorage sourceMappingURL regex
Cc: Adam Johnson, gilmrjc Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

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.

Change History (0)

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