diff --git a/django/contrib/admin/helpers.py b/django/contrib/admin/helpers.py index c9121c7b36..e673199e73 100644 --- a/django/contrib/admin/helpers.py +++ b/django/contrib/admin/helpers.py @@ -14,7 +14,9 @@ from django.db.models.fields.related import ManyToManyRel from django.forms.utils import flatatt from django.template.defaultfilters import capfirst, linebreaksbr from django.utils import six -from django.utils.deprecation import RemovedInDjango110Warning +from django.utils.deprecation import ( + RemovedInDjango20Warning, RemovedInDjango110Warning, +) from django.utils.encoding import force_text, smart_text from django.utils.functional import cached_property from django.utils.html import conditional_escape, format_html @@ -198,11 +200,20 @@ class AdminReadonlyField(object): if boolean: result_repr = _boolean_icon(value) else: - result_repr = smart_text(value) - if getattr(attr, "allow_tags", False): - result_repr = mark_safe(result_repr) + if hasattr(value, "__html__"): + result_repr = value else: - result_repr = linebreaksbr(result_repr) + result_repr = smart_text(value) + if getattr(attr, "allow_tags", False): + warnings.warn( + "Deprecated allow_tags attribute used on %s. " + "Use django.utils.safestring.format_html(), " + "format_html_join(), or mark_safe() instead." % attr, + RemovedInDjango20Warning + ) + result_repr = mark_safe(value) + else: + result_repr = linebreaksbr(result_repr) else: if isinstance(f.remote_field, ManyToManyRel) and value is not None: result_repr = ", ".join(map(six.text_type, value.all())) diff --git a/django/contrib/admin/options.py b/django/contrib/admin/options.py index 935f4f4b66..348a56a4f0 100644 --- a/django/contrib/admin/options.py +++ b/django/contrib/admin/options.py @@ -753,7 +753,6 @@ class ModelAdmin(BaseModelAdmin): """ return helpers.checkbox.render(helpers.ACTION_CHECKBOX_NAME, force_text(obj.pk)) action_checkbox.short_description = mark_safe('') - action_checkbox.allow_tags = True def get_actions(self, request): """ diff --git a/django/contrib/admin/templatetags/admin_list.py b/django/contrib/admin/templatetags/admin_list.py index 3332fe1a35..f4033e235b 100644 --- a/django/contrib/admin/templatetags/admin_list.py +++ b/django/contrib/admin/templatetags/admin_list.py @@ -1,6 +1,7 @@ from __future__ import unicode_literals import datetime +import warnings from django.contrib.admin.templatetags.admin_static import static from django.contrib.admin.templatetags.admin_urls import add_preserved_filters @@ -16,6 +17,7 @@ from django.db import models from django.template import Library from django.template.loader import get_template from django.utils import formats +from django.utils.deprecation import RemovedInDjango20Warning from django.utils.encoding import force_text from django.utils.html import escapejs, format_html from django.utils.safestring import mark_safe @@ -207,12 +209,14 @@ def items_for_result(cl, result, form): row_classes = ['action-checkbox'] allow_tags = getattr(attr, 'allow_tags', False) boolean = getattr(attr, 'boolean', False) - if boolean or not value: - allow_tags = True result_repr = display_for_value(value, empty_value_display, boolean) - # Strip HTML tags in the resulting text, except if the - # function has an "allow_tags" attribute set to True. if allow_tags: + warnings.warn( + "Deprecated allow_tags attribute used on field {}. " + "Use django.utils.safestring.format_html(), " + "format_html_join(), or mark_safe() instead.".format(field_name), + RemovedInDjango20Warning + ) result_repr = mark_safe(result_repr) if isinstance(value, (datetime.date, datetime.time)): row_classes.append('nowrap') diff --git a/docs/internals/deprecation.txt b/docs/internals/deprecation.txt index 96ad90dd92..f37ec25179 100644 --- a/docs/internals/deprecation.txt +++ b/docs/internals/deprecation.txt @@ -265,6 +265,9 @@ details on these changes. * The warning that :class:`~django.core.signing.Signer` issues when given an invalid separator will become an exception. +* Support for the ``allow_tags`` attribute on ``ModelAdmin`` methods will be + removed. + .. _deprecation-removed-in-1.9: 1.9 diff --git a/docs/ref/contrib/admin/index.txt b/docs/ref/contrib/admin/index.txt index 236841dc69..db977b74bd 100644 --- a/docs/ref/contrib/admin/index.txt +++ b/docs/ref/contrib/admin/index.txt @@ -583,11 +583,9 @@ subclass:: ``False``. * If the string given is a method of the model, ``ModelAdmin`` or a - callable, Django will HTML-escape the output by default. If you'd - rather not escape the output of the method, give the method an - ``allow_tags`` attribute whose value is ``True``. However, to avoid an - XSS vulnerability, you should use :func:`~django.utils.html.format_html` - to escape user-provided inputs. + callable, Django will HTML-escape the output by default. To escape + user input and allow your own unescaped tags, use + :func:`~django.utils.html.format_html`. Here's a full example model:: @@ -606,11 +604,17 @@ subclass:: self.first_name, self.last_name) - colored_name.allow_tags = True - class PersonAdmin(admin.ModelAdmin): list_display = ('first_name', 'last_name', 'colored_name') + .. deprecated:: 1.9 + + In older versions, you could add an ``allow_tags`` attribute to the + method to prevent auto-escaping. This attribute is deprecated as it's + safer to use :func:`~django.utils.html.format_html`, + :func:`~django.utils.html.format_html_join`, or + :func:`~django.utils.safestring.mark_safe` instead. + * If the value of a field is ``None``, an empty string, or an iterable without elements, Django will display ``-`` (a dash). You can override this with :attr:`AdminSite.empty_value_display`:: @@ -688,7 +692,6 @@ subclass:: self.color_code, self.first_name) - colored_first_name.allow_tags = True colored_first_name.admin_order_field = 'first_name' class PersonAdmin(admin.ModelAdmin): @@ -1095,12 +1098,10 @@ subclass:: mark_safe('
'), '{}', ((line,) for line in instance.get_full_address()), - ) or "I can't determine this address." + ) or mark_safe("I can't determine this address.") # short_description functions like a model field's verbose_name address_report.short_description = "Address" - # in this example, we have used HTML tags in the output - address_report.allow_tags = True .. attribute:: ModelAdmin.save_as diff --git a/docs/releases/1.9.txt b/docs/releases/1.9.txt index 2bf591b8b0..366e1ac934 100644 --- a/docs/releases/1.9.txt +++ b/docs/releases/1.9.txt @@ -1212,6 +1212,12 @@ Miscellaneous ``SimpleTestCase.assertRaisesMessage()`` is deprecated. Pass the callable as a positional argument instead. +* The ``allow_tags`` attribute on methods of ``ModelAdmin`` has been + deprecated. Use :func:`~django.utils.html.format_html`, + :func:`~django.utils.html.format_html_join`, or + :func:`~django.utils.safestring.mark_safe` when constructing the method's + return value instead. + .. removed-features-1.9: Features removed in 1.9 diff --git a/tests/admin_changelist/tests.py b/tests/admin_changelist/tests.py index 86682581b9..5e773bcb9a 100644 --- a/tests/admin_changelist/tests.py +++ b/tests/admin_changelist/tests.py @@ -12,9 +12,10 @@ from django.contrib.auth.models import User from django.contrib.contenttypes.models import ContentType from django.core.urlresolvers import reverse from django.template import Context, Template -from django.test import TestCase, override_settings +from django.test import TestCase, ignore_warnings, override_settings from django.test.client import RequestFactory from django.utils import formats, six +from django.utils.deprecation import RemovedInDjango20Warning from .admin import ( BandAdmin, ChildAdmin, ChordsBandAdmin, ConcertAdmin, @@ -168,7 +169,7 @@ class ChangeListTests(TestCase): link = reverse('admin:admin_changelist_child_change', args=(new_child.id,)) row_html = ( 'name' - '†-empty-' % link + '†-empty-' % link ) self.assertNotEqual(table_output.find(row_html), -1, 'Failed to find expected row element: %s' % table_output) @@ -253,6 +254,38 @@ class ChangeListTests(TestCase): m.list_filter, m.date_hierarchy, m.search_fields, m.list_select_related, m.list_per_page, m.list_max_show_all, m.list_editable, m)) + @ignore_warnings(category=RemovedInDjango20Warning) + def test_result_list_with_allow_tags(self): + """ + Test for deprecation of allow_tags attribute + """ + new_parent = Parent.objects.create(name='parent') + for i in range(2): + Child.objects.create(name='name %s' % i, parent=new_parent) + request = self.factory.get('/child/') + m = ChildAdmin(Child, custom_site) + + def custom_method(self, obj=None): + return 'Unsafe html
' + custom_method.allow_tags = True + + # Add custom method with allow_tags attribute + m.custom_method = custom_method + m.list_display = ['id', 'name', 'parent', 'custom_method'] + + cl = ChangeList( + request, Child, m.list_display, m.list_display_links, + m.list_filter, m.date_hierarchy, m.search_fields, + m.list_select_related, m.list_per_page, m.list_max_show_all, m.list_editable, m + ) + FormSet = m.get_changelist_formset(request) + cl.formset = FormSet(queryset=cl.result_list) + template = Template('{% load admin_list %}{% spaceless %}{% result_list cl %}{% endspaceless %}') + context = Context({'cl': cl}) + table_output = template.render(context) + custom_field_html = 'Unsafe html
' + self.assertInHTML(custom_field_html, table_output) + def test_custom_paginator(self): new_parent = Parent.objects.create(name='parent') for i in range(200): diff --git a/tests/admin_views/admin.py b/tests/admin_views/admin.py index c3cedf8444..0104181d96 100644 --- a/tests/admin_views/admin.py +++ b/tests/admin_views/admin.py @@ -18,6 +18,7 @@ from django.core.mail import EmailMessage from django.db import models from django.forms.models import BaseModelFormSet from django.http import HttpResponse, StreamingHttpResponse +from django.utils.html import format_html from django.utils.safestring import mark_safe from django.utils.six import StringIO @@ -429,7 +430,8 @@ class PostAdmin(admin.ModelAdmin): list_display = ['title', 'public'] readonly_fields = ( 'posted', 'awesomeness_level', 'coolness', 'value', - 'multiline', 'multiline_html', lambda obj: "foo" + 'multiline', 'multiline_html', lambda obj: "foo", + 'multiline_html_allow_tags', ) inlines = [ @@ -444,15 +446,17 @@ class PostAdmin(admin.ModelAdmin): def value(self, instance): return 1000 + value.short_description = 'Value in $US' def multiline(self, instance): return "Multiline\ntest\nstring" def multiline_html(self, instance): return mark_safe("Multiline
\nhtml
\ncontent") - multiline_html.allow_tags = True - value.short_description = 'Value in $US' + def multiline_html_allow_tags(self, instance): + return "Multiline
html
content
with allow tags" + multiline_html_allow_tags.allow_tags = True class FieldOverridePostForm(forms.ModelForm): @@ -574,8 +578,7 @@ class ComplexSortedPersonAdmin(admin.ModelAdmin): ordering = ('name',) def colored_name(self, obj): - return '%s' % ('ff00ff', obj.name) - colored_name.allow_tags = True + return format_html('{}', obj.name) colored_name.admin_order_field = 'name' diff --git a/tests/admin_views/tests.py b/tests/admin_views/tests.py index 986add8479..942f033b6f 100644 --- a/tests/admin_views/tests.py +++ b/tests/admin_views/tests.py @@ -27,13 +27,14 @@ from django.forms.utils import ErrorList from django.template.loader import render_to_string from django.template.response import TemplateResponse from django.test import ( - SimpleTestCase, TestCase, modify_settings, override_settings, - skipUnlessDBFeature, + SimpleTestCase, TestCase, ignore_warnings, modify_settings, + override_settings, skipUnlessDBFeature, ) from django.test.utils import override_script_prefix, patch_logger from django.utils import formats, six, translation from django.utils._os import upath from django.utils.cache import get_max_age +from django.utils.deprecation import RemovedInDjango20Warning from django.utils.encoding import force_bytes, force_text, iri_to_uri from django.utils.html import escape from django.utils.http import urlencode @@ -4681,6 +4682,7 @@ class ReadonlyTest(TestCase): def setUp(self): self.client.login(username='super', password='secret') + @ignore_warnings(category=RemovedInDjango20Warning) # for allow_tags deprecation def test_readonly_get(self): response = self.client.get(reverse('admin:admin_views_post_add')) self.assertEqual(response.status_code, 200) @@ -4699,6 +4701,8 @@ class ReadonlyTest(TestCase): self.assertContains(response, "Multiline
test
string") self.assertContains(response, "

Multiline
html
content

", html=True) self.assertContains(response, "InlineMultiline
test
string") + # Remove only this last line when the deprecation completes. + self.assertContains(response, "

Multiline
html
content
with allow tags

", html=True) self.assertContains(response, formats.localize(datetime.date.today() - datetime.timedelta(days=7))) @@ -4768,6 +4772,7 @@ class ReadonlyTest(TestCase): response = self.client.get(reverse('admin:admin_views_topping_add')) self.assertEqual(response.status_code, 200) + @ignore_warnings(category=RemovedInDjango20Warning) # for allow_tags deprecation def test_readonly_field_overrides(self): """ Regression test for #22087 - ModelForm Meta overrides are ignored by @@ -5181,6 +5186,7 @@ class CSSTest(TestCase): def setUp(self): self.client.login(username='super', password='secret') + @ignore_warnings(category=RemovedInDjango20Warning) # for allow_tags deprecation def test_field_prefix_css_classes(self): """ Ensure that fields have a CSS class name with a 'field-' prefix.