Fixed #25135 -- Deprecated the contrib.admin allow_tags attribute.

Thanks Jaap Roes for the idea and initial patch.

9 files changed, 96 insertions, 30 deletions

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('<input type="checkbox" id="action-toggle" />')
-    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('<br/>'),
                     '{}',
                     ((line,) for line in instance.get_full_address()),
-                ) or "<span class='errors'>I can't determine this address.</span>"
+                ) or mark_safe("<span class='errors'>I can't determine this address.</span>")
 
             # 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 = (
             '<tbody><tr class="row1"><th class="field-name"><a href="%s">name</a></th>'
-            '<td class="field-age_display">&dagger;</td><td class="field-age">-empty-</td></tr></tbody>' % link
+            '<td class="field-age_display">&amp;dagger;</td><td class="field-age">-empty-</td></tr></tbody>' % 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 <br />'
+        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 = '<td class="field-custom_method">Unsafe html <br /></td>'
+        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<br>\nhtml<br>\ncontent")
-    multiline_html.allow_tags = True
 
-    value.short_description = 'Value in $US'
+    def multiline_html_allow_tags(self, instance):
+        return "Multiline<br>html<br>content<br>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 '<span style="color: #%s;">%s</span>' % ('ff00ff', obj.name)
-    colored_name.allow_tags = True
+        return format_html('<span style="color: #ff00ff;">{}</span>', 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<br />test<br />string")
         self.assertContains(response, "<p>Multiline<br />html<br />content</p>", html=True)
         self.assertContains(response, "InlineMultiline<br />test<br />string")
+        # Remove only this last line when the deprecation completes.
+        self.assertContains(response, "<p>Multiline<br />html<br />content<br />with allow tags</p>", 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.