From b579350cd16601a82efd6a497fed07f086371817 Mon Sep 17 00:00:00 2001 From: Russell Keith-Magee Date: Sat, 10 Apr 2010 07:35:31 +0000 Subject: [PATCH] Fixed #13275 -- Modified the parsing logic of the {% url %} tag to avoid catastrophic backtracking. Thanks to SmileyChris for the patch. git-svn-id: http://code.djangoproject.com/svn/django/trunk@12943 bcc190cf-cafb-0310-a4f2-bffc1f526a37 --- django/template/defaulttags.py | 41 ++++++++++++++++++------ tests/regressiontests/templates/tests.py | 12 +++++++ tests/regressiontests/templates/urls.py | 3 +- 3 files changed, 45 insertions(+), 11 deletions(-) diff --git a/django/template/defaulttags.py b/django/template/defaulttags.py index 6d77eeddd7..9dd5fb766f 100644 --- a/django/template/defaulttags.py +++ b/django/template/defaulttags.py @@ -1065,10 +1065,6 @@ def templatetag(parser, token): return TemplateTagNode(tag) templatetag = register.tag(templatetag) -# Backwards compatibility check which will fail against for old comma -# separated arguments in the url tag. -url_backwards_re = re.compile(r'''(('[^']*'|"[^"]*"|[^,]+)=?)+$''') - def url(parser, token): """ Returns an absolute URL matching given view with its parameters. @@ -1117,13 +1113,38 @@ def url(parser, token): asvar = bits[-1] bits = bits[:-2] - # Backwards compatibility: {% url urlname arg1,arg2 %} or - # {% url urlname arg1,arg2 as foo %} cases. - if bits: - old_args = ''.join(bits) - if not url_backwards_re.match(old_args): - bits = old_args.split(",") + # Backwards compatibility: check for the old comma separated format + # {% url urlname arg1,arg2 %} + # Initial check - that the first space separated bit has a comma in it + if bits and ',' in bits[0]: + check_old_format = True + # In order to *really* be old format, there must be a comma + # in *every* space separated bit, except the last. + for bit in bits[1:-1]: + if ',' not in bit: + # No comma in this bit. Either the comma we found + # in bit 1 was a false positive (e.g., comma in a string), + # or there is a syntax problem with missing commas + check_old_format = False + break + else: + # No comma found - must be new format. + check_old_format = False + if check_old_format: + # Confirm that this is old format by trying to parse the first + # argument. An exception will be raised if the comma is + # unexpected (i.e. outside of a static string). + match = kwarg_re.match(bits[0]) + if match: + value = match.groups()[1] + try: + parser.compile_filter(value) + except TemplateSyntaxError: + bits = ''.join(bits).split(',') + + # Now all the bits are parsed into new format, + # process them as template vars if len(bits): for bit in bits: match = kwarg_re.match(bit) diff --git a/tests/regressiontests/templates/tests.py b/tests/regressiontests/templates/tests.py index 2df02f69be..93ffb4da67 100644 --- a/tests/regressiontests/templates/tests.py +++ b/tests/regressiontests/templates/tests.py @@ -364,8 +364,12 @@ class Templates(unittest.TestCase): settings.TEMPLATE_STRING_IF_INVALID = invalid_str for is_cached in (False, True): try: + start = datetime.now() test_template = loader.get_template(name) + end = datetime.now() output = self.render(test_template, vals) + if end-start > timedelta(seconds=0.2): + failures.append("Template test (Cached='%s', TEMPLATE_STRING_IF_INVALID='%s'): %s -- FAILED. Took too long to parse test" % (is_cached, invalid_str, name)) except ContextStackException: failures.append("Template test (Cached='%s', TEMPLATE_STRING_IF_INVALID='%s'): %s -- FAILED. Context stack was left imbalanced" % (is_cached, invalid_str, name)) continue @@ -1175,13 +1179,20 @@ class Templates(unittest.TestCase): # Successes 'legacyurl02': ('{% url regressiontests.templates.views.client_action id=client.id,action="update" %}', {'client': {'id': 1}}, '/url_tag/client/1/update/'), 'legacyurl02a': ('{% url regressiontests.templates.views.client_action client.id,"update" %}', {'client': {'id': 1}}, '/url_tag/client/1/update/'), + 'legacyurl02b': ("{% url regressiontests.templates.views.client_action id=client.id,action='update' %}", {'client': {'id': 1}}, '/url_tag/client/1/update/'), + 'legacyurl02c': ("{% url regressiontests.templates.views.client_action client.id,'update' %}", {'client': {'id': 1}}, '/url_tag/client/1/update/'), 'legacyurl10': ('{% url regressiontests.templates.views.client_action id=client.id,action="two words" %}', {'client': {'id': 1}}, '/url_tag/client/1/two%20words/'), 'legacyurl13': ('{% url regressiontests.templates.views.client_action id=client.id, action=arg|join:"-" %}', {'client': {'id': 1}, 'arg':['a','b']}, '/url_tag/client/1/a-b/'), 'legacyurl14': ('{% url regressiontests.templates.views.client_action client.id, arg|join:"-" %}', {'client': {'id': 1}, 'arg':['a','b']}, '/url_tag/client/1/a-b/'), + 'legacyurl16': ('{% url regressiontests.templates.views.client_action action="update",id="1" %}', {}, '/url_tag/client/1/update/'), + 'legacyurl16a': ("{% url regressiontests.templates.views.client_action action='update',id='1' %}", {}, '/url_tag/client/1/update/'), + 'legacyurl17': ('{% url regressiontests.templates.views.client_action client_id=client.my_id,action=action %}', {'client': {'my_id': 1}, 'action': 'update'}, '/url_tag/client/1/update/'), 'url01': ('{% url regressiontests.templates.views.client client.id %}', {'client': {'id': 1}}, '/url_tag/client/1/'), 'url02': ('{% url regressiontests.templates.views.client_action id=client.id action="update" %}', {'client': {'id': 1}}, '/url_tag/client/1/update/'), 'url02a': ('{% url regressiontests.templates.views.client_action client.id "update" %}', {'client': {'id': 1}}, '/url_tag/client/1/update/'), + 'url02b': ("{% url regressiontests.templates.views.client_action id=client.id action='update' %}", {'client': {'id': 1}}, '/url_tag/client/1/update/'), + 'url02c': ("{% url regressiontests.templates.views.client_action client.id 'update' %}", {'client': {'id': 1}}, '/url_tag/client/1/update/'), 'url03': ('{% url regressiontests.templates.views.index %}', {}, '/url_tag/'), 'url04': ('{% url named.client client.id %}', {'client': {'id': 1}}, '/url_tag/named-client/1/'), 'url05': (u'{% url метка_оператора v %}', {'v': u'Ω'}, '/url_tag/%D0%AE%D0%BD%D0%B8%D0%BA%D0%BE%D0%B4/%CE%A9/'), @@ -1195,6 +1206,7 @@ class Templates(unittest.TestCase): 'url13': ('{% url regressiontests.templates.views.client_action id=client.id action=arg|join:"-" %}', {'client': {'id': 1}, 'arg':['a','b']}, '/url_tag/client/1/a-b/'), 'url14': ('{% url regressiontests.templates.views.client_action client.id arg|join:"-" %}', {'client': {'id': 1}, 'arg':['a','b']}, '/url_tag/client/1/a-b/'), 'url15': ('{% url regressiontests.templates.views.client_action 12 "test" %}', {}, '/url_tag/client/12/test/'), + 'url18': ('{% url regressiontests.templates.views.client "1,2" %}', {}, '/url_tag/client/1,2/'), # Failures 'url-fail01': ('{% url %}', {}, template.TemplateSyntaxError), diff --git a/tests/regressiontests/templates/urls.py b/tests/regressiontests/templates/urls.py index b7320e5295..28d4133228 100644 --- a/tests/regressiontests/templates/urls.py +++ b/tests/regressiontests/templates/urls.py @@ -6,8 +6,9 @@ urlpatterns = patterns('', # Test urls for testing reverse lookups (r'^$', views.index), - (r'^client/(\d+)/$', views.client), + (r'^client/([\d,]+)/$', views.client), (r'^client/(?P\d+)/(?P[^/]+)/$', views.client_action), + (r'^client/(?P\d+)/(?P[^/]+)/$', views.client_action), url(r'^named-client/(\d+)/$', views.client2, name="named.client"), # Unicode strings are permitted everywhere.