404 with non-/ WSGI, script prefix not removed in core/urlresolvers.py: resolve()

[django@…]

Hello,

I've just had a nasty WSGI bug with Django 1.4 while using a non-/ WSGIScriptAlias with apache2. I tracked down the bug and found a cause so here is a patch for review, if you like.

Sorry in advance if this is a known and fixed bug, but then please consider patching 1.4 stable release?

The bug

Let:

urlpatterns = patterns('', r'^bug/$', 'app.views.Bug')

Django runserver => everything OK

apache2 + WSGIScriptAlias / => everything OK

apache2 + WSGIScriptAlias /test => breaks

Details: request URL '/test/bug/', tried '^bug/$' did not match current URL 'bug/' - silly right? :) obviously this is not reporting the actual URL it was checked against, maybe something to fix to have a more meaningful error message.

A quick debug of WSGI reveals apache is properly giving all the info to Django: SCRIPT_NAME='/test', PATH_INFO='/bug/'.

Proposed fix

After a quick search it seems to be a known bug on the interwebs but solutions I've seen were ugly (write the prefix statically in the app).

Diving into Django I found core/urlresolvers.py with:

class RegexURLResolver(LocaleRegexProvider):
[...]
    def resolve(self, path):
[...]
            new_path = path[match.end():] 
            for pattern in self.url_patterns:

With request path '/test/bug/', new_path becomes 'test/bug/', and this obviously does not match against '^bug/$'.

Proposed fix: if present, remove get_script_prefix() from new_path (after applying the same regexp).

             new_path = path[match.end():]
+            # Get script prefix, apply regexp and remove from path if present.
+            prefix = get_script_prefix()
+            if prefix:
+                match = self.regex.search(prefix)
+                if match:
+                    prefix = prefix[match.end():]
+                    if new_path.startswith(prefix):
+                        new_path = new_path[len(prefix):]
             for pattern in self.url_patterns:

(patch attached)

Now everything works as expected, I hope I did not miss anything.

Let me know if you have any questions and thanks for providing this great framework :)

Cheers,

StalkR

[django@…]

fix proposal #1 to strip script prefix in core/urlresolvers.py:resolve()

[StalkR]

Just checked, this also affects git HEAD (1.5.dev20120619153728) as the code is the same.

The proposed patch applies on HEAD and fixes the issue there too.

[Luke Plant]

Having dug into the code, I'm finding it difficult to believe this bug report. The only thing passed to the URL resolving mechanism is 'request.path_info':

​https://github.com/django/django/blob/4a103086d5c67fa4fcc53c106c9fdf644c742dd8/django/core/handlers/base.py#L102

This, in turn, comes straight from environ['PATH_INFO']:

​https://github.com/django/django/blob/4a103086d5c67fa4fcc53c106c9fdf644c742dd8/django/core/handlers/wsgi.py#L127

And you are saying that PATH_INFO is sent correctly by Apache/modwsgi, so I can't see what could be going wrong. It's difficult to see how new_path could ever be 'test/bug/' as you claim.

The proposed fix is certainly wrong - we don't need the script name at all at this point in the URL resolving process, it shouldn't be a part of the path that is being matched at all.

The only possible bug I can see so far is that the debugging page is not reporting the 'actual URL' it was testing. However, this is a difficult thing to express concisely, since you've got different definitions of the 'actual URL' - the PATH_INFO that Django is matching against (/bug/), the full URL from the client (/test/bug/), and the URL minus the initial / (bug/) which is used in the error message, due to the fact that the RegexURLResolver matches this initial slash automatically. The message is as helpful as it can be in the normal case.

However, if you could supply a test that uses the test client (which itself uses the WSGIRequest class) that would allow us to verify what you are talking about, or some other analysis so that we can see what is going on, I will certainly believe you. Please do re-open if you can provide this information, I'm closing INVALID for now.

[StalkR]

Thanks for the quick response. TL;DR: good news, it was my setup, sorry for the noise.

I tried to build a test client without success so I went back to my setup and code and eventually found the culprit: the 404 was triggered by a custom template tag I use to resolve the current URL to a view in order to know if a view is active, from a template.

Template tag:

{% active request 'app.views.Home' %}

Code:

@register.simple_tag
def active(request, view):
  if resolve(request.path).url_name == view:
    return 'active'
  else:
    return ''

The fix is obviously:

-  if resolve(request.path).url_name == view:
+  if resolve(request.path_info).url_name == view:

Also I'm not using reverse() because unfortunately some of my URLs regexp are not reversible because using conditional parameters.

So again, thanks and sorry for the noise.

How could we improve the debugging message? I really like the main exception handler giving you the entire call stack with locals etc. If this was also on 404, it would have helped me locate the bug in the template tags and not think it was in the main request handler. What do you think?

[Luke Plant]

I think this is probably a fairly obscure corner case, and it wouldn't be worth the complication to get 404 pages to include stack traces. Sorry!

[StalkR]

Ok, I understand. Thanks for your time!