Fixed #12945 -- Corrected the parsing of arguments in {% url %} when the argument list has spaces between commas. This is a revised version of r12503, which was a fix for #12072. Thanks to SmileyChris for the patch, and to dmoisset for finding all the places in the docs that the old style syntax was used.

git-svn-id: http://code.djangoproject.com/svn/django/trunk@12889 bcc190cf-cafb-0310-a4f2-bffc1f526a37

django/contrib/admin/templates/registration/password_reset_email.html

@@ -4,7 +4,7 @@
 
 {% trans "Please go to the following page and choose a new password:" %}
 {% block reset_link %}
-{{ protocol }}://{{ domain }}{% url django.contrib.auth.views.password_reset_confirm uidb36=uid, token=token %}
+{{ protocol }}://{{ domain }}{% url django.contrib.auth.views.password_reset_confirm uidb36=uid token=token %}
 {% endblock %}
 {% trans "Your username, in case you've forgotten:" %} {{ user.username }}
 

django/template/defaulttags.py

@@ -14,6 +14,8 @@ from django.utils.itercompat import groupby
 from django.utils.safestring import mark_safe
 
 register = Library()
+# Regex for token keyword arguments
+kwarg_re = re.compile(r"(?:(\w+)=)?(.+)")
 
 class AutoEscapeControlNode(Node):
     """Implements the actions of the autoescape tag."""
@@ -1063,12 +1065,9 @@ def templatetag(parser, token):
     return TemplateTagNode(tag)
 templatetag = register.tag(templatetag)
 
-# Regex for URL arguments including filters
+# Backwards compatibility check which will fail against for old comma
-url_arg_re = re.compile(
+# separated arguments in the url tag.
-    r"(?:(%(name)s)=)?(%(value)s(?:\|%(name)s(?::%(value)s)?)*)" % {
+url_backwards_re = re.compile(r'''(('[^']*'|"[^"]*"|[^,]+)=?)+$''')
-        'name':'\w+',
-        'value':'''(?:(?:'[^']*')|(?:"[^"]*")|(?:[\w\.-]+))'''},
-    re.VERBOSE)
 
 def url(parser, token):
     """
@@ -1077,7 +1076,11 @@ def url(parser, token):
     This is a way to define links that aren't tied to a particular URL
     configuration::
 
-        {% url path.to.some_view arg1,arg2,name1=value1 %}
+        {% url path.to.some_view arg1 arg2 %}
+
+        or
+
+        {% url path.to.some_view name1=value1 name2=value2 %}
 
     The first argument is a path to a view. It can be an absolute python path
     or just ``app_name.view_name`` without the project name if the view is
@@ -1109,27 +1112,28 @@ def url(parser, token):
     args = []
     kwargs = {}
     asvar = None
+    bits = bits[2:]
+    if len(bits) >= 2 and bits[-2] == 'as':
+        asvar = bits[-1]
+        bits = bits[:-2]
 
-    if len(bits) > 2:
+    # Backwards compatibility: {% url urlname arg1,arg2 %} or
-        bits = iter(bits[2:])
+    # {% 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(",")
+
+    if len(bits):
         for bit in bits:
-            if bit == 'as':
+            match = kwarg_re.match(bit)
-                asvar = bits.next()
+            if not match:
-                break
-            else:
-                end = 0
-                for i, match in enumerate(url_arg_re.finditer(bit)):
-                    if (i == 0 and match.start() != 0) or \
-                          (i > 0 and (bit[end:match.start()] != ',')):
                 raise TemplateSyntaxError("Malformed arguments to url tag")
-                    end = match.end()
+            name, value = match.groups()
-                    name, value = match.group(1), match.group(2)
             if name:
                 kwargs[name] = parser.compile_filter(value)
             else:
                 args.append(parser.compile_filter(value))
-                if end != len(bit):
-                    raise TemplateSyntaxError("Malformed arguments to url tag")
 
     return URLNode(viewname, args, kwargs, asvar)
 url = register.tag(url)

docs/ref/templates/builtins.txt

@@ -904,7 +904,7 @@ Returns an absolute URL (i.e., a URL without the domain name) matching a given
 view function and optional parameters. This is a way to output links without
 violating the DRY principle by having to hard-code URLs in your templates::
 
-    {% url path.to.some_view v1,v2 %}
+    {% url path.to.some_view v1 v2 %}
 
 The first argument is a path to a view function in the format
 ``package.package.module.function``. Additional arguments are optional and
@@ -912,7 +912,7 @@ should be comma-separated values that will be used as arguments in the URL.
 The example above shows passing positional arguments. Alternatively you may
 use keyword syntax::
 
-    {% url path.to.some_view arg1=v1,arg2=v2 %}
+    {% url path.to.some_view arg1=v1 arg2=v2 %}
 
 Do not mix both positional and keyword syntax in a single call. All arguments
 required by the URLconf should be present.
@@ -954,7 +954,7 @@ If you'd like to retrieve a URL without displaying it, you can use a slightly
 different call::
 
 
-    {% url path.to.view arg, arg2 as the_url %}
+    {% url path.to.view arg arg2 as the_url %}
 
     <a href="{{ the_url }}">I'm linking to {{ the_url }}</a>
 
@@ -976,6 +976,20 @@ This will follow the normal :ref:`namespaced URL resolution strategy
 <topics-http-reversing-url-namespaces>`, including using any hints provided
 by the context as to the current application.
 
+.. versionchanged:: 1.2
+
+For backwards compatibility, the ``{% url %}`` tag also supports the
+use of commas to separate arguments. You shouldn't use this in any new
+projects, but for the sake of the people who are still using it,
+here's what it looks like::
+
+    {% url path.to.view arg,arg2 %}
+    {% url path.to.view arg, arg2 %}
+
+This syntax doesn't support the use of literal commas, or or equals
+signs. Did we mention you shouldn't use this syntax in any new
+projects?
+
 .. templatetag:: widthratio
 
 widthratio

tests/regressiontests/templates/tests.py

@@ -1173,9 +1173,15 @@ class Templates(unittest.TestCase):
 
             ### URL TAG ########################################################
             # 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/'),
+            '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/'),
+
             '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/'),
+            '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/'),
+            'url02a': ('{% 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/'),
@@ -1183,10 +1189,12 @@ class Templates(unittest.TestCase):
             'url07': (u'{% url regressiontests.templates.views.client2 tag=v %}', {'v': u'Ω'}, '/url_tag/%D0%AE%D0%BD%D0%B8%D0%BA%D0%BE%D0%B4/%CE%A9/'),
             'url08': (u'{% url метка_оператора v %}', {'v': 'Ω'}, '/url_tag/%D0%AE%D0%BD%D0%B8%D0%BA%D0%BE%D0%B4/%CE%A9/'),
             'url09': (u'{% url метка_оператора_2 tag=v %}', {'v': 'Ω'}, '/url_tag/%D0%AE%D0%BD%D0%B8%D0%BA%D0%BE%D0%B4/%CE%A9/'),
-            'url10': ('{% url regressiontests.templates.views.client_action id=client.id,action="two words" %}', {'client': {'id': 1}}, '/url_tag/client/1/two%20words/'),
+            'url10': ('{% url regressiontests.templates.views.client_action id=client.id action="two words" %}', {'client': {'id': 1}}, '/url_tag/client/1/two%20words/'),
-            'url11': ('{% url regressiontests.templates.views.client_action id=client.id,action="==" %}', {'client': {'id': 1}}, '/url_tag/client/1/==/'),
+            'url11': ('{% url regressiontests.templates.views.client_action id=client.id action="==" %}', {'client': {'id': 1}}, '/url_tag/client/1/==/'),
-            'url12': ('{% url regressiontests.templates.views.client_action id=client.id,action="," %}', {'client': {'id': 1}}, '/url_tag/client/1/,/'),
+            'url12': ('{% url regressiontests.templates.views.client_action id=client.id action="," %}', {'client': {'id': 1}}, '/url_tag/client/1/,/'),
-            'url12': ('{% url regressiontests.templates.views.client_action id=client.id,action=arg|join:"-" %}', {'client': {'id': 1}, 'arg':['a','b']}, '/url_tag/client/1/a-b/'),
+            '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/'),
 
             # Failures
             'url-fail01': ('{% url %}', {}, template.TemplateSyntaxError),