Django

Code

Ticket #285 (new)

Opened 3 years ago

Last modified 1 month ago

WSGIRequest should set request.path to full uri path

Reported by: rmunn@pobox.com Assigned to: mtredinnick
Component: HTTP handling Version: SVN
Keywords: Cc: sam@robots.org.uk, remco@diji.biz, cwebber@imagescape.com, hv@tbz-pariv.de
Triage Stage: Ready for checkin Has patch: 1
Needs documentation: 0 Needs tests: 0
Patch needs improvement: 0

Description

Following the instructions at http://wiki.dreamhost.com/index.php/Django, I got Django running under FastCGI. But the login form points to the wrong URL: instead of pointing to http://django.example.com/django-admin.fcgi/admin/, it points to http://django.example.com/admin/.

The reason is found in django/core/handlers/wsgi.py, where the WSGIRequest class's __init__() sets self.path equal to environ['PATH_INFO']. The request.path value eventually makes its way to the login template, which uses "app_path" as the form submission URL. In a CGI environment (including FastCGI), PATH_INFO contains the path referenced *after* the script name (here, "/admin/"). SCRIPT_NAME contains the path to the script (here, "/django-admin.fcgi". To construct a proper self-referential URL, you need to concatenate SCRIPT_NAME and PATH_INFO to obtain, in this case, "/django-admin.fcgi/admin/".

Attachments

285.patch (2.3 kB) - added by SmileyChris on 07/26/07 21:15:40.
285.2.patch (1.9 kB) - added by SmileyChris on 07/26/07 21:27:20.
script_name_path_info.diff (1.8 kB) - added by jmelesky on 09/14/07 17:38:30.
try and accomodate request.path appropriately in mod_python and under wsgi, add request.full_path for script_name plus path
script_name_path_info2.diff (15.4 kB) - added by jmelesky on 09/15/07 11:38:13.
corrected and far more comprehensive patch, against [6292]
script_name_path_info3.diff (19.6 kB) - added by jmelesky on 09/15/07 13:16:26.
another patch revision, against [6300], including docs and suggestions from Malcom
script_name_path_info4.diff (20.1 kB) - added by jmelesky on 09/15/07 14:30:48.
Another revision, against [6316]. This one actually passes the test suite.

Change History

08/06/05 14:37:25 changed by hugo <gb@bofh.ms>

I think that's more a bug in the FCGI description. It needs a rewrite rule that rewrites the URLs matching /admin/ to /django-admin.fcgi/admin/ so that the right PATH_INFO is passed in. You might have more luck with lighttpd+FCGI or Apache+FCGI, depending on the server you use.

08/07/05 17:01:54 changed by rmunn@pobox.com

I was looking through the code, and there are a lot of places that assume that request.path describes the entire URL. Changing that to a concatenation of SCRIPT_NAME and PATH_INFO would be quite tricky, because you'd need to distinguish between URL's destined for URL pattern matching (where you'd want to match against the PATH_INFO component only), and URL's destined for output in HTML (where you'd want to use the concatenation of SCRIPT_NAME and PATH_INFO). I'm starting to think that the complexity of the fix is not worth it when, as Hugo said, you can solve this one with a simple rewrite rule at the server level.

(follow-up: ↓ 8 ) 08/07/05 17:04:23 changed by rmunn@pobox.com

  • status changed from new to closed.
  • resolution set to wontfix.

Closing ticket as "wontfix" since I've concluded that this should be solved with a rewrite rule at the server level.

05/08/07 03:00:04 changed by Thomas Güttler

According to http://www.ietf.org/rfc/rfc3875 (See 4.1.6.)

PATH_INFO of http://django.example.com/django-admin.fcgi/admin/ should bei /admin/

It is sad, that django does not handle his according to the RFC. If you cannot add a rewrite rule, you can modify the PATH_INFO variable to include the path to the script. I did this for django/core/servers/cgi.py (from Trac ticket). You need to modify your url.py, too.

07/26/07 21:13:31 changed by SmileyChris

  • status changed from closed to reopened.
  • resolution deleted.
  • summary changed from Admin view uses wrong login URL under FastCGI to WSGIRequest should set request.path to full uri path.
  • component changed from Admin interface to HTTP handling.
  • stage changed from Unreviewed to Accepted.

Well if the RFC says it, we should be doing it.

It's not difficult. Thomas' suggestion is close, but rather than modify PATH_INFO, it's better to just change request.path to concatenate SCRIPT_NAME and PATH_INFO. This way, the Django url resolver can still (with a small tweak) use PATH_INFO for it's resolving so nothing breaks.

Related tickets: #1516, #2407

07/26/07 21:15:40 changed by SmileyChris

  • attachment 285.patch added.

07/26/07 21:24:41 changed by SmileyChris

  • has_patch set to 1.

Note: in debugging this, I noticed that ModPythonRequest wasn't actually setting PATH_INFO to the correct value!

It is currently using .path_info which according to the mod_python docs is "What follows after the file name, but is before query args, if anything".

It "sounds" like the right thing, but it isn't. Since we're not actually using SCRIPT_NAME in ModPythonRequest, it should just be set to .uri instead. This is a pretty big claim and I'm no expert, so maybe I'm totally wrong in which case someone please show me the flaw in my simplistic logic :)

07/26/07 21:27:20 changed by SmileyChris

  • attachment 285.2.patch added.

07/27/07 04:25:12 changed by anonymous

  • cc set to sam@robots.org.uk.

(in reply to: ↑ 3 ) 09/02/07 04:23:31 changed by jumo@gmx.de

Replying to rmunn@pobox.com:

Closing ticket as "wontfix" since I've concluded that this should be solved with a rewrite rule at the server level.

this isn't a cool way to fix this bug, i seems indeed very stupid for django beginners wich had some hours of trouble caused by it.

09/02/07 18:33:44 changed by SmileyChris

The ticket has been reopened, so obviously the "wontfix" doesn't apply now.

09/14/07 16:07:19 changed by jmelesky

  • owner changed from nobody to jmelesky.
  • status changed from reopened to new.

09/14/07 17:38:30 changed by jmelesky

  • attachment script_name_path_info.diff added.

try and accomodate request.path appropriately in mod_python and under wsgi, add request.full_path for script_name plus path

09/14/07 17:44:49 changed by jmelesky

  • status changed from new to assigned.

This patch was made against [6230] and has not been fully tested.

In order to accomodate mod_python, it has a new conf directive:

PythonOption? django.root /base/django/uri

So, if, for example, you're trying to run django in location /blog, you'd have something like:

<Location /blog>

... blah blah ... PythonOption? django.root /blog

</Location>

I'll attach a doc patch after i do some more testing.

09/14/07 17:46:22 changed by jmelesky

Oh, and this is being worked on as part of the Sprint14Sep

09/15/07 11:38:13 changed by jmelesky

  • attachment script_name_path_info2.diff added.

corrected and far more comprehensive patch, against [6292]

09/15/07 11:42:59 changed by jmelesky

  • needs_docs set to 1.

The prior patch was both limited and wrong. This patch corrects the bug and expands the expected behavior to include request.get_full_path(), as well as updating a number of internal references from request.path to request.full_path. It has been tested against mod_python, mod_wsgi on apache, and against mod_[fs]cgi on lighty.

I don't, however, know if this is enough. We may also need to update the behavior of {% url %}.

Also, though this is a big change, for existing mod_python installs it is not actually backwards-incompatible. Leaving out the PythonOption? django.root should retain the original behavior.

09/15/07 13:16:26 changed by jmelesky

  • attachment script_name_path_info3.diff added.

another patch revision, against [6300], including docs and suggestions from Malcom

09/15/07 14:30:48 changed by jmelesky

  • attachment script_name_path_info4.diff added.

Another revision, against [6316]. This one actually passes the test suite.

09/15/07 14:53:00 changed by jmelesky

  • needs_docs deleted.
  • version set to SVN.

There is one problem of note, here. If you're running, say, FastCGI, the SCRIPT_NAME gets populated back as 'blahblah.fcgi'. This wasn't an issue before. Now, though, redirects will point back to '/blahblah.fcgi/blah/blah' instead of '/blah/blah'. This will likely still work (unless the server is rewriting everything), but produces ugly and unexpected URLs.

Which points out a fundamental issue: SCRIPT_NAME + PATH_INFO is not the same thing as the URI that the user sees. I think more and more that these are related, but separate problems.

We end up with three distinct pieces of information:

  1. The absolute URL prefix (Apache's <Location> directive, "/mysite")
  2. The name of the callable thing (SCRIPT_NAME, "/user/django.fcgi")
  3. The remainder of the path ("/blog/tags/Sprint14Sep")

The first two might be different due to server URL rewriting.

The question, then, is: Do we want to support all three pieces of information? Because if we do, my patch is insufficient. I'm happy to continue work on this if y'all want to move in that direction, but this needs more design decision.

09/15/07 16:07:28 changed by jmelesky

  • stage changed from Accepted to Design decision needed.

09/22/07 14:50:21 changed by brosner

#5576 marked as a duplicate.

10/04/07 06:54:37 changed by anonymous

  • cc changed from sam@robots.org.uk to sam@robots.org.uk, remco@diji.biz.

11/30/07 15:11:58 changed by jacob

  • stage changed from Design decision needed to Ready for checkin.

Patch needs review (which Malcolm says he'll do), but otherwise this looks great.

11/30/07 16:26:26 changed by mtredinnick

  • owner changed from jmelesky to mtredinnick.
  • status changed from assigned to new.

01/24/08 18:33:27 changed by Rob Hudson <treborhudson@gmail.com>

I just got bit by this and wanted to verify what SmileyChris? saw with WSGI vs modpython, that req.uri under mod_python is the path and not req.path_info. Looks like this patch will fix it in this small case. Thanks.

02/19/08 11:18:34 changed by cwebber@imagescape.com

  • cc changed from sam@robots.org.uk, remco@diji.biz to sam@robots.org.uk, remco@diji.biz, cwebber@imagescape.com.

This patch is great, but it doesn't actually set the path in the wsgi request. Maybe this works with mod_python, but in our scgi environment at my work I'm not seeing it take any effect.

It appears to me that in wsgi.py after self.full_path is set up that this should actually become self.path, correct?

IE, afaict for this to take any effect, WSGIRequest's init_ should look like this:

class WSGIRequest(http.HttpRequest):
    def __init__(self, environ):
        self.environ = environ
        self.path = force_unicode(environ.get('PATH_INFO', '/'))
        self.full_path = (force_unicode(environ.get('SCRIPT_NAME', ''))
                          + force_unicode(environ.get('PATH_INFO', '/')))
        self.META = environ
        self.META['PATH_INFO'] = self.path
        self.META['SCRIPT_NAME'] = force_unicode(environ.get('SCRIPT_NAME', ''))
        self.method = environ['REQUEST_METHOD'].upper()
        self.path = self.full_path  ### <-- the added line

03/18/08 15:21:32 changed by guettli

  • cc changed from sam@robots.org.uk, remco@diji.biz, cwebber@imagescape.com to sam@robots.org.uk, remco@diji.biz, cwebber@imagescape.com, hv@tbz-pariv.de.

04/11/08 16:06:55 changed by ruddick@stanford.edu

I applied these changes, and they mostly fixed the problem, but the links in the upper right corner of the admin page (Documentation, Change Password, Log out) still do not use the full path. I do not know enough to fix the problem, so I will leave it to your expertise.


Add/Change #285 (WSGIRequest should set request.path to full uri path)




Change Properties
Action