mirror of
				https://github.com/django/django.git
				synced 2025-11-04 13:39:16 +00:00 
			
		
		
		
	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]
This commit is contained in:
		
							parent
							
								
									8bbdcc76e4
								
							
						
					
					
						commit
						79558c787e
					
				
					 5 changed files with 25 additions and 12 deletions
				
			
		| 
						 | 
					@ -69,8 +69,11 @@ class ResolverMatch(object):
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					
 | 
				
			||||||
class Resolver404(Http404):
 | 
					class Resolver404(Http404):
 | 
				
			||||||
    pass
 | 
					 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    def __init__(self, path, tried=None):
 | 
				
			||||||
 | 
					        super(Resolver404, self).__init__()
 | 
				
			||||||
 | 
					        self.path = path
 | 
				
			||||||
 | 
					        self.tried = tried
 | 
				
			||||||
 | 
					
 | 
				
			||||||
class NoReverseMatch(Exception):
 | 
					class NoReverseMatch(Exception):
 | 
				
			||||||
    pass
 | 
					    pass
 | 
				
			||||||
| 
						 | 
					@ -322,7 +325,7 @@ class RegexURLResolver(LocaleRegexProvider):
 | 
				
			||||||
                try:
 | 
					                try:
 | 
				
			||||||
                    sub_match = pattern.resolve(new_path)
 | 
					                    sub_match = pattern.resolve(new_path)
 | 
				
			||||||
                except Resolver404 as e:
 | 
					                except Resolver404 as e:
 | 
				
			||||||
                    sub_tried = e.args[0].get('tried')
 | 
					                    sub_tried = e.tried
 | 
				
			||||||
                    if sub_tried is not None:
 | 
					                    if sub_tried is not None:
 | 
				
			||||||
                        tried.extend([pattern] + t for t in sub_tried)
 | 
					                        tried.extend([pattern] + t for t in sub_tried)
 | 
				
			||||||
                    else:
 | 
					                    else:
 | 
				
			||||||
| 
						 | 
					@ -333,8 +336,8 @@ class RegexURLResolver(LocaleRegexProvider):
 | 
				
			||||||
                        sub_match_dict.update(sub_match.kwargs)
 | 
					                        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)
 | 
					                        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])
 | 
					                    tried.append([pattern])
 | 
				
			||||||
            raise Resolver404({'tried': tried, 'path': new_path})
 | 
					            raise Resolver404(new_path, tried=tried)
 | 
				
			||||||
        raise Resolver404({'path': path})
 | 
					        raise Resolver404(path)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    @property
 | 
					    @property
 | 
				
			||||||
    def urlconf_module(self):
 | 
					    def urlconf_module(self):
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -475,9 +475,11 @@ class ExceptionReporter(object):
 | 
				
			||||||
def technical_404_response(request, exception):
 | 
					def technical_404_response(request, exception):
 | 
				
			||||||
    "Create a technical 404 error response. The exception should be the Http404."
 | 
					    "Create a technical 404 error response. The exception should be the Http404."
 | 
				
			||||||
    try:
 | 
					    try:
 | 
				
			||||||
        tried = exception.args[0]['tried']
 | 
					        tried = exception.tried
 | 
				
			||||||
    except (IndexError, TypeError, KeyError):
 | 
					        error_url = exception.path
 | 
				
			||||||
 | 
					    except AttributeError:
 | 
				
			||||||
        tried = []
 | 
					        tried = []
 | 
				
			||||||
 | 
					        error_url = request.path_info[1:] # Trim leading slash
 | 
				
			||||||
    else:
 | 
					    else:
 | 
				
			||||||
        if (not tried                           # empty URLconf
 | 
					        if (not tried                           # empty URLconf
 | 
				
			||||||
            or (request.path == '/'
 | 
					            or (request.path == '/'
 | 
				
			||||||
| 
						 | 
					@ -494,7 +496,7 @@ def technical_404_response(request, exception):
 | 
				
			||||||
    c = Context({
 | 
					    c = Context({
 | 
				
			||||||
        'urlconf': urlconf,
 | 
					        'urlconf': urlconf,
 | 
				
			||||||
        'root_urlconf': settings.ROOT_URLCONF,
 | 
					        'root_urlconf': settings.ROOT_URLCONF,
 | 
				
			||||||
        'request_path': request.path_info[1:],  # Trim leading slash
 | 
					        'request_path': error_url,
 | 
				
			||||||
        'urlpatterns': tried,
 | 
					        'urlpatterns': tried,
 | 
				
			||||||
        'reason': force_bytes(exception, errors='replace'),
 | 
					        'reason': force_bytes(exception, errors='replace'),
 | 
				
			||||||
        'request': request,
 | 
					        'request': request,
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -272,10 +272,10 @@ class ResolverTests(unittest.TestCase):
 | 
				
			||||||
            self.fail('resolve did not raise a 404')
 | 
					            self.fail('resolve did not raise a 404')
 | 
				
			||||||
        except Resolver404 as e:
 | 
					        except Resolver404 as e:
 | 
				
			||||||
            # make sure we at least matched the root ('/') url resolver:
 | 
					            # make sure we at least matched the root ('/') url resolver:
 | 
				
			||||||
            self.assertTrue('tried' in e.args[0])
 | 
					            self.assertTrue(hasattr(e, 'tried'))
 | 
				
			||||||
            tried = e.args[0]['tried']
 | 
					            tried = e.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'])))
 | 
					            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(e.args[0]['tried'], url_types_names):
 | 
					            for tried, expected in zip(tried, url_types_names):
 | 
				
			||||||
                for t, e in zip(tried, expected):
 | 
					                for t, e in zip(tried, expected):
 | 
				
			||||||
                    self.assertIsInstance(t, e['type']), str('%s is not an instance of %s') % (t, e['type'])
 | 
					                    self.assertIsInstance(t, e['type']), str('%s is not an instance of %s') % (t, e['type'])
 | 
				
			||||||
                    if 'name' in e:
 | 
					                    if 'name' in e:
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -69,6 +69,14 @@ class DebugViewTests(TestCase):
 | 
				
			||||||
        response = self.client.get('/raises404/')
 | 
					        response = self.client.get('/raises404/')
 | 
				
			||||||
        self.assertEqual(response.status_code, 404)
 | 
					        self.assertEqual(response.status_code, 404)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    def test_raised_404(self):
 | 
				
			||||||
 | 
					        response = self.client.get('/views/raises404/')
 | 
				
			||||||
 | 
					        self.assertContains(response, "<code>not-in-urls</code>, didn't match", status_code=404)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    def test_404_not_in_urls(self):
 | 
				
			||||||
 | 
					        response = self.client.get('/not-in-urls')
 | 
				
			||||||
 | 
					        self.assertContains(response, "<code>not-in-urls</code>, didn't match", status_code=404)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    def test_view_exceptions(self):
 | 
					    def test_view_exceptions(self):
 | 
				
			||||||
        for n in range(len(except_args)):
 | 
					        for n in range(len(except_args)):
 | 
				
			||||||
            self.assertRaises(BrokenException, self.client.get,
 | 
					            self.assertRaises(BrokenException, self.client.get,
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -57,7 +57,7 @@ def raises403(request):
 | 
				
			||||||
 | 
					
 | 
				
			||||||
def raises404(request):
 | 
					def raises404(request):
 | 
				
			||||||
    resolver = get_resolver(None)
 | 
					    resolver = get_resolver(None)
 | 
				
			||||||
    resolver.resolve('')
 | 
					    resolver.resolve('/not-in-urls')
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					
 | 
				
			||||||
def redirect(request):
 | 
					def redirect(request):
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue