Opened 10 years ago

Closed 10 years ago

#23173 closed Bug (fixed)

SCRIPT_URL on WSGI is misinterpreted when PATH_INFO is empty

Reported by: j-sz@… Owned by: Bas Peschier
Component: Core (URLs) Version: dev
Severity: Normal Keywords: wsgi, script name, negative index slicing, ams2015
Cc: 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 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.

Change History (8)

comment:1 by Baptiste Mispelon, 10 years ago

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.

in reply to:  1 comment:2 by Jan Szejko, 10 years ago

Replying to 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

Version 0, edited 10 years ago by Jan Szejko (next)

comment:3 by Baptiste Mispelon, 10 years ago

Triage Stage: UnreviewedAccepted

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.

comment:4 by Baptiste Mispelon, 10 years ago

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.

comment:5 by Jan Szejko, 10 years ago

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 if it's what you're looking for, but it does the job.

Last edited 10 years ago by Jan Szejko (previous) (diff)

comment:6 by Bas Peschier, 10 years ago

Keywords: ams2015 added
Owner: changed from nobody to Bas Peschier
Status: newassigned

comment:7 by Markus Holtermann, 10 years ago

Has patch: set
Triage Stage: AcceptedReady for checkin

comment:8 by Markus Holtermann <info@…>, 10 years ago

Resolution: fixed
Status: assignedclosed

In 336512fae77ec214ad6db24166b9a8676007cc09:

Fixed #23173 -- Fixed incorrect stripping of SCRIPT_URL

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