Opened 2 years ago

Last modified 20 months ago

#19670 new Bug

CachedFilesMixin Doesn't Limit Substitutions to Extension Matches

Reported by: matthewwithanm Owned by:
Component: contrib.staticfiles Version: 1.4
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The format of the patterns attribute seems to imply that substitutions will only be performed in files with the corresponding extension. However, if a file matches *any* of the filename patterns, then *all* of the listed substitutions will be performed. For example:

    patterns = (
        ("*.css", (
            r"""(url\(['"]{0,1}\s*(.*?)["']{0,1}\))""",
            r"""(@import\s*["']\s*(.*?)["'])""",
        )),
        ("*.js", (
            r"whatever",
        ),
    )

Not only will url_converter() be called for any occurrences of the word "whatever" in JavaScript files, but also for any occurrences of url('something') and @import('something') (in JavaScript files).

Attachments (1)

19670.diff (4.7 KB) - added by DrMeers 20 months ago.
WIP patch

Download all attachments as: .zip

Change History (6)

comment:1 Changed 2 years ago by carljm

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

comment:2 Changed 22 months ago by DrMeers

  • Owner changed from nobody to DrMeers
  • Status changed from new to assigned

I have a patch for this, just need to put the tests together.

comment:3 Changed 20 months ago by stavros

We are affected by this. It seems that changing for patterns in self._patterns.values(): to for patterns in self._patterns.get(extension, []): will do the trick.

comment:4 Changed 20 months ago by DrMeers

  • Owner DrMeers deleted
  • Status changed from assigned to new

Just in case I don't get around to finishing this, I'll attach my WIP patch. From memory it was all working fine apart from an isolation issue whereby an unrelated test method would cause failure depending on the order in which the tests were run.

Changed 20 months ago by DrMeers

WIP patch

comment:5 Changed 20 months ago by stavros

Looks correct to me. We'll try this out in our code and report back tomorrow, thanks.

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