From f49e9a517f2fdc1d9ed7ac841ace77636cbd6747 Mon Sep 17 00:00:00 2001 From: Vladimir A Filonov Date: Sat, 23 Feb 2013 15:07:21 +0100 Subject: [PATCH] Fixed #17906 - Autoescaping {% cycle %} and {% firstof %} templatetags. This commit adds "future" version of these two tags with auto-escaping enabled. --- django/template/defaulttags.py | 48 +++++++++++++------- django/templatetags/future.py | 57 ++++++++++++++++++++++-- docs/internals/deprecation.txt | 4 ++ docs/ref/templates/builtins.txt | 55 ++++++++++++++++++----- docs/releases/1.6.txt | 28 ++++++++++++ tests/regressiontests/templates/tests.py | 12 ++++- 6 files changed, 173 insertions(+), 31 deletions(-) diff --git a/django/template/defaulttags.py b/django/template/defaulttags.py index c1ca947753..41db90c0ac 100644 --- a/django/template/defaulttags.py +++ b/django/template/defaulttags.py @@ -5,13 +5,15 @@ import sys import re from datetime import datetime from itertools import groupby, cycle as itertools_cycle +import warnings from django.conf import settings from django.template.base import (Node, NodeList, Template, Context, Library, TemplateSyntaxError, VariableDoesNotExist, InvalidTemplateLibrary, BLOCK_TAG_START, BLOCK_TAG_END, VARIABLE_TAG_START, VARIABLE_TAG_END, SINGLE_BRACE_START, SINGLE_BRACE_END, COMMENT_TAG_START, COMMENT_TAG_END, - VARIABLE_ATTRIBUTE_SEPARATOR, get_library, token_kwargs, kwarg_re) + VARIABLE_ATTRIBUTE_SEPARATOR, get_library, token_kwargs, kwarg_re, + _render_value_in_context) from django.template.smartif import IfParser, Literal from django.template.defaultfilters import date from django.utils.encoding import smart_text @@ -54,15 +56,15 @@ class CsrfTokenNode(Node): # misconfiguration, so we raise a warning from django.conf import settings if settings.DEBUG: - import warnings warnings.warn("A {% csrf_token %} was used in a template, but the context did not provide the value. This is usually caused by not using RequestContext.") return '' class CycleNode(Node): - def __init__(self, cyclevars, variable_name=None, silent=False): + def __init__(self, cyclevars, variable_name=None, silent=False, escape=False): self.cyclevars = cyclevars self.variable_name = variable_name self.silent = silent + self.escape = escape # only while the "future" version exists def render(self, context): if self not in context.render_context: @@ -74,7 +76,9 @@ class CycleNode(Node): context[self.variable_name] = value if self.silent: return '' - return value + if not self.escape: + value = mark_safe(value) + return _render_value_in_context(value, context) class DebugNode(Node): def render(self, context): @@ -97,14 +101,17 @@ class FilterNode(Node): return filtered class FirstOfNode(Node): - def __init__(self, vars): - self.vars = vars + def __init__(self, variables, escape=False): + self.vars = variables + self.escape = escape # only while the "future" version exists def render(self, context): for var in self.vars: value = var.resolve(context, True) if value: - return smart_text(value) + if not self.escape: + value = mark_safe(value) + return _render_value_in_context(value, context) return '' class ForNode(Node): @@ -508,7 +515,7 @@ def comment(parser, token): return CommentNode() @register.tag -def cycle(parser, token): +def cycle(parser, token, escape=False): """ Cycles among the given strings each time this tag is encountered. @@ -541,6 +548,11 @@ def cycle(parser, token): {% endfor %} """ + if not escape: + warnings.warn( + "'The syntax for the `cycle` template tag is changing. Load it " + "from the `future` tag library to start using the new behavior.", + PendingDeprecationWarning, stacklevel=2) # Note: This returns the exact same node on each {% cycle name %} call; # that is, the node object returned from {% cycle a b c as name %} and the @@ -588,13 +600,13 @@ def cycle(parser, token): if as_form: name = args[-1] values = [parser.compile_filter(arg) for arg in args[1:-2]] - node = CycleNode(values, name, silent=silent) + node = CycleNode(values, name, silent=silent, escape=escape) if not hasattr(parser, '_namedCycleNodes'): parser._namedCycleNodes = {} parser._namedCycleNodes[name] = node else: values = [parser.compile_filter(arg) for arg in args[1:]] - node = CycleNode(values) + node = CycleNode(values, escape=escape) return node @register.tag @@ -643,7 +655,7 @@ def do_filter(parser, token): return FilterNode(filter_expr, nodelist) @register.tag -def firstof(parser, token): +def firstof(parser, token, escape=False): """ Outputs the first variable passed that is not False, without escaping. @@ -657,11 +669,11 @@ def firstof(parser, token): {% if var1 %} {{ var1|safe }} - {% else %}{% if var2 %} + {% elif var2 %} {{ var2|safe }} - {% else %}{% if var3 %} + {% elif var3 %} {{ var3|safe }} - {% endif %}{% endif %}{% endif %} + {% endif %} but obviously much cleaner! @@ -677,10 +689,16 @@ def firstof(parser, token): {% endfilter %} """ + if not escape: + warnings.warn( + "'The syntax for the `firstof` template tag is changing. Load it " + "from the `future` tag library to start using the new behavior.", + PendingDeprecationWarning, stacklevel=2) + bits = token.split_contents()[1:] if len(bits) < 1: raise TemplateSyntaxError("'firstof' statement requires at least one argument") - return FirstOfNode([parser.compile_filter(bit) for bit in bits]) + return FirstOfNode([parser.compile_filter(bit) for bit in bits], escape=escape) @register.tag('for') def do_for(parser, token): diff --git a/django/templatetags/future.py b/django/templatetags/future.py index e6a0127e71..a385c6d565 100644 --- a/django/templatetags/future.py +++ b/django/templatetags/future.py @@ -1,14 +1,65 @@ from django.template import Library -from django.template.defaulttags import url as default_url, ssi as default_ssi +from django.template import defaulttags register = Library() + @register.tag def ssi(parser, token): # Used for deprecation path during 1.3/1.4, will be removed in 2.0 - return default_ssi(parser, token) + return defaulttags.ssi(parser, token) + @register.tag def url(parser, token): # Used for deprecation path during 1.3/1.4, will be removed in 2.0 - return default_url(parser, token) + return defaulttags.url(parser, token) + + +@register.tag +def cycle(parser, token): + """ + This is the future version of `cycle` with auto-escaping. + + By default all strings are escaped. + + If you want to disable auto-escaping of variables you can use: + + {% autoescape off %} + {% cycle var1 var2 var3 as somecycle %} + {% autoescape %} + + Or if only some variables should be escaped, you can use: + + {% cycle var1 var2|safe var3|safe as somecycle %} + """ + return defaulttags.cycle(parser, token, escape=True) + + +@register.tag +def firstof(parser, token): + """ + This is the future version of `firstof` with auto-escaping. + + This is equivalent to: + + {% if var1 %} + {{ var1 }} + {% elif var2 %} + {{ var2 }} + {% elif var3 %} + {{ var3 }} + {% endif %} + + If you want to disable auto-escaping of variables you can use: + + {% autoescape off %} + {% firstof var1 var2 var3 "fallback value" %} + {% autoescape %} + + Or if only some variables should be escaped, you can use: + + {% firstof var1 var2|safe var3 "fallback value"|safe %} + + """ + return defaulttags.firstof(parser, token, escape=True) diff --git a/docs/internals/deprecation.txt b/docs/internals/deprecation.txt index 50b9aa3c19..ef9fd31d15 100644 --- a/docs/internals/deprecation.txt +++ b/docs/internals/deprecation.txt @@ -323,6 +323,10 @@ these changes. 1.8 --- +* The :ttag:`cycle` and :ttag:`firstof` template tags will auto-escape their + arguments. In 1.6 and 1.7, this behavior is provided by the version of these + tags in the ``future`` template tag library. + * The ``SEND_BROKEN_LINK_EMAILS`` setting will be removed. Add the :class:`django.middleware.common.BrokenLinkEmailsMiddleware` middleware to your :setting:`MIDDLEWARE_CLASSES` setting instead. diff --git a/docs/ref/templates/builtins.txt b/docs/ref/templates/builtins.txt index cfc57cc551..149a557356 100644 --- a/docs/ref/templates/builtins.txt +++ b/docs/ref/templates/builtins.txt @@ -147,9 +147,8 @@ You can use any number of values in a ``{% cycle %}`` tag, separated by spaces. Values enclosed in single (``'``) or double quotes (``"``) are treated as string literals, while values without quotes are treated as template variables. -Note that the variables included in the cycle will not be escaped. -This is because template tags do not escape their content. Any HTML or -Javascript code contained in the printed variable will be rendered +Note that currently the variables included in the cycle will not be escaped. +Any HTML or Javascript code contained in the printed variable will be rendered as-is, which could potentially lead to security issues. For backwards compatibility, the ``{% cycle %}`` tag supports the much inferior @@ -190,6 +189,22 @@ call to ``{% cycle %}`` doesn't specify silent:: {% cycle 'row1' 'row2' as rowcolors silent %} {% cycle rowcolors %} +.. versionchanged:: 1.6 + +To improve safety, future versions of ``cycle`` will automatically escape +their output. You're encouraged to activate this behavior by loading +``cycle`` from the ``future`` template library:: + + {% load cycle from future %} + +When using the ``future`` version, you can disable auto-escaping with:: + + {% for o in some_list %} + + ... + + {% endfor %} + .. templatetag:: debug debug @@ -257,28 +272,44 @@ This is equivalent to:: {% if var1 %} {{ var1|safe }} - {% else %}{% if var2 %} + {% elif var2 %} {{ var2|safe }} - {% else %}{% if var3 %} + {% elif var3 %} {{ var3|safe }} - {% endif %}{% endif %}{% endif %} + {% endif %} You can also use a literal string as a fallback value in case all passed variables are False:: {% firstof var1 var2 var3 "fallback value" %} -Note that the variables included in the firstof tag will not be -escaped. This is because template tags do not escape their content. -Any HTML or Javascript code contained in the printed variable will be -rendered as-is, which could potentially lead to security issues. If you -need to escape the variables in the firstof tag, you must do so -explicitly:: +Note that currently the variables included in the firstof tag will not be +escaped. Any HTML or Javascript code contained in the printed variable will be +rendered as-is, which could potentially lead to security issues. If you need +to escape the variables in the firstof tag, you must do so explicitly:: {% filter force_escape %} {% firstof var1 var2 var3 "fallback value" %} {% endfilter %} +.. versionchanged:: 1.6 + +To improve safety, future versions of ``firstof`` will automatically escape +their output. You're encouraged to activate this behavior by loading +``firstof`` from the ``future`` template library:: + + {% load firstof from future %} + +When using the ``future`` version, you can disable auto-escaping with:: + + {% autoescape off %} + {% firstof var1 var2 var3 "fallback value" %} + {% endautoescape %} + +Or if only some variables should be escaped, you can use:: + + {% firstof var1 var2|safe var3 "fallback value"|safe %} + .. templatetag:: for for diff --git a/docs/releases/1.6.txt b/docs/releases/1.6.txt index 67c032a362..ce1e643946 100644 --- a/docs/releases/1.6.txt +++ b/docs/releases/1.6.txt @@ -160,6 +160,34 @@ Backwards incompatible changes in 1.6 Features deprecated in 1.6 ========================== +Changes to :ttag:`cycle` and :ttag:`firstof` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +The template system generally escapes all variables to avoid XSS attacks. +However, due to an accident of history, the :ttag:`cycle` and :ttag:`firstof` +tags render their arguments as-is. + +Django 1.6 starts a process to correct this inconsistency. The ``future`` +template library provides alternate implementations of :ttag:`cycle` and +:ttag:`firstof` that autoescape their inputs. If you're using these tags, +you're encourage to include the following line at the top of your templates to +enable the new behavior:: + + {% load cycle from future %} + +or:: + + {% load firstof from future %} + +The tags implementing the old behavior have been deprecated, and in Django +1.8, the old behavior will be replaced with the new behavior. To ensure +compatibility with future versions of Django, existing templates should be +modified to use the ``future`` versions. + +If necessary, you can temporarily disable auto-escaping with +:func:`~django.utils.safestring.mark_safe` or :ttag:`{% autoescape off %} +`. + ``SEND_BROKEN_LINK_EMAILS`` setting ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/tests/regressiontests/templates/tests.py b/tests/regressiontests/templates/tests.py index 02d3460b72..95b090fcde 100644 --- a/tests/regressiontests/templates/tests.py +++ b/tests/regressiontests/templates/tests.py @@ -773,6 +773,11 @@ class Templates(TestCase): 'cycle23': ("{% for x in values %}{% cycle 'a' 'b' 'c' as abc silent %}{{ abc }}{{ x }}{% endfor %}", {'values': [1,2,3,4]}, "a1b2c3a4"), 'included-cycle': ('{{ abc }}', {'abc': 'xxx'}, 'xxx'), 'cycle24': ("{% for x in values %}{% cycle 'a' 'b' 'c' as abc silent %}{% include 'included-cycle' %}{% endfor %}", {'values': [1,2,3,4]}, "abca"), + 'cycle25': ('{% cycle a as abc %}', {'a': '<'}, '<'), + + 'cycle26': ('{% load cycle from future %}{% cycle a b as ab %}{% cycle ab %}', {'a': '<', 'b': '>'}, '<>'), + 'cycle27': ('{% load cycle from future %}{% autoescape off %}{% cycle a b as ab %}{% cycle ab %}{% endautoescape %}', {'a': '<', 'b': '>'}, '<>'), + 'cycle28': ('{% load cycle from future %}{% cycle a|safe b as ab %}{% cycle ab %}', {'a': '<', 'b': '>'}, '<>'), ### EXCEPTIONS ############################################################ @@ -804,7 +809,12 @@ class Templates(TestCase): 'firstof07': ('{% firstof a b "c" %}', {'a':0}, 'c'), 'firstof08': ('{% firstof a b "c and d" %}', {'a':0,'b':0}, 'c and d'), 'firstof09': ('{% firstof %}', {}, template.TemplateSyntaxError), - 'firstof10': ('{% firstof a %}', {'a': '<'}, '<'), # Variables are NOT auto-escaped. + 'firstof10': ('{% firstof a %}', {'a': '<'}, '<'), + + 'firstof11': ('{% load firstof from future %}{% firstof a b %}', {'a': '<', 'b': '>'}, '<'), + 'firstof12': ('{% load firstof from future %}{% firstof a b %}', {'a': '', 'b': '>'}, '>'), + 'firstof13': ('{% load firstof from future %}{% autoescape off %}{% firstof a %}{% endautoescape %}', {'a': '<'}, '<'), + 'firstof14': ('{% load firstof from future %}{% firstof a|safe b %}', {'a': '<'}, '<'), ### FOR TAG ############################################################### 'for-tag01': ("{% for val in values %}{{ val }}{% endfor %}", {"values": [1, 2, 3]}, "123"),