From 3483682749577b4b5a8141a766489d5b460e30e9 Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Tue, 23 Dec 2014 22:29:01 +0100 Subject: [PATCH] [1.7.x] 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. Backport of 6d52f6f from master. --- django/template/base.py | 4 ++-- django/template/debug.py | 4 ++-- django/utils/html.py | 10 ++++++++- django/utils/safestring.py | 8 +++---- docs/releases/1.7.2.txt | 3 +++ tests/utils_tests/test_safestring.py | 33 ++++++++++++++++++++++++---- 6 files changed, 49 insertions(+), 13 deletions(-) diff --git a/django/template/base.py b/django/template/base.py index e829604418..822e026b8b 100644 --- a/django/template/base.py +++ b/django/template/base.py @@ -17,7 +17,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 @@ -881,7 +881,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 569810c6cc..b6b4e1cbd7 100644 --- a/django/utils/html.py +++ b/django/utils/html.py @@ -36,7 +36,12 @@ trailing_empty_content_re = re.compile(r'(?:

(?: |\s|
)*?

\s*)+\ def escape(text): """ - Returns the given text with ampersands, quotes and angle brackets encoded for use in HTML. + 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('&', '&').replace('<', '<').replace('>', '>').replace('"', '"').replace("'", ''')) escape = allow_lazy(escape, six.text_type) @@ -68,6 +73,9 @@ escapejs = allow_lazy(escapejs, six.text_type) 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/docs/releases/1.7.2.txt b/docs/releases/1.7.2.txt index 84188cd494..da6721a1ba 100644 --- a/docs/releases/1.7.2.txt +++ b/docs/releases/1.7.2.txt @@ -178,3 +178,6 @@ Bugfixes * Restored support for objects that aren't :class:`str` or :class:`bytes` in :func:`~django.utils.safestring.mark_for_escaping` on Python 3. + +* Supported strings escaped by third-party libraries with the ``__html__`` + convention in the template engine (:ticket:`23831`). diff --git a/tests/utils_tests/test_safestring.py b/tests/utils_tests/test_safestring.py index aaa4a07d1c..e5d6506b9c 100644 --- a/tests/utils_tests/test_safestring.py +++ b/tests/utils_tests/test_safestring.py @@ -11,6 +11,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) @@ -23,6 +30,14 @@ class SafeStringTest(TestCase): self.assertRenderEqual('{{ s }}', 'a&b', s=s) self.assertRenderEqual('{{ s|force_escape }}', 'a&b', s=s) + def test_mark_safe_object_implementing_dunder_html(self): + e = customescape('') + s = mark_safe(e) + self.assertIs(s, e) + + self.assertRenderEqual('{{ s }}', '<>', s=s) + self.assertRenderEqual('{{ s|force_escape }}', '<a&b>', s=s) + def test_mark_safe_lazy(self): s = lazystr('a&b') b = lazybytes(b'a&b') @@ -40,11 +55,25 @@ class SafeStringTest(TestCase): self.assertRenderEqual('{{ s }}', '', 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&b', s=s) self.assertRenderEqual('{{ s }}', 'a&b', s=mark_for_escaping(s)) + def test_mark_for_escaping_object_implementing_dunder_html(self): + e = customescape('') + s = mark_for_escaping(e) + self.assertIs(s, e) + + self.assertRenderEqual('{{ s }}', '<>', s=s) + self.assertRenderEqual('{{ s|force_escape }}', '<a&b>', s=s) + def test_mark_for_escaping_lazy(self): s = lazystr('a&b') b = lazybytes(b'a&b') @@ -53,10 +82,6 @@ class SafeStringTest(TestCase): self.assertIsInstance(mark_for_escaping(b), EscapeData) self.assertRenderEqual('{% autoescape off %}{{ s }}{% endautoescape %}', 'a&b', s=mark_for_escaping(s)) - def test_html(self): - s = '

interop

' - self.assertEqual(s, mark_safe(s).__html__()) - def test_mark_for_escaping_object_implementing_dunder_str(self): class Obj(object): def __str__(self):