Fixed #23831 -- Supported strings escaped by third-party libs in Django.

Refs #7261 -- Made strings escaped by Django usable in third-party libs.

The changes in mark_safe and mark_for_escaping are straightforward. The
more tricky part is to handle correctly objects that implement __html__.

Historically escape() has escaped SafeData. Even if that doesn't seem a
good behavior, changing it would create security concerns. Therefore
support for __html__() was only added to conditional_escape() where this
concern doesn't exist.

Then using conditional_escape() instead of escape() in the Django
template engine makes it understand data escaped by other libraries.

Template filter |escape accounts for __html__() when it's available.
|force_escape forces the use of Django's HTML escaping implementation.

Here's why the change in render_value_in_context() is safe. Before Django
1.7 conditional_escape() was implemented as follows:

    if isinstance(text, SafeData):
        return text
    else:
        return escape(text)

render_value_in_context() never called escape() on SafeData. Therefore
replacing escape() with conditional_escape() doesn't change the
autoescaping logic as it was originally intended.

This change should be backported to Django 1.7 because it corrects a
feature added in Django 1.7.

Thanks mitsuhiko for the report.

5 files changed, 44 insertions, 12 deletions

diff --git a/django/template/base.py b/django/template/base.py
index 8a9099baa5..4ac20b13e1 100644
--- a/django/template/base.py
+++ b/django/template/base.py
@@ -19,7 +19,7 @@ from django.utils.translation import ugettext_lazy, pgettext_lazy
 from django.utils.safestring import (SafeData, EscapeData, mark_safe,
     mark_for_escaping)
 from django.utils.formats import localize
-from django.utils.html import escape
+from django.utils.html import conditional_escape
 from django.utils.module_loading import module_has_submodule
 from django.utils import six
 from django.utils.timezone import template_localtime
@@ -887,7 +887,7 @@ def render_value_in_context(value, context):
     value = force_text(value)
     if ((context.autoescape and not isinstance(value, SafeData)) or
             isinstance(value, EscapeData)):
-        return escape(value)
+        return conditional_escape(value)
     else:
         return value
 
diff --git a/django/template/debug.py b/django/template/debug.py
index cf9c2a59fc..465d7ff926 100644
--- a/django/template/debug.py
+++ b/django/template/debug.py
@@ -1,6 +1,6 @@
 from django.template.base import Lexer, Parser, tag_re, NodeList, VariableNode, TemplateSyntaxError
 from django.utils.encoding import force_text
-from django.utils.html import escape
+from django.utils.html import conditional_escape
 from django.utils.safestring import SafeData, EscapeData
 from django.utils.formats import localize
 from django.utils.timezone import template_localtime
@@ -98,6 +98,6 @@ class DebugVariableNode(VariableNode):
                 e.django_template_source = self.source
             raise
         if (context.autoescape and not isinstance(output, SafeData)) or isinstance(output, EscapeData):
-            return escape(output)
+            return conditional_escape(output)
         else:
             return output
diff --git a/django/utils/html.py b/django/utils/html.py
index 3c03210c11..a596662d22 100644
--- a/django/utils/html.py
+++ b/django/utils/html.py
@@ -44,6 +44,10 @@ def escape(text):
     """
     Returns the given text with ampersands, quotes and angle brackets encoded
     for use in HTML.
+
+    This function always escapes its input, even if it's already escaped and
+    marked as such. This may result in double-escaping. If this is a concern,
+    use conditional_escape() instead.
     """
     return mark_safe(force_text(text).replace('&', '&amp;').replace('<', '&lt;')
         .replace('>', '&gt;').replace('"', '&quot;').replace("'", '&#39;'))
@@ -76,6 +80,9 @@ escapejs = allow_lazy(escapejs, six.text_type, SafeText)
 def conditional_escape(text):
     """
     Similar to escape(), except that it doesn't operate on pre-escaped strings.
+
+    This function relies on the __html__ convention used both by Django's
+    SafeData class and by third-party libraries like markupsafe.
     """
     if hasattr(text, '__html__'):
         return text.__html__()
diff --git a/django/utils/safestring.py b/django/utils/safestring.py
index 50b0c03686..ab4d8149c9 100644
--- a/django/utils/safestring.py
+++ b/django/utils/safestring.py
@@ -36,9 +36,9 @@ else:
 class SafeData(object):
     def __html__(self):
         """
-        Returns the html representation of a string.
+        Returns the html representation of a string for interoperability.
 
-        Allows interoperability with other template engines.
+        This allows other template engines to understand Django's SafeData.
         """
         return self
 
@@ -121,7 +121,7 @@ def mark_safe(s):
 
     Can be called multiple times on a single string.
     """
-    if isinstance(s, SafeData):
+    if hasattr(s, '__html__'):
         return s
     if isinstance(s, bytes) or (isinstance(s, Promise) and s._delegate_bytes):
         return SafeBytes(s)
@@ -138,7 +138,7 @@ def mark_for_escaping(s):
     Can be called multiple times on a single string (the resulting escaping is
     only applied once).
     """
-    if isinstance(s, (SafeData, EscapeData)):
+    if hasattr(s, '__html__') or isinstance(s, EscapeData):
         return s
     if isinstance(s, bytes) or (isinstance(s, Promise) and s._delegate_bytes):
         return EscapeBytes(s)
diff --git a/tests/utils_tests/test_safestring.py b/tests/utils_tests/test_safestring.py
index 053d9f42fa..e23851815b 100644
--- a/tests/utils_tests/test_safestring.py
+++ b/tests/utils_tests/test_safestring.py
@@ -13,6 +13,13 @@ lazystr = lazy(force_text, six.text_type)
 lazybytes = lazy(force_bytes, bytes)
 
 
+class customescape(six.text_type):
+    def __html__(self):
+        # implement specific and obviously wrong escaping
+        # in order to be able to tell for sure when it runs
+        return self.replace('<', '<<').replace('>', '>>')
+
+
 class SafeStringTest(TestCase):
     def assertRenderEqual(self, tpl, expected, **context):
         context = Context(context)
@@ -25,6 +32,14 @@ class SafeStringTest(TestCase):
         self.assertRenderEqual('{{ s }}', 'a&b', s=s)
         self.assertRenderEqual('{{ s|force_escape }}', 'a&amp;b', s=s)
 
+    def test_mark_safe_object_implementing_dunder_html(self):
+        e = customescape('<a&b>')
+        s = mark_safe(e)
+        self.assertIs(s, e)
+
+        self.assertRenderEqual('{{ s }}', '<<a&b>>', s=s)
+        self.assertRenderEqual('{{ s|force_escape }}', '&lt;a&amp;b&gt;', s=s)
+
     def test_mark_safe_lazy(self):
         s = lazystr('a&b')
         b = lazybytes(b'a&b')
@@ -42,11 +57,25 @@ class SafeStringTest(TestCase):
 
         self.assertRenderEqual('{{ s }}', '<obj>', s=s)
 
+    def test_mark_safe_result_implements_dunder_html(self):
+        self.assertEqual(mark_safe('a&b').__html__(), 'a&b')
+
+    def test_mark_safe_lazy_result_implements_dunder_html(self):
+        self.assertEqual(mark_safe(lazystr('a&b')).__html__(), 'a&b')
+
     def test_mark_for_escaping(self):
         s = mark_for_escaping('a&b')
         self.assertRenderEqual('{{ s }}', 'a&amp;b', s=s)
         self.assertRenderEqual('{{ s }}', 'a&amp;b', s=mark_for_escaping(s))
 
+    def test_mark_for_escaping_object_implementing_dunder_html(self):
+        e = customescape('<a&b>')
+        s = mark_for_escaping(e)
+        self.assertIs(s, e)
+
+        self.assertRenderEqual('{{ s }}', '<<a&b>>', s=s)
+        self.assertRenderEqual('{{ s|force_escape }}', '&lt;a&amp;b&gt;', s=s)
+
     def test_mark_for_escaping_lazy(self):
         s = lazystr('a&b')
         b = lazybytes(b'a&b')
@@ -55,10 +84,6 @@ class SafeStringTest(TestCase):
         self.assertIsInstance(mark_for_escaping(b), EscapeData)
         self.assertRenderEqual('{% autoescape off %}{{ s }}{% endautoescape %}', 'a&amp;b', s=mark_for_escaping(s))
 
-    def test_html(self):
-        s = '<h1>interop</h1>'
-        self.assertEqual(s, mark_safe(s).__html__())
-
     def test_mark_for_escaping_object_implementing_dunder_str(self):
         class Obj(object):
             def __str__(self):