Fixed #24464 -- Made built-in HTML template filter functions escape t…

…heir input by default. This may cause some backwards compatibility issues, but may also resolve security issues in third party projects that fail to heed warnings in our documentation. Thanks Markus Holtermann for help with tests and docs.
django · Mar 9, 2015 · fa350e2 · fa350e2
1 parent fb14619
commit fa350e2
Show file tree

Hide file tree

Showing 10 changed files with 174 additions and 24 deletions.
diff --git a/django/template/defaultfilters.py b/django/template/defaultfilters.py
@@ -191,7 +191,7 @@ def iriencode(value):
 
 @register.filter(is_safe=True, needs_autoescape=True)
 @stringfilter
-def linenumbers(value, autoescape=None):
+def linenumbers(value, autoescape=True):
     """Displays text with line numbers."""
     lines = value.split('\n')
     # Find the maximum width of the line count, for use with zero padding
@@ -353,14 +353,14 @@ def urlencode(value, safe=None):
 
 @register.filter(is_safe=True, needs_autoescape=True)
 @stringfilter
-def urlize(value, autoescape=None):
+def urlize(value, autoescape=True):
     """Converts URLs in plain text into clickable links."""
     return mark_safe(_urlize(value, nofollow=True, autoescape=autoescape))
 
 
 @register.filter(is_safe=True, needs_autoescape=True)
 @stringfilter
-def urlizetrunc(value, limit, autoescape=None):
+def urlizetrunc(value, limit, autoescape=True):
     """
     Converts URLs into clickable links, truncating URLs to the given character
     limit, and adding 'rel=nofollow' attribute to discourage spamming.
@@ -457,7 +457,7 @@ def force_escape(value):
 
 @register.filter("linebreaks", is_safe=True, needs_autoescape=True)
 @stringfilter
-def linebreaks_filter(value, autoescape=None):
+def linebreaks_filter(value, autoescape=True):
     """
     Replaces line breaks in plain text with appropriate HTML; a single
     newline becomes an HTML line break (``<br />``) and a new line
@@ -469,7 +469,7 @@ def linebreaks_filter(value, autoescape=None):
 
 @register.filter(is_safe=True, needs_autoescape=True)
 @stringfilter
-def linebreaksbr(value, autoescape=None):
+def linebreaksbr(value, autoescape=True):
     """
     Converts all newlines in a piece of plain text to HTML line breaks
     (``<br />``).
@@ -552,7 +552,7 @@ def first(value):
 
 
 @register.filter(is_safe=True, needs_autoescape=True)
-def join(value, arg, autoescape=None):
+def join(value, arg, autoescape=True):
     """
     Joins a list with a string, like Python's ``str.join(list)``.
     """
@@ -622,7 +622,7 @@ def slice_filter(value, arg):
 
 
 @register.filter(is_safe=True, needs_autoescape=True)
-def unordered_list(value, autoescape=None):
+def unordered_list(value, autoescape=True):
     """
     Recursively takes a self-nested list and returns an HTML unordered list --
     WITHOUT opening and closing <ul> tags.

diff --git a/docs/howto/custom-template-tags.txt b/docs/howto/custom-template-tags.txt
@@ -281,7 +281,9 @@ Template filter code falls into one of two situations:
    (If you don't specify this flag, it defaults to ``False``). This flag tells
    Django that your filter function wants to be passed an extra keyword
    argument, called ``autoescape``, that is ``True`` if auto-escaping is in
-   effect and ``False`` otherwise.
+   effect and ``False`` otherwise. It is recommended to set the default of the
+   ``autoescape`` parameter to ``True``, so that if you call the function
+   from Python code it will have escaping enabled by default.
 
    For example, let's write a filter that emphasizes the first character of
    a string::
@@ -293,7 +295,7 @@ Template filter code falls into one of two situations:
       register = template.Library()
 
       @register.filter(needs_autoescape=True)
-      def initial_letter_filter(text, autoescape=None):
+      def initial_letter_filter(text, autoescape=True):
           first, other = text[0], text[1:]
           if autoescape:
               esc = conditional_escape
@@ -323,19 +325,28 @@ Template filter code falls into one of two situations:
 
 .. warning:: Avoiding XSS vulnerabilities when reusing built-in filters
 
-    Be careful when reusing Django's built-in filters. You'll need to pass
-    ``autoescape=True`` to the filter in order to get the proper autoescaping
-    behavior and avoid a cross-site script vulnerability.
+    .. versionchanged:: 1.8
+
+    Django's built-in filters have ``autoescape=True`` by default in order to
+    get the proper autoescaping behavior and avoid a cross-site script
+    vulnerability.
+
+    In older versions of Django, be careful when reusing Django's built-in
+    filters as ``autoescape`` defaults to ``None``. You'll need to pass
+    ``autoescape=True`` to get autoescaping.
 
     For example, if you wanted to write a custom filter called
     ``urlize_and_linebreaks`` that combined the :tfilter:`urlize` and
     :tfilter:`linebreaksbr` filters, the filter would look like::
 
         from django.template.defaultfilters import linebreaksbr, urlize
 
-        @register.filter
-        def urlize_and_linebreaks(text):
-            return linebreaksbr(urlize(text, autoescape=True), autoescape=True)
+        @register.filter(needs_autoescape=True)
+        def urlize_and_linebreaks(text, autoescape=True):
+            return linebreaksbr(
+                urlize(text, autoescape=autoescape),
+                autoescape=autoescape
+            )
 
     Then:
 

diff --git a/docs/releases/1.8.txt b/docs/releases/1.8.txt
@@ -1012,6 +1012,26 @@ those writing third-party backends in updating their code:
   now takes a second argument named ``obj_id`` which is the serialized
   identifier used to retrieve the object before deletion.
 
+Default autoescaping of functions in ``django.template.defaultfilters``
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+In order to make built-in template filters that output HTML "safe by default"
+when calling them in Python code, the following functions in
+``django.template.defaultfilters`` have been changed to automatically escape
+their input value:
+
+* ``join``
+* ``linebreaksbr``
+* ``linebreaks_filter``
+* ``linenumbers``
+* ``unordered_list``
+* ``urlize``
+* ``urlizetrunc``
+
+You can revert to the old behavior by specifying ``autoescape=False`` if you
+are passing trusted content. This change doesn't have any effect when using
+the corresponding filters in templates.
+
 Miscellaneous
 ~~~~~~~~~~~~~
 

diff --git a/tests/template_tests/filter_tests/test_join.py b/tests/template_tests/filter_tests/test_join.py
@@ -54,3 +54,15 @@ class FunctionTests(SimpleTestCase):
 
     def test_list(self):
         self.assertEqual(join([0, 1, 2], 'glue'), '0glue1glue2')
+
+    def test_autoescape(self):
+        self.assertEqual(
+            join(['<a>', '<img>', '</a>'], '<br>'),
+            '&lt;a&gt;&lt;br&gt;&lt;img&gt;&lt;br&gt;&lt;/a&gt;',
+        )
+
+    def test_autoescape_off(self):
+        self.assertEqual(
+            join(['<a>', '<img>', '</a>'], '<br>', autoescape=False),
+            '<a>&lt;br&gt;<img>&lt;br&gt;</a>',
+        )
diff --git a/tests/template_tests/filter_tests/test_linebreaks.py b/tests/template_tests/filter_tests/test_linebreaks.py
@@ -39,3 +39,15 @@ def test_carriage_newline(self):
 
     def test_non_string_input(self):
         self.assertEqual(linebreaks_filter(123), '<p>123</p>')
+
+    def test_autoescape(self):
+        self.assertEqual(
+            linebreaks_filter('foo\n<a>bar</a>\nbuz'),
+            '<p>foo<br />&lt;a&gt;bar&lt;/a&gt;<br />buz</p>',
+        )
+
+    def test_autoescape_off(self):
+        self.assertEqual(
+            linebreaks_filter('foo\n<a>bar</a>\nbuz', autoescape=False),
+            '<p>foo<br /><a>bar</a><br />buz</p>',
+        )
diff --git a/tests/template_tests/filter_tests/test_linebreaksbr.py b/tests/template_tests/filter_tests/test_linebreaksbr.py
@@ -36,3 +36,15 @@ def test_carriage_newline(self):
 
     def test_non_string_input(self):
         self.assertEqual(linebreaksbr(123), '123')
+
+    def test_autoescape(self):
+        self.assertEqual(
+            linebreaksbr('foo\n<a>bar</a>\nbuz'),
+            'foo<br />&lt;a&gt;bar&lt;/a&gt;<br />buz',
+        )
+
+    def test_autoescape_off(self):
+        self.assertEqual(
+            linebreaksbr('foo\n<a>bar</a>\nbuz', autoescape=False),
+            'foo<br /><a>bar</a><br />buz',
+        )
diff --git a/tests/template_tests/filter_tests/test_linenumbers.py b/tests/template_tests/filter_tests/test_linenumbers.py
@@ -44,3 +44,15 @@ def test_linenumbers2(self):
 
     def test_non_string_input(self):
         self.assertEqual(linenumbers(123), '1. 123')
+
+    def test_autoescape(self):
+        self.assertEqual(
+            linenumbers('foo\n<a>bar</a>\nbuz'),
+            '1. foo\n2. &lt;a&gt;bar&lt;/a&gt;\n3. buz',
+        )
+
+    def test_autoescape_off(self):
+        self.assertEqual(
+            linenumbers('foo\n<a>bar</a>\nbuz', autoescape=False),
+            '1. foo\n2. <a>bar</a>\n3. buz'
+        )
diff --git a/tests/template_tests/filter_tests/test_unordered_list.py b/tests/template_tests/filter_tests/test_unordered_list.py
@@ -99,6 +99,18 @@ def test_nested_multiple2(self):
             '\n\t\t<li>Illinois</li>\n\t</ul>\n\t</li>',
         )
 
+    def test_autoescape(self):
+        self.assertEqual(
+            unordered_list(['<a>item 1</a>', 'item 2']),
+            '\t<li>&lt;a&gt;item 1&lt;/a&gt;</li>\n\t<li>item 2</li>',
+        )
+
+    def test_autoescape_off(self):
+        self.assertEqual(
+            unordered_list(['<a>item 1</a>', 'item 2'], autoescape=False),
+            '\t<li><a>item 1</a></li>\n\t<li>item 2</li>',
+        )
+
     def test_ulitem(self):
         @python_2_unicode_compatible
         class ULItem(object):
@@ -110,13 +122,48 @@ def __str__(self):
 
         a = ULItem('a')
         b = ULItem('b')
-        self.assertEqual(unordered_list([a, b]), '\t<li>ulitem-a</li>\n\t<li>ulitem-b</li>')
+        c = ULItem('<a>c</a>')
+        self.assertEqual(
+            unordered_list([a, b, c]),
+            '\t<li>ulitem-a</li>\n\t<li>ulitem-b</li>\n\t<li>ulitem-&lt;a&gt;c&lt;/a&gt;</li>',
+        )
+
+        def item_generator():
+            yield a
+            yield b
+            yield c
+
+        self.assertEqual(
+            unordered_list(item_generator()),
+            '\t<li>ulitem-a</li>\n\t<li>ulitem-b</li>\n\t<li>ulitem-&lt;a&gt;c&lt;/a&gt;</li>',
+        )
+
+    def test_ulitem_autoescape_off(self):
+        @python_2_unicode_compatible
+        class ULItem(object):
+            def __init__(self, title):
+                self.title = title
+
+            def __str__(self):
+                return 'ulitem-%s' % str(self.title)
+
+        a = ULItem('a')
+        b = ULItem('b')
+        c = ULItem('<a>c</a>')
+        self.assertEqual(
+            unordered_list([a, b, c], autoescape=False),
+            '\t<li>ulitem-a</li>\n\t<li>ulitem-b</li>\n\t<li>ulitem-<a>c</a></li>',
+        )
 
         def item_generator():
             yield a
             yield b
+            yield c
 
-        self.assertEqual(unordered_list(item_generator()), '\t<li>ulitem-a</li>\n\t<li>ulitem-b</li>')
+        self.assertEqual(
+            unordered_list(item_generator(), autoescape=False),
+            '\t<li>ulitem-a</li>\n\t<li>ulitem-b</li>\n\t<li>ulitem-<a>c</a></li>',
+        )
 
     @ignore_warnings(category=RemovedInDjango20Warning)
     def test_legacy(self):

diff --git a/tests/template_tests/filter_tests/test_urlize.py b/tests/template_tests/filter_tests/test_urlize.py
@@ -259,27 +259,27 @@ def test_quotation_marks(self):
         #20364 - Check urlize correctly include quotation marks in links
         """
         self.assertEqual(
-            urlize('before "hi@example.com" afterwards'),
+            urlize('before "hi@example.com" afterwards', autoescape=False),
             'before "<a href="mailto:hi@example.com">hi@example.com</a>" afterwards',
         )
         self.assertEqual(
-            urlize('before hi@example.com" afterwards'),
+            urlize('before hi@example.com" afterwards', autoescape=False),
             'before <a href="mailto:hi@example.com">hi@example.com</a>" afterwards',
         )
         self.assertEqual(
-            urlize('before "hi@example.com afterwards'),
+            urlize('before "hi@example.com afterwards', autoescape=False),
             'before "<a href="mailto:hi@example.com">hi@example.com</a> afterwards',
         )
         self.assertEqual(
-            urlize('before \'hi@example.com\' afterwards'),
+            urlize('before \'hi@example.com\' afterwards', autoescape=False),
             'before \'<a href="mailto:hi@example.com">hi@example.com</a>\' afterwards',
         )
         self.assertEqual(
-            urlize('before hi@example.com\' afterwards'),
+            urlize('before hi@example.com\' afterwards', autoescape=False),
             'before <a href="mailto:hi@example.com">hi@example.com</a>\' afterwards',
         )
         self.assertEqual(
-            urlize('before \'hi@example.com afterwards'),
+            urlize('before \'hi@example.com afterwards', autoescape=False),
             'before \'<a href="mailto:hi@example.com">hi@example.com</a> afterwards',
         )
 
@@ -288,7 +288,7 @@ def test_quote_commas(self):
         #20364 - Check urlize copes with commas following URLs in quotes
         """
         self.assertEqual(
-            urlize('Email us at "hi@example.com", or phone us at +xx.yy'),
+            urlize('Email us at "hi@example.com", or phone us at +xx.yy', autoescape=False),
             'Email us at "<a href="mailto:hi@example.com">hi@example.com</a>", or phone us at +xx.yy',
         )
 
@@ -316,3 +316,15 @@ def test_exclamation_marks(self):
 
     def test_non_string_input(self):
         self.assertEqual(urlize(123), '123')
+
+    def test_autoescape(self):
+        self.assertEqual(
+            urlize('foo<a href=" google.com ">bar</a>buz'),
+            'foo&lt;a href=&quot; <a href="http://google.com" rel="nofollow">google.com</a> &quot;&gt;bar&lt;/a&gt;buz',
+        )
+
+    def test_autoescape_off(self):
+        self.assertEqual(
+            urlize('foo<a href=" google.com ">bar</a>buz', autoescape=False),
+            'foo<a href=" <a href="http://google.com" rel="nofollow">google.com</a> ">bar</a>buz',
+        )
diff --git a/tests/template_tests/filter_tests/test_urlizetrunc.py b/tests/template_tests/filter_tests/test_urlizetrunc.py
@@ -78,3 +78,15 @@ def test_query_string(self):
 
     def test_non_string_input(self):
         self.assertEqual(urlizetrunc(123, 1), '123')
+
+    def test_autoescape(self):
+        self.assertEqual(
+            urlizetrunc('foo<a href=" google.com ">bar</a>buz', 10),
+            'foo&lt;a href=&quot; <a href="http://google.com" rel="nofollow">google.com</a> &quot;&gt;bar&lt;/a&gt;buz',
+        )
+
+    def test_autoescape_off(self):
+        self.assertEqual(
+            urlizetrunc('foo<a href=" google.com ">bar</a>buz', 9, autoescape=False),
+            'foo<a href=" <a href="http://google.com" rel="nofollow">google...</a> ">bar</a>buz',
+        )