Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Use `token.split_contents()` in tags that can take variables (fixes #6271 and #18260) #751

Merged
merged 1 commit into from

3 participants

@bmispelon
Collaborator

After fixing #19882, I grepped through the code to check for usage of token.contents.split().

It turned out that several other tags were affected by the same issue, so I fix those.

There are still tags that use token.contents.split() instead of token.split_contents(), but since they don't take any variable argument, they are not affected.

Here's a list of the remaining instances of token.contents.split() and the tags they are associated with:

django/template/base.py:262: <not a template tag>
django/template/defaulttags.py:492: autoescape
django/template/defaulttags.py:636: filter
django/template/defaulttags.py:1014: load
django/template/defaulttags.py:1186: templatetag
django/template/loader_tags.py:178: block
django/templatetags/i18n.py:188: get_available_languages
django/templatetags/i18n.py:260: get_current_language
django/templatetags/i18n.py:278: get_current_language_bidi
django/templatetags/static.py:30: get_static_prefix, get_media_prefix
django/templatetags/tz.py:193: get_current_timezone

Also note that there are still tags that are affected by this issue in contrib.
Namely, all the tags from contrib.comments as well as the get_admin_log for contrib.admin.
Those did not have any tests and I didn't see any bugs in the tracker, so I left them as-is (for now).

@aaugustin
Owner

I've wanted to write that patch forever...

@aaugustin
Owner

Would you mind:

  • adding a short comment above each use of .content.split that you didn't change?

    # token.split_contents() isn't useful here because this tag doesn't accept variable as arguments

  • rebasing this to a single commit with the following commit message ?

    Used token.split_contents() for tokenisation in template tags accepting variables.

    Fixed #6271, #18260.


Then I'll run the tests and merge it.

@bmispelon
Collaborator

Done.

As discussed on IRC, I'll file a separate ticket for the similar issues in contrib.comments and contrib.admin.

@aaugustin aaugustin merged commit 5278776 into from
@funkybob

Here you parse_filter the fragment_name, but it's never resolved above. It only works because the FilterExpression.str method returns self.token

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Feb 23, 2013
  1. @bmispelon

    Used token.split_contents() for tokenisation in template tags accepti…

    bmispelon authored
    …ng variables.
    
    Fixed #6271, #18260.
This page is out of date. Refresh to see the latest.
View
23 django/template/defaulttags.py
@@ -489,6 +489,7 @@ def autoescape(parser, token):
"""
Force autoescape behavior for this block.
"""
+ # token.split_contents() isn't useful here because this tag doesn't accept variable as arguments
args = token.contents.split()
if len(args) != 2:
raise TemplateSyntaxError("'autoescape' tag requires exactly one argument.")
@@ -633,6 +634,7 @@ def do_filter(parser, token):
Instead, use the ``autoescape`` tag to manage autoescaping for blocks of
template code.
"""
+ # token.split_contents() isn't useful here because this tag doesn't accept variable as arguments
_, rest = token.contents.split(None, 1)
filter_expr = parser.compile_filter("var|%s" % (rest))
for func, unused in filter_expr.filters:
@@ -954,7 +956,7 @@ def ifchanged(parser, token):
{% endifchanged %}
{% endfor %}
"""
- bits = token.contents.split()
+ bits = token.split_contents()
nodelist_true = parser.parse(('else', 'endifchanged'))
token = parser.next_token()
if token.contents == 'else':
@@ -1011,6 +1013,7 @@ def load(parser, token):
{% load byline from news %}
"""
+ # token.split_contents() isn't useful here because this tag doesn't accept variable as arguments
bits = token.contents.split()
if len(bits) >= 4 and bits[-2] == "from":
try:
@@ -1109,17 +1112,16 @@ def regroup(parser, token):
{% regroup people|dictsort:"gender" by gender as grouped %}
"""
- firstbits = token.contents.split(None, 3)
- if len(firstbits) != 4:
+ bits = token.split_contents()
+ if len(bits) != 6:
raise TemplateSyntaxError("'regroup' tag takes five arguments")
- target = parser.compile_filter(firstbits[1])
- if firstbits[2] != 'by':
+ target = parser.compile_filter(bits[1])
+ if bits[2] != 'by':
raise TemplateSyntaxError("second argument to 'regroup' tag must be 'by'")
- lastbits_reversed = firstbits[3][::-1].split(None, 2)
- if lastbits_reversed[1][::-1] != 'as':
+ if bits[4] != 'as':
raise TemplateSyntaxError("next-to-last argument to 'regroup' tag must"
" be 'as'")
- var_name = lastbits_reversed[0][::-1]
+ var_name = bits[5]
# RegroupNode will take each item in 'target', put it in the context under
# 'var_name', evaluate 'var_name'.'expression' in the current context, and
# group by the resulting value. After all items are processed, it will
@@ -1128,7 +1130,7 @@ def regroup(parser, token):
# doesn't provide a context-aware equivalent of Python's getattr.
expression = parser.compile_filter(var_name +
VARIABLE_ATTRIBUTE_SEPARATOR +
- lastbits_reversed[2][::-1])
+ bits[3])
return RegroupNode(target, expression, var_name)
@register.tag
@@ -1184,6 +1186,7 @@ def templatetag(parser, token):
``closecomment`` ``#}``
================== =======
"""
+ # token.split_contents() isn't useful here because this tag doesn't accept variable as arguments
bits = token.contents.split()
if len(bits) != 2:
raise TemplateSyntaxError("'templatetag' statement takes one argument")
@@ -1325,7 +1328,7 @@ def widthratio(parser, token):
the image in the above example will be 88 pixels wide
(because 175/200 = .875; .875 * 100 = 87.5 which is rounded up to 88).
"""
- bits = token.contents.split()
+ bits = token.split_contents()
if len(bits) != 4:
raise TemplateSyntaxError("widthratio takes three arguments")
tag, this_value_expr, max_value_expr, max_width = bits
View
1  django/template/loader_tags.py
@@ -174,6 +174,7 @@ def do_block(parser, token):
"""
Define a block that can be overridden by child templates.
"""
+ # token.split_contents() isn't useful here because this tag doesn't accept variable as arguments
bits = token.contents.split()
if len(bits) != 2:
raise TemplateSyntaxError("'%s' tag takes only one argument" % bits[0])
View
14 django/templatetags/cache.py
@@ -1,8 +1,7 @@
from __future__ import unicode_literals
import hashlib
-from django.template import Library, Node, TemplateSyntaxError, Variable, VariableDoesNotExist
-from django.template import resolve_variable
+from django.template import Library, Node, TemplateSyntaxError, VariableDoesNotExist
from django.core.cache import cache
from django.utils.encoding import force_bytes
from django.utils.http import urlquote
@@ -12,7 +11,7 @@
class CacheNode(Node):
def __init__(self, nodelist, expire_time_var, fragment_name, vary_on):
self.nodelist = nodelist
- self.expire_time_var = Variable(expire_time_var)
+ self.expire_time_var = expire_time_var
self.fragment_name = fragment_name
self.vary_on = vary_on
@@ -26,7 +25,7 @@ def render(self, context):
except (ValueError, TypeError):
raise TemplateSyntaxError('"cache" tag got a non-integer timeout value: %r' % expire_time)
# Build a key for this fragment and all vary-on's.
- key = ':'.join([urlquote(resolve_variable(var, context)) for var in self.vary_on])
+ key = ':'.join([urlquote(var.resolve(context)) for var in self.vary_on])
args = hashlib.md5(force_bytes(key))
cache_key = 'template.cache.%s.%s' % (self.fragment_name, args.hexdigest())
value = cache.get(cache_key)
@@ -59,7 +58,10 @@ def do_cache(parser, token):
"""
nodelist = parser.parse(('endcache',))
parser.delete_first_token()
- tokens = token.contents.split()
+ tokens = token.split_contents()
if len(tokens) < 3:
raise TemplateSyntaxError("'%r' tag requires at least 2 arguments." % tokens[0])
- return CacheNode(nodelist, tokens[1], tokens[2], tokens[3:])
+ return CacheNode(nodelist,
+ parser.compile_filter(tokens[1]),
+ parser.compile_filter(tokens[2]),
+ [parser.compile_filter(token) for token in tokens[3:]])
View
15 django/templatetags/i18n.py
@@ -24,7 +24,7 @@ def render(self, context):
class GetLanguageInfoNode(Node):
def __init__(self, lang_code, variable):
- self.lang_code = Variable(lang_code)
+ self.lang_code = lang_code
self.variable = variable
def render(self, context):
@@ -35,7 +35,7 @@ def render(self, context):
class GetLanguageInfoListNode(Node):
def __init__(self, languages, variable):
- self.languages = Variable(languages)
+ self.languages = languages
self.variable = variable
def get_language_info(self, language):
@@ -185,6 +185,7 @@ def do_get_available_languages(parser, token):
your setting file (or the default settings) and
put it into the named variable.
"""
+ # token.split_contents() isn't useful here because this tag doesn't accept variable as arguments
args = token.contents.split()
if len(args) != 3 or args[1] != 'as':
raise TemplateSyntaxError("'get_available_languages' requires 'as variable' (got %r)" % args)
@@ -204,10 +205,10 @@ def do_get_language_info(parser, token):
{{ l.name_local }}
{{ l.bidi|yesno:"bi-directional,uni-directional" }}
"""
- args = token.contents.split()
+ args = token.split_contents()
if len(args) != 5 or args[1] != 'for' or args[3] != 'as':
raise TemplateSyntaxError("'%s' requires 'for string as variable' (got %r)" % (args[0], args[1:]))
- return GetLanguageInfoNode(args[2], args[4])
+ return GetLanguageInfoNode(parser.compile_filter(args[2]), args[4])
@register.tag("get_language_info_list")
def do_get_language_info_list(parser, token):
@@ -227,10 +228,10 @@ def do_get_language_info_list(parser, token):
{{ l.bidi|yesno:"bi-directional,uni-directional" }}
{% endfor %}
"""
- args = token.contents.split()
+ args = token.split_contents()
if len(args) != 5 or args[1] != 'for' or args[3] != 'as':
raise TemplateSyntaxError("'%s' requires 'for sequence as variable' (got %r)" % (args[0], args[1:]))
- return GetLanguageInfoListNode(args[2], args[4])
+ return GetLanguageInfoListNode(parser.compile_filter(args[2]), args[4])
@register.filter
def language_name(lang_code):
@@ -257,6 +258,7 @@ def do_get_current_language(parser, token):
put it's value into the ``language`` context
variable.
"""
+ # token.split_contents() isn't useful here because this tag doesn't accept variable as arguments
args = token.contents.split()
if len(args) != 3 or args[1] != 'as':
raise TemplateSyntaxError("'get_current_language' requires 'as variable' (got %r)" % args)
@@ -275,6 +277,7 @@ def do_get_current_language_bidi(parser, token):
put it's value into the ``bidi`` context variable.
True indicates right-to-left layout, otherwise left-to-right
"""
+ # token.split_contents() isn't useful here because this tag doesn't accept variable as arguments
args = token.contents.split()
if len(args) != 3 or args[1] != 'as':
raise TemplateSyntaxError("'get_current_language_bidi' requires 'as variable' (got %r)" % args)
View
1  django/templatetags/static.py
@@ -27,6 +27,7 @@ def handle_token(cls, parser, token, name):
"""
Class method to parse prefix node and return a Node.
"""
+ # token.split_contents() isn't useful here because tags using this method don't accept variable as arguments
tokens = token.contents.split()
if len(tokens) > 1 and tokens[1] != 'as':
raise template.TemplateSyntaxError(
View
1  django/templatetags/tz.py
@@ -190,6 +190,7 @@ def get_current_timezone_tag(parser, token):
This will fetch the currently active time zone and put its name
into the ``TIME_ZONE`` context variable.
"""
+ # token.split_contents() isn't useful here because this tag doesn't accept variable as arguments
args = token.contents.split()
if len(args) != 3 or args[1] != 'as':
raise TemplateSyntaxError("'get_current_timezone' requires "
View
7 tests/regressiontests/templates/templatetags/custom.py
@@ -12,6 +12,13 @@
def trim(value, num):
return value[:num]
+@register.filter
+def noop(value, param=None):
+ """A noop filter that always return its first argument and does nothing with
+ its second (optional) one.
+ Useful for testing out whitespace in filter arguments (see #19882)."""
+ return value
+
@register.simple_tag
def no_params():
"""Expected no_params __doc__"""
View
25 tests/regressiontests/templates/tests.py
@@ -834,7 +834,7 @@ def get_template_tests(self):
'for-tag-empty02': ("{% for val in values %}{{ val }}{% empty %}values array empty{% endfor %}", {"values": []}, "values array empty"),
'for-tag-empty03': ("{% for val in values %}{{ val }}{% empty %}values array not found{% endfor %}", {}, "values array not found"),
# Ticket 19882
- 'for-tag-filter-ws': ("{% for x in ''|add:'a b c' %}{{ x }}{% endfor %}", {}, 'a b c'),
+ 'for-tag-filter-ws': ("{% load custom %}{% for x in s|noop:'x y' %}{{ x }}{% endfor %}", {'s': 'abc'}, 'abc'),
### IF TAG ################################################################
'if-tag01': ("{% if foo %}yes{% else %}no{% endif %}", {"foo": True}, "yes"),
@@ -1003,6 +1003,9 @@ def get_template_tests(self):
'ifchanged-else04': ('{% for id in ids %}{% ifchanged %}***{{ id }}*{% else %}...{% endifchanged %}{{ forloop.counter }}{% endfor %}', {'ids': [1,1,2,2,2,3,4]}, '***1*1...2***2*3...4...5***3*6***4*7'),
+ # Test whitespace in filter arguments
+ 'ifchanged-filter-ws': ('{% load custom %}{% for n in num %}{% ifchanged n|noop:"x y" %}..{% endifchanged %}{{ n }}{% endfor %}', {'num': (1,2,3)}, '..1..2..3'),
+
### IFEQUAL TAG ###########################################################
'ifequal01': ("{% ifequal a b %}yes{% endifequal %}", {"a": 1, "b": 2}, ""),
'ifequal02': ("{% ifequal a b %}yes{% endifequal %}", {"a": 1, "b": 1}, "yes"),
@@ -1343,6 +1346,10 @@ def get_template_tests(self):
'i18n36': ('{% load i18n %}{% trans "Page not found" as page_not_found noop %}{{ page_not_found }}', {'LANGUAGE_CODE': 'de'}, "Page not found"),
'i18n37': ('{% load i18n %}{% trans "Page not found" as page_not_found %}{% blocktrans %}Error: {{ page_not_found }}{% endblocktrans %}', {'LANGUAGE_CODE': 'de'}, "Error: Seite nicht gefunden"),
+ # Test whitespace in filter arguments
+ 'i18n38': ('{% load i18n custom %}{% get_language_info for "de"|noop:"x y" as l %}{{ l.code }}: {{ l.name }}/{{ l.name_local }} bidi={{ l.bidi }}', {}, 'de: German/Deutsch bidi=False'),
+ 'i18n38_2': ('{% load i18n custom %}{% get_language_info_list for langcodes|noop:"x y" as langs %}{% for l in langs %}{{ l.code }}: {{ l.name }}/{{ l.name_local }} bidi={{ l.bidi }}; {% endfor %}', {'langcodes': ['it', 'no']}, 'it: Italian/italiano bidi=False; no: Norwegian/norsk bidi=False; '),
+
### HANDLING OF TEMPLATE_STRING_IF_INVALID ###################################
'invalidstr01': ('{{ var|default:"Foo" }}', {}, ('Foo','INVALID')),
@@ -1424,6 +1431,16 @@ def get_template_tests(self):
{'foo': 'z', 'bar': ['a', 'd']}]},
'abc:xy,ad:z,'),
+ # Test syntax
+ 'regroup05': ('{% regroup data by bar as %}', {},
+ template.TemplateSyntaxError),
+ 'regroup06': ('{% regroup data by bar thisaintright grouped %}', {},
+ template.TemplateSyntaxError),
+ 'regroup07': ('{% regroup data thisaintright bar as grouped %}', {},
+ template.TemplateSyntaxError),
+ 'regroup08': ('{% regroup data by bar as grouped toomanyargs %}', {},
+ template.TemplateSyntaxError),
+
### SSI TAG ########################################################
# Test normal behavior
@@ -1492,6 +1509,9 @@ def get_template_tests(self):
'widthratio14a': ('{% widthratio a b c %}', {'a':0,'b':100,'c':'c'}, template.TemplateSyntaxError),
'widthratio14b': ('{% widthratio a b c %}', {'a':0,'b':100,'c':None}, template.TemplateSyntaxError),
+ # Test whitespace in filter argument
+ 'widthratio15': ('{% load custom %}{% widthratio a|noop:"x y" b 0 %}', {'a':50,'b':100}, '0'),
+
### WITH TAG ########################################################
'with01': ('{% with key=dict.key %}{{ key }}{% endwith %}', {'dict': {'key': 50}}, '50'),
'legacywith01': ('{% with dict.key as key %}{{ key }}{% endwith %}', {'dict': {'key': 50}}, '50'),
@@ -1593,6 +1613,9 @@ def get_template_tests(self):
# Regression test for #11270.
'cache17': ('{% load cache %}{% cache 10 long_cache_key poem %}Some Content{% endcache %}', {'poem': 'Oh freddled gruntbuggly/Thy micturations are to me/As plurdled gabbleblotchits/On a lurgid bee/That mordiously hath bitled out/Its earted jurtles/Into a rancid festering/Or else I shall rend thee in the gobberwarts with my blurglecruncheon/See if I dont.'}, 'Some Content'),
+ # Test whitespace in filter arguments
+ 'cache18': ('{% load cache custom %}{% cache 2|noop:"x y" cache18 %}cache18{% endcache %}', {}, 'cache18'),
+
### AUTOESCAPE TAG ##############################################
'autoescape-tag01': ("{% autoescape off %}hello{% endautoescape %}", {}, "hello"),
Something went wrong with that request. Please try again.