HashedFilesMixin does not properly skip protocol-relative urls when STATIC_URL='/'

[Will Gulian]

While protocol-relative urls have been deprecated it would be nice for Django staticfiles to support it since a lot of code still uses it or explicitly not support it. Right now the relevant snippet implies that the code does filter out protocol-relative urls but it currently does not:

# django/contrib/staticfiles/storage.py

# Ignore absolute/protocol-relative and data-uri URLs.
if re.match(r'^[a-z]+:', url):
    return matched

I've included an example snippet that uses a protocol-relative url but is not filtered:

 @import url("//fonts.googleapis.com/css?family=Source+Sans+Pro:400,700|Raleway:400,800,900");

[Tim Graham]

The code changed in 08ed3cc6d160d0d864ff687db9a62959a86e7372 so the comment is outdated but as far as I see, a URL starting with // would likely be filtered out in the next block: if url.startswith('/') and not url.startswith(settings.STATIC_URL):. Anyway, there's still a test assertion for //foobar remaining unchanged and I don't see a change to the URL you provided if I add that to the test. Can you find the difference between that test and your situation that reproduces the problem?

[Will Gulian]

Replying to Tim Graham:

The code changed in 08ed3cc6d160d0d864ff687db9a62959a86e7372 so the comment is outdated but as far as I see, a URL starting with // would likely be filtered out in the next block: if url.startswith('/') and not url.startswith(settings.STATIC_URL):. Anyway, there's still a test assertion for //foobar remaining unchanged and I don't see a change to the URL you provided if I add that to the test. Can you find the difference between that test and your situation that reproduces the problem?

Sorry I should have looked at that function more closely. It's not being caught in my case because my STATIC_URL is / so the line that should exit doesn't because the protocol-relative url actually starts with my STATIC_URL.

[Adam Zapletal]

It looks like adding back a simple check for protocol-relative URLs before the STATIC_URL check will fix this. That seems reasonable to me if Django is going to support setting STATIC_URL to /.

I opened a PR with this change and a regression test for discussion.

[Mariusz Felisiak <felisiak.mariusz@…>]

In 107aa76:

Fixed #29022 -- Fixed handling protocol-relative URLs in ManifestStaticFilesStorage when STATIC_URL is set to /.