From 845817b039fc059955bb1eafa5fd78565a49159d Mon Sep 17 00:00:00 2001 From: Tim Graham Date: Fri, 6 Mar 2015 12:45:53 -0500 Subject: [PATCH] Fixed #24466 -- Added JavaScript escaping in a couple places in the admin. Thanks Aymeric Augustin and Florian Apolloner for work on the patch. --- django/contrib/admin/options.py | 5 ++-- .../templates/admin/edit_inline/stacked.html | 8 +++--- .../templates/admin/edit_inline/tabular.html | 8 +++--- .../admin/templates/admin/popup_response.html | 6 ++--- django/contrib/admin/widgets.py | 4 +-- tests/admin_inlines/test_templates.py | 21 ++++++++++++++++ tests/admin_inlines/tests.py | 12 ++++----- tests/admin_views/tests.py | 25 +++++++++++++++++++ tests/admin_widgets/tests.py | 14 ++++++++--- 9 files changed, 77 insertions(+), 26 deletions(-) create mode 100644 tests/admin_inlines/test_templates.py diff --git a/django/contrib/admin/options.py b/django/contrib/admin/options.py index b85f0fdeac4..50e7227cd04 100644 --- a/django/contrib/admin/options.py +++ b/django/contrib/admin/options.py @@ -1051,9 +1051,8 @@ class ModelAdmin(BaseModelAdmin): attr = obj._meta.pk.attname value = obj.serializable_value(attr) return SimpleTemplateResponse('admin/popup_response.html', { - 'pk_value': escape(pk_value), # for possible backwards-compatibility - 'value': escape(value), - 'obj': escapejs(obj) + 'value': value, + 'obj': obj, }) elif "_continue" in request.POST: diff --git a/django/contrib/admin/templates/admin/edit_inline/stacked.html b/django/contrib/admin/templates/admin/edit_inline/stacked.html index 5b5c22d34ef..2a837a16864 100644 --- a/django/contrib/admin/templates/admin/edit_inline/stacked.html +++ b/django/contrib/admin/templates/admin/edit_inline/stacked.html @@ -21,10 +21,10 @@ diff --git a/django/contrib/admin/templates/admin/edit_inline/tabular.html b/django/contrib/admin/templates/admin/edit_inline/tabular.html index 379595a0444..a890cc334d3 100644 --- a/django/contrib/admin/templates/admin/edit_inline/tabular.html +++ b/django/contrib/admin/templates/admin/edit_inline/tabular.html @@ -74,10 +74,10 @@ diff --git a/django/contrib/admin/templates/admin/popup_response.html b/django/contrib/admin/templates/admin/popup_response.html index 3f8de111026..626ed98edad 100644 --- a/django/contrib/admin/templates/admin/popup_response.html +++ b/django/contrib/admin/templates/admin/popup_response.html @@ -4,11 +4,11 @@ diff --git a/django/contrib/admin/widgets.py b/django/contrib/admin/widgets.py index fd0021c0571..491c78870f6 100644 --- a/django/contrib/admin/widgets.py +++ b/django/contrib/admin/widgets.py @@ -15,7 +15,7 @@ from django.template.loader import render_to_string from django.utils import six from django.utils.encoding import force_text from django.utils.html import ( - escape, format_html, format_html_join, smart_urlquote, + escape, escapejs, format_html, format_html_join, smart_urlquote, ) from django.utils.safestring import mark_safe from django.utils.text import Truncator @@ -50,7 +50,7 @@ class FilteredSelectMultiple(forms.SelectMultiple): # TODO: "id_" is hard-coded here. This should instead use the correct # API to determine the ID dynamically. output.append('SelectFilter.init("id_%s", "%s", %s); });\n' - % (name, self.verbose_name.replace('"', '\\"'), int(self.is_stacked))) + % (name, escapejs(self.verbose_name), int(self.is_stacked))) return mark_safe(''.join(output)) diff --git a/tests/admin_inlines/test_templates.py b/tests/admin_inlines/test_templates.py new file mode 100644 index 00000000000..1ed52c91157 --- /dev/null +++ b/tests/admin_inlines/test_templates.py @@ -0,0 +1,21 @@ +from __future__ import unicode_literals + +from django.template.loader import render_to_string +from django.test import SimpleTestCase + + +class TestTemplates(SimpleTestCase): + def test_javascript_escaping(self): + context = { + 'inline_admin_formset': { + 'formset': {'prefix': 'my-prefix'}, + 'opts': {'verbose_name': 'verbose name\\'}, + }, + } + output = render_to_string('admin/edit_inline/stacked.html', context) + self.assertIn('prefix: "my\\u002Dprefix",', output) + self.assertIn('addText: "Add another Verbose name\\u005C"', output) + + output = render_to_string('admin/edit_inline/tabular.html', context) + self.assertIn('prefix: "my\\u002Dprefix",', output) + self.assertIn('addText: "Add another Verbose name\\u005C"', output) diff --git a/tests/admin_inlines/tests.py b/tests/admin_inlines/tests.py index c262de7d272..19260baf4da 100644 --- a/tests/admin_inlines/tests.py +++ b/tests/admin_inlines/tests.py @@ -78,7 +78,7 @@ class TestInline(TestDataMixin, TestCase): # The heading for the m2m inline block uses the right text self.assertContains(response, '

Author-book relationships

') # The "add another" label is correct - self.assertContains(response, 'Add another Author-book relationship') + self.assertContains(response, 'Add another Author\\u002Dbook relationship') # The '+' is dropped from the autogenerated form prefix (Author_books+) self.assertContains(response, 'id="id_Author_books-TOTAL_FORMS"') @@ -524,7 +524,7 @@ class TestInlinePermissions(TestCase): response = self.client.get(reverse('admin:admin_inlines_author_add')) # No change permission on books, so no inline self.assertNotContains(response, '

Author-book relationships

') - self.assertNotContains(response, 'Add another Author-Book Relationship') + self.assertNotContains(response, 'Add another Author\\u002DBook Relationship') self.assertNotContains(response, 'id="id_Author_books-TOTAL_FORMS"') def test_inline_add_fk_noperm(self): @@ -538,7 +538,7 @@ class TestInlinePermissions(TestCase): response = self.client.get(self.author_change_url) # No change permission on books, so no inline self.assertNotContains(response, '

Author-book relationships

') - self.assertNotContains(response, 'Add another Author-Book Relationship') + self.assertNotContains(response, 'Add another Author\\u002DBook Relationship') self.assertNotContains(response, 'id="id_Author_books-TOTAL_FORMS"') def test_inline_change_fk_noperm(self): @@ -554,7 +554,7 @@ class TestInlinePermissions(TestCase): response = self.client.get(reverse('admin:admin_inlines_author_add')) # No change permission on Books, so no inline self.assertNotContains(response, '

Author-book relationships

') - self.assertNotContains(response, 'Add another Author-Book Relationship') + self.assertNotContains(response, 'Add another Author\\u002DBook Relationship') self.assertNotContains(response, 'id="id_Author_books-TOTAL_FORMS"') def test_inline_add_fk_add_perm(self): @@ -573,7 +573,7 @@ class TestInlinePermissions(TestCase): response = self.client.get(self.author_change_url) # No change permission on books, so no inline self.assertNotContains(response, '

Author-book relationships

') - self.assertNotContains(response, 'Add another Author-Book Relationship') + self.assertNotContains(response, 'Add another Author\\u002DBook Relationship') self.assertNotContains(response, 'id="id_Author_books-TOTAL_FORMS"') self.assertNotContains(response, 'id="id_Author_books-0-DELETE"') @@ -583,7 +583,7 @@ class TestInlinePermissions(TestCase): response = self.client.get(self.author_change_url) # We have change perm on books, so we can add/change/delete inlines self.assertContains(response, '

Author-book relationships

') - self.assertContains(response, 'Add another Author-book relationship') + self.assertContains(response, 'Add another Author\\u002Dbook relationship') self.assertContains(response, '', html=True) self.assertContains(response, ' self.assertEqual(response.status_code, 200) self.assertEqual(response.template_name, 'admin/popup_response.html') + def test_popup_template_escaping(self): + context = { + 'new_value': 'new_value\\', + 'obj': 'obj\\', + 'value': 'value\\', + } + output = render_to_string('admin/popup_response.html', context) + self.assertIn( + 'opener.dismissAddRelatedObjectPopup(window, "value\\u005C", "obj\\u005C");', output + ) + + context['action'] = 'change' + output = render_to_string('admin/popup_response.html', context) + self.assertIn( + 'opener.dismissChangeRelatedObjectPopup(window, ' + '"value\\u005C", "obj\\u005C", "new_value\\u005C");', output + ) + + context['action'] = 'delete' + output = render_to_string('admin/popup_response.html', context) + self.assertIn( + 'opener.dismissDeleteRelatedObjectPopup(window, "value\\u005C");', output + ) + @override_settings(PASSWORD_HASHERS=['django.contrib.auth.hashers.SHA1PasswordHasher'], ROOT_URLCONF="admin_views.urls") diff --git a/tests/admin_widgets/tests.py b/tests/admin_widgets/tests.py index 5083295c467..1f0ec00eedc 100644 --- a/tests/admin_widgets/tests.py +++ b/tests/admin_widgets/tests.py @@ -264,17 +264,23 @@ class AdminForeignKeyRawIdWidget(TestDataMixin, DjangoTestCase): class FilteredSelectMultipleWidgetTest(DjangoTestCase): def test_render(self): - w = widgets.FilteredSelectMultiple('test', False) + # Backslash in verbose_name to ensure it is JavaScript escaped. + w = widgets.FilteredSelectMultiple('test\\', False) self.assertHTMLEqual( w.render('test', 'test'), - '\n' + '' + '\n' ) def test_stacked_render(self): - w = widgets.FilteredSelectMultiple('test', True) + # Backslash in verbose_name to ensure it is JavaScript escaped. + w = widgets.FilteredSelectMultiple('test\\', True) self.assertHTMLEqual( w.render('test', 'test'), - '\n' + '' + '\n' )