Opened 4 years ago

Closed 5 weeks ago

Last modified 4 weeks ago

#17133 closed Cleanup/optimization (fixed)

get_script_name goofs when there is Apache URL rewriting

Reported by: gjanee@… Owned by: nobody
Component: HTTP handling Version: master
Severity: Normal Keywords:
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 (last modified by claudep)

When running under Apache+mod_wsgi (I'm not sure the mod_wsgi is relevant here), using a WSGIScriptAlias of /foo, a request for URL /foo/bar//baz (note double slash) gets transformed by Apache to /foo/bar/baz and then by Django to /foo//bar/baz (note where double slashes are now). Furthermore, a callback function for urlpattern "^bar/.*" will be called on this latter path even though it doesn't match the pattern. That is, request.path will be /foo//bar/baz, which can be confusing for a callback function that is expecting request.path to match the WSGIScriptAlias plus the urlpattern.

I don't totally understand what Django is doing here, but I believe the problem is in django.core.handlers.base.get_script_name. In that function, to get the script name in this situation Django starts with the environment variable SCRIPT_URL, and strips off a number of characters at the end equal to the length of environment variable PATH_INFO. Because in this case Apache collapses the double slash in /foo/bar//baz to /foo/bar/baz, get_script_name strips off one less character than necessary, so that instead of the script name being /foo it is /foo/. This may account for the double slash appearing /foo, as in /foo//bar/baz.

Attachments (1)

django_script_path.patch (710 bytes) - added by mwilck 3 years ago.
Suggested patch (for Django 1.3.1, but should apply on master as well)

Download all attachments as: .zip

Change History (13)

comment:2 Changed 4 years ago by aaugustin

  • Resolution set to needsinfo
  • Status changed from new to closed

While it's clear that something's wrong in your setup, I'm unable to confirm that there's a bug in Django, and determine how we could fix it, sorry.

You might be exceeding the limits of how much mod_rewrite magic Django can compensate for; if so, you can work around the problem with the FORCE_SCRIPT_NAME setting.

If we wanted to debug this further, we'd need the values of the relevant environment variables, especially SCRIPT_URL, REDIRECT_URL, PATH_INFO, SCRIPT_NAME, as well as the relevant bits of your Apache configuration.

comment:3 Changed 3 years ago by mwilck

  • Has patch set
  • Resolution needsinfo deleted
  • Status changed from closed to reopened

This is definitely a Django bug, given that Apache/mod_wsgi is as it is.
Here are the variables for my case:

SCRIPT_URL = '/mst/milestones//accounts/login//help'
PATH_INFO = u'/milestones/accounts/login/help'

SCRIPT_NAME = u'/mst/m'

The correct SCRIPT_NAME would be just u'/mst'.

Last edited 3 years ago by claudep (previous) (diff)

Changed 3 years ago by mwilck

Suggested patch (for Django 1.3.1, but should apply on master as well)

comment:4 Changed 3 years ago by mwilck

See e.g. for an independent assessment of the same problem.

comment:6 Changed 3 years ago by claudep

  • Description modified (diff)

comment:7 Changed 3 years ago by lrekucki

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

Here's an explanation from Graham Dumpleton (a bit old, but seems up-to-date):

Nitpicking: The import and regexp definition should be on top of the file and a comment with a link to this issue would decrease the WTH level ;)

comment:8 Changed 3 years ago by aaugustin

  • Type changed from Uncategorized to Cleanup/optimization

comment:9 Changed 3 years ago by aaugustin

  • Status changed from reopened to new

comment:10 Changed 18 months ago by timo

  • Needs tests set

Is there any way to add a test?

comment:11 Changed 5 weeks ago by claudep

  • Needs tests unset
  • Version changed from 1.3 to master

comment:12 Changed 5 weeks ago by timgraham

  • Triage Stage changed from Accepted to Ready for checkin

comment:13 Changed 5 weeks ago by Claude Paroz <claude@…>

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

In 10ace52a:

Fixed #17133 -- Properly handled successive slashes in incoming requests

Thanks gjanee@… for the report and Tim Graham for the review.

comment:14 Changed 4 weeks ago by Claude Paroz <claude@…>

In ea2f48c:

Refs #17133 -- Optimized script_url handling in get_script_name

10ace52a added some regex processing for each request with SCRIPT_URL set.
In a speed critical section, conditionally apply of the regex will save some

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