Opened 10 months ago

Closed 3 months ago

#23173 closed Bug (fixed)

SCRIPT_URL on WSGI is misinterpreted when PATH_INFO is empty

Reported by: j-sz@… Owned by: bpeschier
Component: Core (URLs) Version: master
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 follow-up: Changed 10 months ago by bmispelon

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

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.

comment:2 in reply to: ↑ 1 Changed 10 months ago by janek37

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 PATH_INFO is not set, get_script_name(environ) will return an empty string 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

Last edited 10 months ago by janek37 (previous) (diff)

comment:3 Changed 10 months ago by bmispelon

  • Triage Stage changed from Unreviewed to Accepted

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 Changed 10 months ago 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.

comment:5 Changed 10 months ago 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.

Version 0, edited 10 months ago by janek37 (next)

comment:6 Changed 3 months ago by bpeschier

  • Keywords ams2015 added
  • Owner changed from nobody to bpeschier
  • Status changed from new to assigned

comment:7 Changed 3 months ago by MarkusH

  • Has patch set
  • Triage Stage changed from Accepted to Ready for checkin

comment:8 Changed 3 months ago by Markus Holtermann <info@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In 336512fae77ec214ad6db24166b9a8676007cc09:

Fixed #23173 -- Fixed incorrect stripping of SCRIPT_URL

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