From b4382b7055fc8b0078cbb50ed9c3f924635d9971 Mon Sep 17 00:00:00 2001 From: Bas Peschier Date: Sat, 21 Mar 2015 13:19:13 +0100 Subject: [PATCH] Fixed #16362 -- Allowed lookaround assertions in URL patterns. --- django/utils/regex_helper.py | 11 ++++--- docs/releases/1.9.txt | 5 ++++ docs/spelling_wordlist | 1 + tests/urlpatterns_reverse/tests.py | 47 ++++++++++++++++++++++++++++++ tests/urlpatterns_reverse/urls.py | 4 +++ 5 files changed, 62 insertions(+), 6 deletions(-) diff --git a/django/utils/regex_helper.py b/django/utils/regex_helper.py index 53736dffa77..4a697b29200 100644 --- a/django/utils/regex_helper.py +++ b/django/utils/regex_helper.py @@ -59,11 +59,10 @@ def normalize(pattern): (3) Select the first (essentially an arbitrary) element from any character class. Select an arbitrary character for any unordered class (e.g. '.' or '\w') in the pattern. - (5) Ignore comments and any of the reg-exp flags that won't change - what we construct ("iLmsu"). "(?x)" is an error, however. - (6) Raise an error on all other non-capturing (?...) forms (e.g. - look-ahead and look-behind matches) and any disjunctive ('|') - constructs. + (4) Ignore comments, look-ahead and look-behind assertions, and any of the + reg-exp flags that won't change what we construct ("iLmsu"). "(?x)" is + an error, however. + (5) Raise an error on any disjunctive ('|') constructs. Django's URLs for forward resolving are either all positional arguments or all keyword arguments. That is assumed here, as well. Although reverse @@ -129,7 +128,7 @@ def normalize(pattern): walk_to_end(ch, pattern_iter) else: ch, escaped = next(pattern_iter) - if ch in "iLmsu#": + if ch in "iLmsu#!=<": # All of these are ignorable. Walk to the end of the # group. walk_to_end(ch, pattern_iter) diff --git a/docs/releases/1.9.txt b/docs/releases/1.9.txt index 3fa068c97fb..10a4db55182 100644 --- a/docs/releases/1.9.txt +++ b/docs/releases/1.9.txt @@ -205,6 +205,11 @@ Tests * ... +URLs +^^^^ + +* Regular expression lookaround assertions are now allowed in URL patterns. + Validators ^^^^^^^^^^ diff --git a/docs/spelling_wordlist b/docs/spelling_wordlist index 78480713fb9..cf9a71db973 100644 --- a/docs/spelling_wordlist +++ b/docs/spelling_wordlist @@ -436,6 +436,7 @@ Login logout Logout Loïc +lookaround lookup Lookup lookups diff --git a/tests/urlpatterns_reverse/tests.py b/tests/urlpatterns_reverse/tests.py index eca5bc1e645..8706c64b64c 100644 --- a/tests/urlpatterns_reverse/tests.py +++ b/tests/urlpatterns_reverse/tests.py @@ -799,3 +799,50 @@ class IncludeTests(SimpleTestCase): msg = "Must specify a namespace if specifying app_name." with self.assertRaisesMessage(ValueError, msg): include('urls', app_name='bar') + + +@override_settings(ROOT_URLCONF='urlpatterns_reverse.urls') +class LookaheadTests(TestCase): + def test_valid_resolve(self): + test_urls = [ + '/lookahead-/a-city/', + '/lookbehind-/a-city/', + '/lookahead+/a-city/', + '/lookbehind+/a-city/', + ] + for test_url in test_urls: + match = resolve(test_url) + self.assertEqual(match.kwargs, {'city': 'a-city'}) + + def test_invalid_resolve(self): + test_urls = [ + '/lookahead-/not-a-city/', + '/lookbehind-/not-a-city/', + '/lookahead+/other-city/', + '/lookbehind+/other-city/', + ] + for test_url in test_urls: + with self.assertRaises(Resolver404): + resolve(test_url) + + def test_valid_reverse(self): + url = reverse('lookahead-positive', kwargs={'city': 'a-city'}) + self.assertEqual(url, '/lookahead+/a-city/') + url = reverse('lookahead-negative', kwargs={'city': 'a-city'}) + self.assertEqual(url, '/lookahead-/a-city/') + + url = reverse('lookbehind-positive', kwargs={'city': 'a-city'}) + self.assertEqual(url, '/lookbehind+/a-city/') + url = reverse('lookbehind-negative', kwargs={'city': 'a-city'}) + self.assertEqual(url, '/lookbehind-/a-city/') + + def test_invalid_reverse(self): + with self.assertRaises(NoReverseMatch): + reverse('lookahead-positive', kwargs={'city': 'other-city'}) + with self.assertRaises(NoReverseMatch): + reverse('lookahead-negative', kwargs={'city': 'not-a-city'}) + + with self.assertRaises(NoReverseMatch): + reverse('lookbehind-positive', kwargs={'city': 'other-city'}) + with self.assertRaises(NoReverseMatch): + reverse('lookbehind-negative', kwargs={'city': 'not-a-city'}) diff --git a/tests/urlpatterns_reverse/urls.py b/tests/urlpatterns_reverse/urls.py index d6b692d220a..25609c14cfe 100644 --- a/tests/urlpatterns_reverse/urls.py +++ b/tests/urlpatterns_reverse/urls.py @@ -62,6 +62,10 @@ with warnings.catch_warnings(): url(r'^outer/(?P[0-9]+)/', include('urlpatterns_reverse.included_urls')), url(r'^outer-no-kwargs/([0-9]+)/', include('urlpatterns_reverse.included_no_kwargs_urls')), url('', include('urlpatterns_reverse.extra_urls')), + url(r'^lookahead-/(?!not-a-city)(?P[^/]+)/$', empty_view, name='lookahead-negative'), + url(r'^lookahead\+/(?=a-city)(?P[^/]+)/$', empty_view, name='lookahead-positive'), + url(r'^lookbehind-/(?P[^/]+)(?[^/]+)(?<=a-city)/$', empty_view, name='lookbehind-positive'), # Partials should be fine. url(r'^partial/', empty_view_partial, name="partial"),