From 79558c787ebfd0fe723acb061a375b19a27f18cd Mon Sep 17 00:00:00 2001 From: Grzegorz Nosek Date: Sat, 15 Feb 2014 14:41:01 +0100 Subject: [PATCH] Fixed #18373 - improved handling of Resolver404s from views When django.core.urlresolvers.resolve was called from a view, failed and the exception was propagated and rendered by technical_404_response, the URL mentioned on the page was the current URL instead of the URL passed to resolve(). Fixed by using the path attribute from the Resolver404 exception instead of request.path_info. Also cleaned up the exceptions to use standard named parameters instead of stuffing a dict in args[0] --- django/core/urlresolvers.py | 11 +++++++---- django/views/debug.py | 8 +++++--- tests/urlpatterns_reverse/tests.py | 8 ++++---- tests/view_tests/tests/test_debug.py | 8 ++++++++ tests/view_tests/views.py | 2 +- 5 files changed, 25 insertions(+), 12 deletions(-) diff --git a/django/core/urlresolvers.py b/django/core/urlresolvers.py index 1dca15b4aa..e9bb441b3d 100644 --- a/django/core/urlresolvers.py +++ b/django/core/urlresolvers.py @@ -69,8 +69,11 @@ class ResolverMatch(object): class Resolver404(Http404): - pass + def __init__(self, path, tried=None): + super(Resolver404, self).__init__() + self.path = path + self.tried = tried class NoReverseMatch(Exception): pass @@ -322,7 +325,7 @@ class RegexURLResolver(LocaleRegexProvider): try: sub_match = pattern.resolve(new_path) except Resolver404 as e: - sub_tried = e.args[0].get('tried') + sub_tried = e.tried if sub_tried is not None: tried.extend([pattern] + t for t in sub_tried) else: @@ -333,8 +336,8 @@ class RegexURLResolver(LocaleRegexProvider): sub_match_dict.update(sub_match.kwargs) return ResolverMatch(sub_match.func, sub_match.args, sub_match_dict, sub_match.url_name, self.app_name or sub_match.app_name, [self.namespace] + sub_match.namespaces) tried.append([pattern]) - raise Resolver404({'tried': tried, 'path': new_path}) - raise Resolver404({'path': path}) + raise Resolver404(new_path, tried=tried) + raise Resolver404(path) @property def urlconf_module(self): diff --git a/django/views/debug.py b/django/views/debug.py index d665c4e36b..2fb15ac34f 100644 --- a/django/views/debug.py +++ b/django/views/debug.py @@ -475,9 +475,11 @@ class ExceptionReporter(object): def technical_404_response(request, exception): "Create a technical 404 error response. The exception should be the Http404." try: - tried = exception.args[0]['tried'] - except (IndexError, TypeError, KeyError): + tried = exception.tried + error_url = exception.path + except AttributeError: tried = [] + error_url = request.path_info[1:] # Trim leading slash else: if (not tried # empty URLconf or (request.path == '/' @@ -494,7 +496,7 @@ def technical_404_response(request, exception): c = Context({ 'urlconf': urlconf, 'root_urlconf': settings.ROOT_URLCONF, - 'request_path': request.path_info[1:], # Trim leading slash + 'request_path': error_url, 'urlpatterns': tried, 'reason': force_bytes(exception, errors='replace'), 'request': request, diff --git a/tests/urlpatterns_reverse/tests.py b/tests/urlpatterns_reverse/tests.py index a7fb342da7..6b0d5f37c6 100644 --- a/tests/urlpatterns_reverse/tests.py +++ b/tests/urlpatterns_reverse/tests.py @@ -272,10 +272,10 @@ class ResolverTests(unittest.TestCase): self.fail('resolve did not raise a 404') except Resolver404 as e: # make sure we at least matched the root ('/') url resolver: - self.assertTrue('tried' in e.args[0]) - tried = e.args[0]['tried'] - self.assertEqual(len(e.args[0]['tried']), len(url_types_names), 'Wrong number of tried URLs returned. Expected %s, got %s.' % (len(url_types_names), len(e.args[0]['tried']))) - for tried, expected in zip(e.args[0]['tried'], url_types_names): + self.assertTrue(hasattr(e, 'tried')) + tried = e.tried + self.assertEqual(len(tried), len(url_types_names), 'Wrong number of tried URLs returned. Expected %s, got %s.' % (len(url_types_names), len(tried))) + for tried, expected in zip(tried, url_types_names): for t, e in zip(tried, expected): self.assertIsInstance(t, e['type']), str('%s is not an instance of %s') % (t, e['type']) if 'name' in e: diff --git a/tests/view_tests/tests/test_debug.py b/tests/view_tests/tests/test_debug.py index 44e70347bd..b66bdac32d 100644 --- a/tests/view_tests/tests/test_debug.py +++ b/tests/view_tests/tests/test_debug.py @@ -69,6 +69,14 @@ class DebugViewTests(TestCase): response = self.client.get('/raises404/') self.assertEqual(response.status_code, 404) + def test_raised_404(self): + response = self.client.get('/views/raises404/') + self.assertContains(response, "not-in-urls, didn't match", status_code=404) + + def test_404_not_in_urls(self): + response = self.client.get('/not-in-urls') + self.assertContains(response, "not-in-urls, didn't match", status_code=404) + def test_view_exceptions(self): for n in range(len(except_args)): self.assertRaises(BrokenException, self.client.get, diff --git a/tests/view_tests/views.py b/tests/view_tests/views.py index e0f6fdb753..2e8a270e94 100644 --- a/tests/view_tests/views.py +++ b/tests/view_tests/views.py @@ -57,7 +57,7 @@ def raises403(request): def raises404(request): resolver = get_resolver(None) - resolver.resolve('') + resolver.resolve('/not-in-urls') def redirect(request):