Merge pull request #2382 from erikr/deprecate-fix-ampersands

Fixed #22130 -- Deprecated fix_ampersands and clean_html.

django/utils/html.py

@@ -3,6 +3,7 @@
 from __future__ import unicode_literals
 
 import re
+import warnings
 
 from django.utils.safestring import SafeData, mark_safe
 from django.utils.encoding import force_text, force_str
@@ -174,6 +175,9 @@ strip_entities = allow_lazy(strip_entities, six.text_type)
 
 def fix_ampersands(value):
     """Returns the given HTML with all unencoded ampersands encoded correctly."""
+    # As fix_ampersands is wrapped in allow_lazy, stacklevel 3 is more useful than 2.
+    warnings.warn("The fix_ampersands function is deprecated and will be removed in Django 1.8.",
+                  DeprecationWarning, stacklevel=3)
     return unencoded_ampersands_re.sub('&', force_text(value))
 fix_ampersands = allow_lazy(fix_ampersands, six.text_type)
 
@@ -290,6 +294,9 @@ def clean_html(text):
         * Remove stuff like "<p>&nbsp;&nbsp;</p>", but only if it's at the
           bottom of the text.
     """
+    # As clean_html is wrapped in allow_lazy, stacklevel 3 is more useful than 2.
+    warnings.warn("The clean_html function is deprecated and will be removed in Django 1.8.",
+                  DeprecationWarning, stacklevel=3)
     text = normalize_newlines(text)
     text = re.sub(r'<(/?)\s*b\s*>', '<\\1strong>', text)
     text = re.sub(r'<(/?)\s*i\s*>', '<\\1em>', text)

docs/internals/deprecation.txt

@@ -209,6 +209,9 @@ details on these changes.
   (``django.contrib.gis.sitemaps.views.index`` and
   ``django.contrib.gis.sitemaps.views.sitemap``).
 
+* ``django.utils.html.fix_ampersands``, the ``fix_ampersands`` template filter and
+  ``django.utils.html.clean_html`` will be removed following an accelerated deprecation.
+
 .. _deprecation-removed-in-1.7:
 
 1.7

docs/ref/templates/builtins.txt

@@ -1582,6 +1582,9 @@ fix_ampersands
     This is rarely useful as ampersands are automatically escaped. See
     :tfilter:`escape` for more information.
 
+.. deprecated:: 1.7
+    This filter has been deprecated and will be removed in Django 1.8.
+
 Replaces ampersands with ``&`` entities.
 
 For example::

docs/releases/1.7.txt

@@ -1406,3 +1406,16 @@ strings, you should use ``django.utils.html.escapejs`` or the
 :tfilter:`escapejs` template filter.
 If all you need is to generate valid javascript strings, you can simply use
 ``json.dumps()``.
+
+``fix_ampersands`` utils method and template filter
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+The ``django.utils.html.fix_ampersands`` method and the :tfilter:`fix_ampersands`
+template filter are deprecated, as the escaping of ampersands is already taken care
+of by Django's standard HTML escaping features. Combining this with ``fix_ampersands``
+would either result in double escaping, or, if the output is assumed to be safe,
+a risk of introducing XSS vulnerabilities. Along with ``fix_ampersands``,
+``django.utils.html.clean_html`` is deprecated, an undocumented function that calls
+``fix_ampersands``.
+As this is an accelerated deprecation, ``fix_ampersands`` and ``clean_html``
+will be removed in Django 1.8.

tests/defaultfilters/tests.py

@@ -4,6 +4,7 @@ from __future__ import unicode_literals
 import datetime
 import decimal
 import unittest
+import warnings
 
 from django.template.defaultfilters import (
     add, addslashes, capfirst, center, cut, date, default, default_if_none,
@@ -124,8 +125,11 @@ class DefaultFiltersTests(TestCase):
             'paragraph separator:\\u2029and line separator:\\u2028')
 
     def test_fix_ampersands(self):
-        self.assertEqual(fix_ampersands_filter('Jack & Jill & Jeroboam'),
-                         'Jack & Jill & Jeroboam')
+        with warnings.catch_warnings(record=True) as w:
+            warnings.simplefilter("always", DeprecationWarning)
+            self.assertEqual(fix_ampersands_filter('Jack & Jill & Jeroboam'),
+                             'Jack & Jill & Jeroboam')
+        self.assertEqual(len(w), 1)
 
     def test_linenumbers(self):
         self.assertEqual(linenumbers('line 1\nline 2'),

tests/template_tests/tests.py

@@ -607,7 +607,10 @@ class TemplateTests(TestCase):
                                     failures.append("Template test (Cached='%s', TEMPLATE_STRING_IF_INVALID='%s', TEMPLATE_DEBUG=%s): %s -- FAILED. Template loading invoked method that shouldn't have been invoked." % (is_cached, invalid_str, template_debug, name))
 
                                 try:
-                                    output = self.render(test_template, vals)
+                                    with warnings.catch_warnings():
+                                        # Ignore deprecation of fix_ampersands
+                                        warnings.filterwarnings("ignore", category=DeprecationWarning, module='django.template.defaultfilters')
+                                        output = self.render(test_template, vals)
                                 except ShouldNotExecuteException:
                                     failures.append("Template test (Cached='%s', TEMPLATE_STRING_IF_INVALID='%s', TEMPLATE_DEBUG=%s): %s -- FAILED. Template rendering invoked method that shouldn't have been invoked." % (is_cached, invalid_str, template_debug, name))
                             except ContextStackException:

tests/utils_tests/test_html.py

@@ -4,6 +4,7 @@ from __future__ import unicode_literals
 from datetime import datetime
 import os
 from unittest import TestCase
+import warnings
 
 from django.utils import html, safestring
 from django.utils._os import upath
@@ -130,25 +131,29 @@ class TestUtilsHtml(TestCase):
                 self.check_output(f, in_pattern % {'entity': entity}, output)
 
     def test_fix_ampersands(self):
-        f = html.fix_ampersands
-        # Strings without ampersands or with ampersands already encoded.
-        values = ("a", "b", "&a;", "& &x; ", "asdf")
-        patterns = (
-            ("%s", "%s"),
-            ("&%s", "&%s"),
-            ("&%s&", "&%s&"),
-        )
-        for value in values:
-            for in_pattern, out_pattern in patterns:
-                self.check_output(f, in_pattern % value, out_pattern % value)
-        # Strings with ampersands that need encoding.
-        items = (
-            ("&#;", "&#;"),
-            ("&#875 ;", "&#875 ;"),
-            ("&#4abc;", "&#4abc;"),
-        )
-        for value, output in items:
-            self.check_output(f, value, output)
+        with warnings.catch_warnings():
+            warnings.simplefilter("ignore", DeprecationWarning)
+            f = html.fix_ampersands
+            # Strings without ampersands or with ampersands already encoded.
+            values = ("a", "b", "&a;", "& &x; ", "asdf")
+            patterns = (
+                ("%s", "%s"),
+                ("&%s", "&%s"),
+                ("&%s&", "&%s&"),
+            )
+
+            for value in values:
+                for in_pattern, out_pattern in patterns:
+                    self.check_output(f, in_pattern % value, out_pattern % value)
+
+            # Strings with ampersands that need encoding.
+            items = (
+                ("&#;", "&#;"),
+                ("&#875 ;", "&#875 ;"),
+                ("&#4abc;", "&#4abc;"),
+            )
+            for value, output in items:
+                self.check_output(f, value, output)
 
     def test_escapejs(self):
         f = html.escapejs
@@ -171,8 +176,10 @@ class TestUtilsHtml(TestCase):
             # also a regression test for #7267: this used to raise an UnicodeDecodeError
             ('<p>* foo</p><p>* bar</p>', '<ul>\n<li> foo</li><li> bar</li>\n</ul>'),
         )
-        for value, output in items:
-            self.check_output(f, value, output)
+        with warnings.catch_warnings():
+            warnings.simplefilter("ignore", DeprecationWarning)
+            for value, output in items:
+                self.check_output(f, value, output)
 
     def test_remove_tags(self):
         f = html.remove_tags