From 1f2abf784a9fe550959de242d91963b2ad6f7e9c Mon Sep 17 00:00:00 2001 From: Moritz Sichert Date: Wed, 18 Mar 2015 21:42:59 +0100 Subject: [PATCH] Fixed #24469 -- Refined escaping of Django's form elements in non-Django templates. --- AUTHORS | 1 + django/contrib/gis/maps/google/overlays.py | 8 ++- django/forms/forms.py | 10 +-- django/forms/formsets.py | 2 + django/forms/utils.py | 4 +- django/forms/widgets.py | 9 +-- django/utils/html.py | 31 ++++++++++ docs/ref/utils.txt | 14 +++++ tests/forms_tests/tests/test_forms.py | 10 +++ tests/forms_tests/tests/test_formsets.py | 6 ++ tests/forms_tests/tests/test_media.py | 6 ++ tests/forms_tests/tests/test_utils.py | 13 +++- tests/forms_tests/tests/test_widgets.py | 21 ++++++- tests/gis_tests/maps/tests.py | 13 ++++ tests/utils_tests/test_html.py | 71 ++++++++++++++++++++-- 15 files changed, 198 insertions(+), 21 deletions(-) diff --git a/AUTHORS b/AUTHORS index 3b3748ef82..420d109905 100644 --- a/AUTHORS +++ b/AUTHORS @@ -498,6 +498,7 @@ answer newbie questions, and generally made Django that much better: mitakummaa@gmail.com mmarshall Moayad Mardini + Moritz Sichert Morten Bagai msaelices msundstr diff --git a/django/contrib/gis/maps/google/overlays.py b/django/contrib/gis/maps/google/overlays.py index f249e5d809..ac0a6bd315 100644 --- a/django/contrib/gis/maps/google/overlays.py +++ b/django/contrib/gis/maps/google/overlays.py @@ -6,9 +6,10 @@ from django.contrib.gis.geos import ( from django.utils import six from django.utils.encoding import python_2_unicode_compatible from django.utils.functional import total_ordering -from django.utils.safestring import mark_safe +from django.utils.html import html_safe +@html_safe @python_2_unicode_compatible class GEvent(object): """ @@ -56,9 +57,10 @@ class GEvent(object): def __str__(self): "Returns the parameter part of a GEvent." - return mark_safe('"%s", %s' % (self.event, self.action)) + return '"%s", %s' % (self.event, self.action) +@html_safe @python_2_unicode_compatible class GOverlayBase(object): def __init__(self): @@ -74,7 +76,7 @@ class GOverlayBase(object): def __str__(self): "The string representation is the JavaScript API call." - return mark_safe('%s(%s)' % (self.__class__.__name__, self.js_params)) + return '%s(%s)' % (self.__class__.__name__, self.js_params) class GPolygon(GOverlayBase): diff --git a/django/forms/forms.py b/django/forms/forms.py index 24cc27d87c..c5b190bdf5 100644 --- a/django/forms/forms.py +++ b/django/forms/forms.py @@ -16,7 +16,7 @@ from django.utils import six from django.utils.encoding import ( force_text, python_2_unicode_compatible, smart_text, ) -from django.utils.html import conditional_escape, format_html +from django.utils.html import conditional_escape, format_html, html_safe from django.utils.safestring import mark_safe from django.utils.translation import ugettext as _ @@ -67,6 +67,7 @@ class DeclarativeFieldsMetaclass(MediaDefiningClass): return new_class +@html_safe @python_2_unicode_compatible class BaseForm(object): # This is the main implementation of all the Form logic. Note that this @@ -122,9 +123,6 @@ class BaseForm(object): fields.update(self.fields) # add remaining fields in original order self.fields = fields - def __html__(self): - return force_text(self) - def __str__(self): return self.as_table() @@ -504,6 +502,7 @@ class Form(six.with_metaclass(DeclarativeFieldsMetaclass, BaseForm)): # BaseForm itself has no way of designating self.fields. +@html_safe @python_2_unicode_compatible class BoundField(object): "A Field plus data" @@ -521,9 +520,6 @@ class BoundField(object): self.help_text = field.help_text or '' self._initial_value = UNSET - def __html__(self): - return force_text(self) - def __str__(self): """Renders this field as an HTML widget.""" if self.field.show_hidden_initial: diff --git a/django/forms/formsets.py b/django/forms/formsets.py index adacd00fdc..eafe35720f 100644 --- a/django/forms/formsets.py +++ b/django/forms/formsets.py @@ -8,6 +8,7 @@ from django.forms.widgets import HiddenInput from django.utils import six from django.utils.encoding import python_2_unicode_compatible from django.utils.functional import cached_property +from django.utils.html import html_safe from django.utils.safestring import mark_safe from django.utils.six.moves import range from django.utils.translation import ugettext as _, ungettext @@ -46,6 +47,7 @@ class ManagementForm(Form): super(ManagementForm, self).__init__(*args, **kwargs) +@html_safe @python_2_unicode_compatible class BaseFormSet(object): """ diff --git a/django/forms/utils.py b/django/forms/utils.py index ab0c80769b..9d41ba3639 100644 --- a/django/forms/utils.py +++ b/django/forms/utils.py @@ -7,7 +7,7 @@ from django.conf import settings from django.core.exceptions import ValidationError # backwards compatibility from django.utils import six, timezone from django.utils.encoding import force_text, python_2_unicode_compatible -from django.utils.html import escape, format_html, format_html_join +from django.utils.html import escape, format_html, format_html_join, html_safe from django.utils.translation import ugettext_lazy as _ try: @@ -40,6 +40,7 @@ def flatatt(attrs): ) +@html_safe @python_2_unicode_compatible class ErrorDict(dict): """ @@ -72,6 +73,7 @@ class ErrorDict(dict): return self.as_ul() +@html_safe @python_2_unicode_compatible class ErrorList(UserList, list): """ diff --git a/django/forms/widgets.py b/django/forms/widgets.py index 9a73ddb9ca..4f38418a93 100644 --- a/django/forms/widgets.py +++ b/django/forms/widgets.py @@ -18,7 +18,7 @@ from django.utils.encoding import ( force_str, force_text, python_2_unicode_compatible, ) from django.utils.formats import get_format -from django.utils.html import conditional_escape, format_html +from django.utils.html import conditional_escape, format_html, html_safe from django.utils.safestring import mark_safe from django.utils.six.moves import range from django.utils.six.moves.urllib.parse import urljoin @@ -37,6 +37,7 @@ __all__ = ( MEDIA_TYPES = ('css', 'js') +@html_safe @python_2_unicode_compatible class Media(object): def __init__(self, media=None, **kwargs): @@ -51,9 +52,6 @@ class Media(object): for name in MEDIA_TYPES: getattr(self, 'add_' + name)(media_attrs.get(name, None)) - def __html__(self): - return force_text(self) - def __str__(self): return self.render() @@ -159,6 +157,7 @@ class MediaDefiningClass(type): return new_class +@html_safe @python_2_unicode_compatible class SubWidget(object): """ @@ -602,6 +601,7 @@ class SelectMultiple(Select): return data.get(name, None) +@html_safe @python_2_unicode_compatible class ChoiceInput(SubWidget): """ @@ -667,6 +667,7 @@ class CheckboxChoiceInput(ChoiceInput): return self.choice_value in self.value +@html_safe @python_2_unicode_compatible class ChoiceFieldRenderer(object): """ diff --git a/django/utils/html.py b/django/utils/html.py index 1cf131b8a0..cafc3ab6e1 100644 --- a/django/utils/html.py +++ b/django/utils/html.py @@ -360,3 +360,34 @@ def avoid_wrapping(value): spaces where there previously were normal spaces. """ return value.replace(" ", "\xa0") + + +def html_safe(klass): + """ + A decorator that defines the __html__ method. This helps non-Django + templates to detect classes whose __str__ methods return SafeText. + """ + if '__html__' in klass.__dict__: + raise ValueError( + "can't apply @html_safe to %s because it defines " + "__html__()." % klass.__name__ + ) + if six.PY2: + if '__unicode__' not in klass.__dict__: + raise ValueError( + "can't apply @html_safe to %s because it doesn't " + "define __unicode__()." % klass.__name__ + ) + klass_unicode = klass.__unicode__ + klass.__unicode__ = lambda self: mark_safe(klass_unicode(self)) + klass.__html__ = lambda self: unicode(self) + else: + if '__str__' not in klass.__dict__: + raise ValueError( + "can't apply @html_safe to %s because it doesn't " + "define __str__()." % klass.__name__ + ) + klass_str = klass.__str__ + klass.__str__ = lambda self: mark_safe(klass_str(self)) + klass.__html__ = lambda self: str(self) + return klass diff --git a/docs/ref/utils.txt b/docs/ref/utils.txt index 166df30141..b69d11fc36 100644 --- a/docs/ref/utils.txt +++ b/docs/ref/utils.txt @@ -657,6 +657,20 @@ escaping HTML. .. _str.format: https://docs.python.org/library/stdtypes.html#str.format .. _bleach: https://pypi.python.org/pypi/bleach +.. function:: html_safe() + + .. versionadded:: 1.8 + + The ``__html__()`` method on a class helps non-Django templates detect + classes whose output doesn't require HTML escaping. + + This decorator defines the ``__html__()`` method on the decorated class + by wrapping the ``__unicode__()`` (Python 2) or ``__str__()`` (Python 3) + in :meth:`~django.utils.safestring.mark_safe`. Ensure the ``__unicode__()`` + or ``__str__()`` method does indeed return text that doesn't require HTML + escaping. + + ``django.utils.http`` ===================== diff --git a/tests/forms_tests/tests/test_forms.py b/tests/forms_tests/tests/test_forms.py index ef447a94e1..c713b1bf5b 100644 --- a/tests/forms_tests/tests/test_forms.py +++ b/tests/forms_tests/tests/test_forms.py @@ -2570,3 +2570,13 @@ class FormsTestCase(TestCase): self.assertFalse(FormWithoutFile().is_multipart()) self.assertTrue(FormWithFile().is_multipart()) self.assertTrue(FormWithImage().is_multipart()) + + def test_html_safe(self): + class SimpleForm(Form): + username = CharField() + + form = SimpleForm() + self.assertTrue(hasattr(SimpleForm, '__html__')) + self.assertEqual(force_text(form), form.__html__()) + self.assertTrue(hasattr(form['username'], '__html__')) + self.assertEqual(force_text(form['username']), form['username'].__html__()) diff --git a/tests/forms_tests/tests/test_formsets.py b/tests/forms_tests/tests/test_formsets.py index 7f450247eb..9285a83038 100644 --- a/tests/forms_tests/tests/test_formsets.py +++ b/tests/forms_tests/tests/test_formsets.py @@ -10,6 +10,7 @@ from django.forms import ( from django.forms.formsets import BaseFormSet, formset_factory from django.forms.utils import ErrorList from django.test import TestCase +from django.utils.encoding import force_text class Choice(Form): @@ -1093,6 +1094,11 @@ class FormsFormsetTestCase(TestCase): formset = ChoiceFormSet(data, auto_id=False, prefix='choices') self.assertEqual(formset.total_error_count(), 2) + def test_html_safe(self): + formset = self.make_choiceformset() + self.assertTrue(hasattr(formset, '__html__')) + self.assertEqual(force_text(formset), formset.__html__()) + data = { 'choices-TOTAL_FORMS': '1', # the number of forms rendered diff --git a/tests/forms_tests/tests/test_media.py b/tests/forms_tests/tests/test_media.py index 027e410f30..64d29c44cb 100644 --- a/tests/forms_tests/tests/test_media.py +++ b/tests/forms_tests/tests/test_media.py @@ -2,6 +2,7 @@ from django.forms import CharField, Form, Media, MultiWidget, TextInput from django.template import Context, Template from django.test import TestCase, override_settings +from django.utils.encoding import force_text @override_settings( @@ -455,6 +456,11 @@ class FormsMediaTestCase(TestCase): """) + def test_html_safe(self): + media = Media(css={'all': ['/path/to/css']}, js=['/path/to/js']) + self.assertTrue(hasattr(Media, '__html__')) + self.assertEqual(force_text(media), media.__html__()) + @override_settings( STATIC_URL='http://media.example.com/static/', diff --git a/tests/forms_tests/tests/test_utils.py b/tests/forms_tests/tests/test_utils.py index 4519963d96..7baf975bea 100644 --- a/tests/forms_tests/tests/test_utils.py +++ b/tests/forms_tests/tests/test_utils.py @@ -7,7 +7,7 @@ from django.core.exceptions import ValidationError from django.forms.utils import ErrorDict, ErrorList, flatatt from django.test import TestCase from django.utils import six -from django.utils.encoding import python_2_unicode_compatible +from django.utils.encoding import force_text, python_2_unicode_compatible from django.utils.safestring import mark_safe from django.utils.translation import ugettext_lazy @@ -131,3 +131,14 @@ class FormsUtilsTestCase(TestCase): e_deepcopy = copy.deepcopy(e) self.assertEqual(e, e_deepcopy) self.assertEqual(e.as_data(), e_copy.as_data()) + + def test_error_dict_html_safe(self): + e = ErrorDict() + e['username'] = 'Invalid username.' + self.assertTrue(hasattr(ErrorDict, '__html__')) + self.assertEqual(force_text(e), e.__html__()) + + def test_error_list_html_safe(self): + e = ErrorList(['Invalid username.']) + self.assertTrue(hasattr(ErrorList, '__html__')) + self.assertEqual(force_text(e), e.__html__()) diff --git a/tests/forms_tests/tests/test_widgets.py b/tests/forms_tests/tests/test_widgets.py index 93384737f8..62a14dfa5c 100644 --- a/tests/forms_tests/tests/test_widgets.py +++ b/tests/forms_tests/tests/test_widgets.py @@ -15,7 +15,9 @@ from django.forms import ( RadioSelect, Select, SelectDateWidget, SelectMultiple, SplitDateTimeField, SplitDateTimeWidget, Textarea, TextInput, TimeInput, ValidationError, ) -from django.forms.widgets import RadioFieldRenderer +from django.forms.widgets import ( + ChoiceFieldRenderer, ChoiceInput, RadioFieldRenderer, +) from django.test import TestCase, override_settings from django.utils import six, translation from django.utils.dates import MONTHS_AP @@ -1147,6 +1149,23 @@ beatle J R Ringo False""") ) self.assertEqual(f.cleaned_data['field1'], 'some text,JP,2007-04-25 06:24:00') + def test_sub_widget_html_safe(self): + widget = TextInput() + subwidget = next(widget.subwidgets('username', 'John Doe')) + self.assertTrue(hasattr(subwidget, '__html__')) + self.assertEqual(force_text(subwidget), subwidget.__html__()) + + def test_choice_input_html_safe(self): + widget = ChoiceInput('choices', 'CHOICE1', {}, ('CHOICE1', 'first choice'), 0) + self.assertTrue(hasattr(ChoiceInput, '__html__')) + self.assertEqual(force_text(widget), widget.__html__()) + + def test_choice_field_renderer_html_safe(self): + renderer = ChoiceFieldRenderer('choices', 'CHOICE1', {}, [('CHOICE1', 'first_choice')]) + renderer.choice_input_class = lambda *args: args + self.assertTrue(hasattr(ChoiceFieldRenderer, '__html__')) + self.assertEqual(force_text(renderer), renderer.__html__()) + class NullBooleanSelectLazyForm(Form): """Form to test for lazy evaluation. Refs #17190""" diff --git a/tests/gis_tests/maps/tests.py b/tests/gis_tests/maps/tests.py index 0e9635e8ef..991109af5c 100644 --- a/tests/gis_tests/maps/tests.py +++ b/tests/gis_tests/maps/tests.py @@ -4,8 +4,10 @@ from __future__ import unicode_literals from unittest import skipUnless from django.contrib.gis.geos import HAS_GEOS +from django.contrib.gis.maps.google.overlays import GEvent, GOverlayBase from django.test import TestCase from django.test.utils import modify_settings, override_settings +from django.utils.encoding import force_text GOOGLE_MAPS_API_KEY = 'XXXX' @@ -41,3 +43,14 @@ class GoogleMapsTest(TestCase): title='En français !') google_map = GoogleMap(center=center, zoom=18, markers=[marker]) self.assertIn("En français", google_map.scripts) + + def test_gevent_html_safe(self): + event = GEvent('click', 'function() {location.href = "http://www.google.com"}') + self.assertTrue(hasattr(GEvent, '__html__')) + self.assertEqual(force_text(event), event.__html__()) + + def test_goverlay_html_safe(self): + overlay = GOverlayBase() + overlay.js_params = '"foo", "bar"' + self.assertTrue(hasattr(GOverlayBase, '__html__')) + self.assertEqual(force_text(overlay), overlay.__html__()) diff --git a/tests/utils_tests/test_html.py b/tests/utils_tests/test_html.py index 7456b67d50..58a4d48753 100644 --- a/tests/utils_tests/test_html.py +++ b/tests/utils_tests/test_html.py @@ -3,16 +3,15 @@ from __future__ import unicode_literals import os from datetime import datetime -from unittest import TestCase -from django.test import ignore_warnings -from django.utils import html, safestring +from django.test import SimpleTestCase, ignore_warnings +from django.utils import html, safestring, six from django.utils._os import upath from django.utils.deprecation import RemovedInDjango20Warning from django.utils.encoding import force_text -class TestUtilsHtml(TestCase): +class TestUtilsHtml(SimpleTestCase): def check_output(self, function, value, output=None): """ @@ -185,3 +184,67 @@ class TestUtilsHtml(TestCase): self.assertEqual(html.conditional_escape(s), '<h1>interop</h1>') self.assertEqual(html.conditional_escape(safestring.mark_safe(s)), s) + + def test_html_safe(self): + @html.html_safe + class HtmlClass(object): + if six.PY2: + def __unicode__(self): + return "

I'm a html class!

" + else: + def __str__(self): + return "

I'm a html class!

" + + html_obj = HtmlClass() + self.assertTrue(hasattr(HtmlClass, '__html__')) + self.assertTrue(hasattr(html_obj, '__html__')) + self.assertEqual(force_text(html_obj), html_obj.__html__()) + + def test_html_safe_subclass(self): + if six.PY2: + class BaseClass(object): + def __html__(self): + # defines __html__ on its own + return 'some html content' + + def __unicode__(self): + return 'some non html content' + + @html.html_safe + class Subclass(BaseClass): + def __unicode__(self): + # overrides __unicode__ and is marked as html_safe + return 'some html safe content' + else: + class BaseClass(object): + def __html__(self): + # defines __html__ on its own + return 'some html content' + + def __str__(self): + return 'some non html content' + + @html.html_safe + class Subclass(BaseClass): + def __str__(self): + # overrides __str__ and is marked as html_safe + return 'some html safe content' + + subclass_obj = Subclass() + self.assertEqual(force_text(subclass_obj), subclass_obj.__html__()) + + def test_html_safe_defines_html_error(self): + msg = "can't apply @html_safe to HtmlClass because it defines __html__()." + with self.assertRaisesMessage(ValueError, msg): + @html.html_safe + class HtmlClass(object): + def __html__(self): + return "

I'm a html class!

" + + def test_html_safe_doesnt_define_str(self): + method_name = '__unicode__()' if six.PY2 else '__str__()' + msg = "can't apply @html_safe to HtmlClass because it doesn't define %s." % method_name + with self.assertRaisesMessage(ValueError, msg): + @html.html_safe + class HtmlClass(object): + pass