BaseHandler does not handle resolver returning None

[xerolas]

django.core.handler.base.BaseHandler has the following call to the resolver:

            callback, callback_args, callback_kwargs = resolver.resolve(
	                    request.path_info)

However resolve() can return None (this happens to me when I get bogus HTTP requests from bots) and TypeError is not explicitly caught (for trying to unpack None).

The result is that 500 is returned instead of 404 and an email to the admins is sent (by default).

[Malcolm Tredinnick]

This is probably addressing the wrong end of the issue. As things stand, there is a problem. But it isn't that we aren't handling None. It's that we are seeing None in the first place.

We should be raising an exception (either Http404 or Resolver404 might be appropriate). It's an exceptional condition and should enter the normal code paths for handling non-existent URLs, not require special handling in the non-exceptional path.

[ccahoon]

I raise a Resolver404 if self.regex.search(path) returns None. This happens if path is not a regular expression. The unit test gives some examples of test cases that caused resolver to return None before the patch.

[ccahoon]

regression text and fix, updated to exclude the else statement

[Russell Keith-Magee]

(In [11120]) Fixed #10834 -- Added bucket condition to ensure that URL resolvers won't ever return None. Thanks to Chris Cahoon for the patch.

[mwelch]

This fixed breaks the debug screen. When setting up a new app and setting entries in the urls.py file, you will still get the default blue 'Welcome to django' screen instead of the 404 debug screen.

[anonymous]

Replying to mwelch:

This fixed breaks the debug screen. When setting up a new app and setting entries in the urls.py file, you will still get the default blue 'Welcome to django' screen instead of the 404 debug screen.

Please provide more information. Using r11126 I cannot recreate any problem getting the 404 debug screen to show up when I have DEBUG set to True and attempt to access a URL that has no mapping in my urlpatterns.

[Karen Tracey]

Sorry, anonymous at 7:01:33 was me. (Normally the spam blocker lets me know when my login has expired but not this time.)

[ccahoon]

That's very interesting. I was able to recreate showing the Welcome to Django screen if the following urlpattern is not covered:

r'^$'

However, the other urls still functioned properly. I looked around and it looks like it is because of this code in django/views/debug.py (this is on trunk r11126):

244 def technical_404_response(request, exception):
245     "Create a technical 404 error response. The exception should be the     
    Http404."
246     try:
247         tried = exception.args[0]['tried']
248     except (IndexError, TypeError):
249         tried = []
250     else:
251         if not tried:
252             # tried exists but is an empty list. The URLconf must've been   
    empty.
253             return empty_urlconf(request)

The content of the changeset (which was different from my patch) creates an empty "tried" list in the dictionary returned in the Resolver404 exception. Thus, debug thinks there is nothing in the urlconf. Attaching a patch.

I am pretty sure I ran into this issue when creating the fix -- only by not even having "tried" in the dictionary does the 404 behave properly. Now I am not sure if this is correct overall behavior, but for a patch this small, it seemed appropriate to just leave it out.

[Russell Keith-Magee]

@ccahoon: There must be something else going on here, because removing tried from the dictionary causes mass test failures in the test_client and test_client_regress unit tests. Your analysis regarding why tried should be omitted looks sound, though - we'll have to dig deeper to find the cause of the failures in the test_client tests.

[ccahoon]

@russellm: I've had very recent experience with test_client_regress expecting behavior that is consistent with Django but not necessarily correct. I will take a look and see what is going on later tonight.

[ccahoon]

Updated to pass regression tests.

[ccahoon]

That patch passes the test_client_regress tests and seems to behave properly in real use. The machine I made the change on can't handle running the full test suite at the moment, so I do not know about other regression tests.

[ccahoon]

The patch above passes the regression test suite. @russellm, what do you think of the solution?

[Russell Keith-Magee]

@ccahoon - Not thrilled. The format of the error message with the provided patch is completely misleading. I suspect the patch will need to be more than a single line change - looking at the code, there may be a need to tweak the 404 template to differentiate between the error case described here, and the "new project, no URL's" message.

[Russell Keith-Magee]

(In [11156]) [1.0.X] Fixed #10834 -- Corrected [11120] to ensure that there is a difference between catching a bad URL pattern and an new (no URLs) project. Thanks to Matt Welch for the report.

Merge of r11155 from trunk.

[Russell Keith-Magee]

(In [11155]) Fixed #10834 -- Corrected [11120] to ensure that there is a difference between catching a bad URL pattern and an new (no URLs) project. Thanks to Matt Welch for the report.

[Jacob]

Milestone 1.1 deleted