[Django] #23173: SCRIPT_URL on WSGI is misinterpreted when PATH_INFO is empty
Django
-------------------------+-------------------------------------------------
Reporter: j-sz@… | Owner: nobody
Type: Bug | Status: new
Component: Core | Version: master
(URLs) | Keywords: wsgi, script name, negative index
Severity: Normal | slicing
Triage Stage: | Has patch: 0
Unreviewed | UI/UX: 0
Easy pickings: 0 |
-------------------------+-------------------------------------------------
This bug is pretty obvious. In the dev version it's in
django/core/handlers/wsgi.py:235 in get_script_name; in 1.5.1 (where I've
found it) it's in django/core/handlers/base.py:280. I haven't checked how
long it's been there.
The problem is due to slicing with a negative end index:
{{{
script_name = script_url[:-len(path_info)]
}}}
It works fine as long as path_info is not empty. On my system it was empty
and the whole script_url was truncated.
I've grepped the source tree for `'\[:-[^0-9]'` to check if there are some
other instances of this pattern and it returned a couple of results. I'd
suggest examining them and making sure the indices are non-zero.
A patch would be trivial, but I'm not giving it, because I feel that a
general function 'chop off n elements from the end' might be handy and I'm
not sure where to put it.
--
Ticket URL: <https://code.djangoproject.com/ticket/23173>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: j-sz@… | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: wsgi, script name, | Triage Stage:
negative index slicing | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by bmispelon):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
Isn't `path_info` guaranteed to be non-empty because of
https://github.com/django/django/blob/master/django/core/handlers/wsgi.py#L87-L92?
Could you give us some details on what error you're seeing and how you're
triggering it?
Thanks.
--
Ticket URL: <https://code.djangoproject.com/ticket/23173#comment:1>
Django
Reporter: j-sz@… | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: wsgi, script name, | Triage Stage:
negative index slicing | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by janek37):
Replying to [comment:1 bmispelon]:
> Isn't `path_info` guaranteed to be non-empty because of
https://github.com/django/django/blob/master/django/core/handlers/wsgi.py#L87-L92?
No, it isn't. Look at it in context:
https://github.com/django/django/blob/master/django/core/handlers/wsgi.py#L233-L235.
It reads PATH_INFO straight from the environment and it assumes an empty
string if the variable is not set.
>
> Could you give us some details on what error you're seeing and how
you're triggering it?
>
> Thanks.
In my evironment the PATH_INFO variable is not set. I don't get any error
message, I just get wrong urls when calling
django.core.urlresolvers.reverse or {% url %} template tag. Eg. I have a
WSGI server at /foobar/ and the resolve function returns '/some/url/'
instead of '/foobar/some/url/'. Easy to see, when SCRIPT_URL variable is
set to '/foobar/' and PAT_INFO is not set, get_script_name(environ) will
return '' while it should return '/foobar/'. Just look at the code, it's
obvious:
https://github.com/django/django/blob/master/django/core/handlers/wsgi.py#L229-L237
--
Ticket URL: <https://code.djangoproject.com/ticket/23173#comment:2>
Django
Reporter: j-sz@… | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
negative index slicing | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by bmispelon):
* stage: Unreviewed => Accepted
Comment:
OK, marking this as `accepted`.
FYI, it's easier for me to triage your ticket if you provide a concrete
example of how to trigger the bug because then I know how to try and
reproduce it.
If you just point to a line of code, I have to try and figure out
where/how that code is used and see if there's a way to trigger the bug in
a normal use-case. This can be hard and time consuming, especially if I'm
not familiar with the codebase (which is the case here for example).
This is why I asked for more details in my first comment.
Thanks.
--
Ticket URL: <https://code.djangoproject.com/ticket/23173#comment:3>
Django
Reporter: j-sz@… | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: wsgi, script name, | Triage Stage: Accepted
negative index slicing | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by bmispelon):
Also, I took a quick look at other instances of `'\[:-[^0-9]'` and they
all seem OK except maybe one.
There's one in
https://github.com/django/django/blob/master/django/contrib/staticfiles/storage.py#L180
that could be an issue but I couldn't figure out if there's actually a way
to trigger the problematic case.
If you can figure it out, please open a separate ticket.
Thanks.
--
Ticket URL: <https://code.djangoproject.com/ticket/23173#comment:4>
Django
Reporter: j-sz@… | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: wsgi, script name, | Triage Stage: Accepted
negative index slicing | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Comment (by janek37):
One way to reproduce this bug is to run in a Django console:
{{{
from django.core.handlers.wsgi import get_script_name
get_script_name({'SCRIPT_URL': '/foobar/'})
}}}
I'm not sure it's what you're looking for, but it does the job.
--
Ticket URL: <https://code.djangoproject.com/ticket/23173#comment:5>
Django
Reporter: j-sz@… | Owner: bpeschier
Type: Bug | Status: assigned
Component: Core (URLs) | Version: master
Severity: Normal | Resolution:
Keywords: wsgi, script name, | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* keywords: wsgi, script name, negative index slicing => wsgi, script
name, negative index slicing,ams2015
* owner: nobody => bpeschier
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/23173#comment:6>
Django
Reporter: j-sz@… | Owner: bpeschier
Type: Bug | Status: assigned
Component: Core (URLs) | Version: master
Severity: Normal | Resolution:
negative index slicing,ams2015 | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/23173#comment:7>
Django
Reporter: j-sz@… | Owner: bpeschier
Component: Core (URLs) | Version: master
Keywords: wsgi, script name, | Triage Stage: Ready for
negative index slicing,ams2015 | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"336512fae77ec214ad6db24166b9a8676007cc09"]:
{{{
#!CommitTicketReference repository=""
revision="336512fae77ec214ad6db24166b9a8676007cc09"
Fixed #23173 -- Fixed incorrect stripping of SCRIPT_URL
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23173#comment:8>