[Django] #29022: HashedFilesMixin does not properly skip protocol-relative urls

[Django]

#29022: HashedFilesMixin does not properly skip protocol-relative urls
-----------------------------------------------+------------------------
Reporter: Will Gulian | Owner: nobody
Type: Bug | Status: new
Component: contrib.staticfiles | Version: 2.0
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 |
-----------------------------------------------+------------------------
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:
{{{#!python
# 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:

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

--
Ticket URL: <https://code.djangoproject.com/ticket/29022>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#29022: HashedFilesMixin does not properly skip protocol-relative urls

-------------------------------------+-------------------------------------

Reporter: Will Gulian | Owner: nobody
Type: Bug | Status: new
Component: contrib.staticfiles | Version: 2.0

Severity: Normal | Resolution:

Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------

Comment (by 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?

--
Ticket URL: <https://code.djangoproject.com/ticket/29022#comment:1>

[Django]

#29022: HashedFilesMixin does not properly skip protocol-relative urls

-------------------------------------+-------------------------------------

Reporter: Will Gulian | Owner: nobody
Type: Bug | Status: new
Component: contrib.staticfiles | Version: 2.0

Severity: Normal | Resolution:

Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------

Comment (by Will Gulian):

Replying to [comment:1 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`.

--
Ticket URL: <https://code.djangoproject.com/ticket/29022#comment:2>

[Django]

#29022: HashedFilesMixin does not properly skip protocol-relative urls when
STATIC_URL='/'
-------------------------------------+------------------------------------

Reporter: Will Gulian | Owner: nobody
Type: Bug | Status: new
Component: contrib.staticfiles | Version: 2.0

Severity: Normal | Resolution:
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 Tim Graham):

* stage: Unreviewed => Accepted

--
Ticket URL: <https://code.djangoproject.com/ticket/29022#comment:3>

[Django]

#29022: HashedFilesMixin does not properly skip protocol-relative urls when
STATIC_URL='/'
-------------------------------------+-------------------------------------
Reporter: Will Gulian | Owner: Adam
| Zapletal
Type: Bug | Status: assigned
Component: contrib.staticfiles | Version: 2.0

Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------
Changes (by Adam Zapletal):

* has_patch: 0 => 1
* owner: nobody => Adam Zapletal
* status: new => assigned

Comment:

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.
--
Ticket URL: <https://code.djangoproject.com/ticket/29022#comment:4>

[Django]

#29022: HashedFilesMixin does not properly skip protocol-relative urls when
STATIC_URL='/'
-------------------------------------+-------------------------------------
Reporter: Will Gulian | Owner: Adam
| Zapletal
Type: Bug | Status: assigned

Component: contrib.staticfiles | Version: 2.0

Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak):

* needs_better_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/29022#comment:5>

[Django]

#29022: HashedFilesMixin does not properly skip protocol-relative urls when
STATIC_URL='/'
-------------------------------------+-------------------------------------
Reporter: Will Gulian | Owner: Adam
| Zapletal
Type: Bug | Status: assigned

Component: contrib.staticfiles | Version: 2.0

Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------
Changes (by Adam Zapletal):

* needs_better_patch: 1 => 0

--
Ticket URL: <https://code.djangoproject.com/ticket/29022#comment:6>

[Django]

#29022: HashedFilesMixin does not properly skip protocol-relative urls when
STATIC_URL='/'
-------------------------------------+-------------------------------------
Reporter: Will Gulian | Owner: Adam
| Zapletal
Type: Bug | Status: assigned

Component: contrib.staticfiles | Version: 2.0

Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak):

* needs_tests: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/29022#comment:7>

[Django]

#29022: HashedFilesMixin does not properly skip protocol-relative urls when
STATIC_URL='/'
-------------------------------------+-------------------------------------
Reporter: Will Gulian | Owner: Adam
| Zapletal
Type: Bug | Status: assigned

Component: contrib.staticfiles | Version: 2.0

Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------
Changes (by Adam Zapletal):

* needs_tests: 1 => 0

--
Ticket URL: <https://code.djangoproject.com/ticket/29022#comment:8>

[Django]

#29022: HashedFilesMixin does not properly skip protocol-relative urls when
STATIC_URL='/'
-------------------------------------+-------------------------------------
Reporter: Will Gulian | Owner: Adam
| Zapletal
Type: Bug | Status: assigned

Component: contrib.staticfiles | Version: 2.0
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak):

* stage: Accepted => Ready for checkin

--
Ticket URL: <https://code.djangoproject.com/ticket/29022#comment:9>

[Django]

#29022: HashedFilesMixin does not properly skip protocol-relative urls when
STATIC_URL='/'
-------------------------------------+-------------------------------------
Reporter: Will Gulian | Owner: Adam
| Zapletal

Type: Bug | Status: closed
Component: contrib.staticfiles | Version: 2.0
Severity: Normal | Resolution: fixed

Keywords: | Triage Stage: Ready for
| checkin

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak <felisiak.mariusz@…>):

* resolution: => fixed
* status: assigned => closed

Comment:

In [changeset:"107aa76bcf5d5599460fdce61dfa15bb147acc62" 107aa76]:
{{{#!CommitTicketReference repository=""
revision="107aa76bcf5d5599460fdce61dfa15bb147acc62"
Fixed #29022 -- Fixed handling protocol-relative URLs in
ManifestStaticFilesStorage when STATIC_URL is set to /.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/29022#comment:10>