From c9f1a12925ef18d82b5f9683d589bb2c1e067650 Mon Sep 17 00:00:00 2001 From: Bas Peschier Date: Sun, 8 Mar 2015 11:07:55 +0100 Subject: [PATCH] Fixed #24013 -- Fixed escaping of reverse() prefix. Prefix was treated as a part of the url pattern, which it is not. Improved tests to conform with RFC 3986 which allows certain characters in path segments without being escaped. --- django/core/urlresolvers.py | 15 ++++++--------- tests/urlpatterns_reverse/tests.py | 23 ++++++++++++++++++----- 2 files changed, 24 insertions(+), 14 deletions(-) diff --git a/django/core/urlresolvers.py b/django/core/urlresolvers.py index 76b12c1ee4..59114795bc 100644 --- a/django/core/urlresolvers.py +++ b/django/core/urlresolvers.py @@ -453,16 +453,15 @@ class RegexURLResolver(LocaleRegexProvider): ) possibilities = self.reverse_dict.getlist(lookup_view) - prefix_norm, prefix_args = normalize(urlquote(_prefix))[0] for possibility, pattern, defaults in possibilities: for result, params in possibility: if args: - if len(args) != len(params) + len(prefix_args): + if len(args) != len(params): continue - candidate_subs = dict(zip(prefix_args + params, text_args)) + candidate_subs = dict(zip(params, text_args)) else: if (set(kwargs.keys()) | set(defaults.keys()) != set(params) | - set(defaults.keys()) | set(prefix_args)): + set(defaults.keys())): continue matches = True for k, v in defaults.items(): @@ -477,12 +476,10 @@ class RegexURLResolver(LocaleRegexProvider): # without quoting to build a decoded URL and look for a match. # Then, if we have a match, redo the substitution with quoted # arguments in order to return a properly encoded URL. - candidate_pat = prefix_norm.replace('%', '%%') + result - if re.search('^%s%s' % (prefix_norm, pattern), candidate_pat % candidate_subs, re.UNICODE): + candidate_pat = _prefix.replace('%', '%%') + result + if re.search('^%s%s' % (re.escape(_prefix), pattern), candidate_pat % candidate_subs, re.UNICODE): # safe characters from `pchar` definition of RFC 3986 - candidate_subs = dict((k, urlquote(v, safe=RFC3986_SUBDELIMS + str('/~:@'))) - for (k, v) in candidate_subs.items()) - url = candidate_pat % candidate_subs + url = urlquote(candidate_pat % candidate_subs, safe=RFC3986_SUBDELIMS + str('/~:@')) # Don't allow construction of scheme relative urls. if url.startswith('//'): url = '/%%2F%s' % url[2:] diff --git a/tests/urlpatterns_reverse/tests.py b/tests/urlpatterns_reverse/tests.py index 7d08cd21e9..fa553fc6eb 100644 --- a/tests/urlpatterns_reverse/tests.py +++ b/tests/urlpatterns_reverse/tests.py @@ -202,17 +202,24 @@ class URLPatternReverse(TestCase): reverse('non_path_include', prefix='/{{invalid}}/')) def test_prefix_parenthesis(self): - self.assertEqual('/bogus%29/includes/non_path_include/', - reverse('non_path_include', prefix='/bogus)/')) + # Parentheses are allowed and should not cause errors or be escaped + self.assertEqual( + '/bogus)/includes/non_path_include/', + reverse('non_path_include', prefix='/bogus)/') + ) + self.assertEqual( + '/(bogus)/includes/non_path_include/', + reverse('non_path_include', prefix='/(bogus)/') + ) def test_prefix_format_char(self): self.assertEqual('/bump%2520map/includes/non_path_include/', reverse('non_path_include', prefix='/bump%20map/')) def test_non_urlsafe_prefix_with_args(self): - # Regression for #20022 - self.assertEqual('/%7Eme/places/1/', - reverse('places', args=[1], prefix='/~me/')) + # Regression for #20022, adjusted for #24013 because ~ is an unreserved + # character. Tests whether % is escaped. + self.assertEqual('/%257Eme/places/1/', reverse('places', args=[1], prefix='/%7Eme/')) def test_patterns_reported(self): # Regression for #17076 @@ -227,6 +234,12 @@ class URLPatternReverse(TestCase): # exception self.fail("Expected a NoReverseMatch, but none occurred.") + def test_script_name_escaping(self): + self.assertEqual( + reverse('optional', args=['foo:bar'], prefix='/script:name/'), + '/script:name/optional/foo:bar/' + ) + def test_reverse_returns_unicode(self): name, expected, args, kwargs = test_data[0] self.assertIsInstance(