From 418af66f014d118c264091a32f5501f9be63692e Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Thu, 4 Jan 2018 12:44:57 +0000 Subject: [PATCH] Ensure that snippet PKs are quoted in calls to reverse --- wagtail/snippets/models.py | 3 +- wagtail/snippets/tests.py | 46 ++++++++++++------------ wagtail/snippets/views/chooser.py | 4 +-- wagtail/snippets/views/snippets.py | 2 +- wagtail/tests/testapp/fixtures/test.json | 2 +- 5 files changed, 29 insertions(+), 28 deletions(-) diff --git a/wagtail/snippets/models.py b/wagtail/snippets/models.py index 81a13b5ca2..ca9f447bf7 100644 --- a/wagtail/snippets/models.py +++ b/wagtail/snippets/models.py @@ -1,3 +1,4 @@ +from django.contrib.admin.utils import quote from django.urls import reverse from wagtail.admin.utils import get_object_usage @@ -20,4 +21,4 @@ def register_snippet(model): def get_snippet_usage_url(self): return reverse('wagtailsnippets:usage', args=( - self._meta.app_label, self._meta.model_name, self.pk)) + self._meta.app_label, self._meta.model_name, quote(self.pk))) diff --git a/wagtail/snippets/tests.py b/wagtail/snippets/tests.py index 25ca9f10fb..11ddab9e24 100644 --- a/wagtail/snippets/tests.py +++ b/wagtail/snippets/tests.py @@ -226,12 +226,12 @@ class BaseTestSnippetEditView(TestCase, WagtailTestUtils): def get(self, params={}): snippet = self.test_snippet - args = (snippet._meta.app_label, snippet._meta.model_name, snippet.pk) + args = (snippet._meta.app_label, snippet._meta.model_name, quote(snippet.pk)) return self.client.get(reverse('wagtailsnippets:edit', args=args), params) def post(self, post_data={}): snippet = self.test_snippet - args = (snippet._meta.app_label, snippet._meta.model_name, snippet.pk) + args = (snippet._meta.app_label, snippet._meta.model_name, quote(snippet.pk)) return self.client.post(reverse('wagtailsnippets:edit', args=args), post_data) def setUp(self): @@ -254,7 +254,7 @@ class TestSnippetEditView(BaseTestSnippetEditView): self.assertNotContains(response, 'Other', html=True) def test_non_existant_model(self): - response = self.client.get(reverse('wagtailsnippets:edit', args=('tests', 'foo', self.test_snippet.pk))) + response = self.client.get(reverse('wagtailsnippets:edit', args=('tests', 'foo', quote(self.test_snippet.pk)))) self.assertEqual(response.status_code, 404) def test_nonexistant_id(self): @@ -339,12 +339,12 @@ class TestSnippetDelete(TestCase, WagtailTestUtils): self.login() def test_delete_get(self): - response = self.client.get(reverse('wagtailsnippets:delete', args=('tests', 'advert', self.test_snippet.pk, ))) + response = self.client.get(reverse('wagtailsnippets:delete', args=('tests', 'advert', quote(self.test_snippet.pk), ))) self.assertEqual(response.status_code, 200) def test_delete_post(self): response = self.client.post( - reverse('wagtailsnippets:delete', args=('tests', 'advert', self.test_snippet.pk, )) + reverse('wagtailsnippets:delete', args=('tests', 'advert', quote(self.test_snippet.pk), )) ) # Should be redirected to explorer page @@ -355,7 +355,7 @@ class TestSnippetDelete(TestCase, WagtailTestUtils): @override_settings(WAGTAIL_USAGE_COUNT_ENABLED=True) def test_usage_link(self): - response = self.client.get(reverse('wagtailsnippets:delete', args=('tests', 'advert', self.test_snippet.pk, ))) + response = self.client.get(reverse('wagtailsnippets:delete', args=('tests', 'advert', quote(self.test_snippet.pk), ))) self.assertEqual(response.status_code, 200) self.assertTemplateUsed(response, 'wagtailsnippets/snippets/confirm_delete.html') self.assertIn('Used 2 times', str(response.content)) @@ -594,12 +594,12 @@ class TestAddOnlyPermissions(TestCase, WagtailTestUtils): def test_get_edit(self): response = self.client.get(reverse('wagtailsnippets:edit', - args=('tests', 'advert', self.test_snippet.pk))) + args=('tests', 'advert', quote(self.test_snippet.pk)))) # permission should be denied self.assertRedirects(response, reverse('wagtailadmin_home')) def test_get_delete(self): - response = self.client.get(reverse('wagtailsnippets:delete', args=('tests', 'advert', self.test_snippet.pk, ))) + response = self.client.get(reverse('wagtailsnippets:delete', args=('tests', 'advert', quote(self.test_snippet.pk), ))) # permission should be denied self.assertRedirects(response, reverse('wagtailadmin_home')) @@ -638,12 +638,12 @@ class TestEditOnlyPermissions(TestCase, WagtailTestUtils): def test_get_edit(self): response = self.client.get(reverse('wagtailsnippets:edit', - args=('tests', 'advert', self.test_snippet.pk))) + args=('tests', 'advert', quote(self.test_snippet.pk)))) self.assertEqual(response.status_code, 200) self.assertTemplateUsed(response, 'wagtailsnippets/snippets/edit.html') def test_get_delete(self): - response = self.client.get(reverse('wagtailsnippets:delete', args=('tests', 'advert', self.test_snippet.pk, ))) + response = self.client.get(reverse('wagtailsnippets:delete', args=('tests', 'advert', quote(self.test_snippet.pk), ))) # permission should be denied self.assertRedirects(response, reverse('wagtailadmin_home')) @@ -682,12 +682,12 @@ class TestDeleteOnlyPermissions(TestCase, WagtailTestUtils): def test_get_edit(self): response = self.client.get(reverse('wagtailsnippets:edit', - args=('tests', 'advert', self.test_snippet.pk))) + args=('tests', 'advert', quote(self.test_snippet.pk)))) # permission should be denied self.assertRedirects(response, reverse('wagtailadmin_home')) def test_get_delete(self): - response = self.client.get(reverse('wagtailsnippets:delete', args=('tests', 'advert', self.test_snippet.pk, ))) + response = self.client.get(reverse('wagtailsnippets:delete', args=('tests', 'advert', quote(self.test_snippet.pk), ))) self.assertEqual(response.status_code, 200) self.assertTemplateUsed(response, 'wagtailsnippets/snippets/confirm_delete.html') @@ -787,9 +787,9 @@ class TestSnippetListViewWithCustomPrimaryKey(TestCase, WagtailTestUtils): self.login() # Create some instances of the searchable snippet for testing - self.snippet_a = StandardSnippetWithCustomPrimaryKey.objects.create(snippet_id="snippet_01", text="Hello") - self.snippet_b = StandardSnippetWithCustomPrimaryKey.objects.create(snippet_id="snippet_02", text="Hello") - self.snippet_c = StandardSnippetWithCustomPrimaryKey.objects.create(snippet_id="snippet_03", text="Hello") + self.snippet_a = StandardSnippetWithCustomPrimaryKey.objects.create(snippet_id="snippet/01", text="Hello") + self.snippet_b = StandardSnippetWithCustomPrimaryKey.objects.create(snippet_id="snippet/02", text="Hello") + self.snippet_c = StandardSnippetWithCustomPrimaryKey.objects.create(snippet_id="snippet/03", text="Hello") def get(self, params={}): return self.client.get(reverse('wagtailsnippets:list', @@ -814,7 +814,7 @@ class TestSnippetViewWithCustomPrimaryKey(TestCase, WagtailTestUtils): def setUp(self): super(TestSnippetViewWithCustomPrimaryKey, self).setUp() self.login() - self.snippet_a = StandardSnippetWithCustomPrimaryKey.objects.create(snippet_id="snippet_01", text="Hello") + self.snippet_a = StandardSnippetWithCustomPrimaryKey.objects.create(snippet_id="snippet/01", text="Hello") def get(self, snippet, params={}): args = (snippet._meta.app_label, snippet._meta.model_name, quote(snippet.pk)) @@ -849,7 +849,7 @@ class TestSnippetViewWithCustomPrimaryKey(TestCase, WagtailTestUtils): def test_create(self): response = self.create(self.snippet_a, post_data={'text': 'test snippet', - 'snippet_id': 'snippet_02'}) + 'snippet_id': 'snippet/02'}) self.assertRedirects(response, reverse('wagtailsnippets:list', args=('snippetstests', 'standardsnippetwithcustomprimarykey'))) snippets = StandardSnippetWithCustomPrimaryKey.objects.all() @@ -875,7 +875,7 @@ class TestSnippetChooserBlockWithCustomPrimaryKey(TestCase): def test_serialize(self): """The value of a SnippetChooserBlock (a snippet instance) should serialize to an ID""" block = SnippetChooserBlock(AdvertWithCustomPrimaryKey) - test_advert = AdvertWithCustomPrimaryKey.objects.get(pk='advert_01') + test_advert = AdvertWithCustomPrimaryKey.objects.get(pk='advert/01') self.assertEqual(block.get_prep_value(test_advert), test_advert.pk) @@ -885,7 +885,7 @@ class TestSnippetChooserBlockWithCustomPrimaryKey(TestCase): def test_deserialize(self): """The serialized value of a SnippetChooserBlock (an ID) should deserialize to a snippet instance""" block = SnippetChooserBlock(AdvertWithCustomPrimaryKey) - test_advert = AdvertWithCustomPrimaryKey.objects.get(pk='advert_01') + test_advert = AdvertWithCustomPrimaryKey.objects.get(pk='advert/01') self.assertEqual(block.to_python(test_advert.pk), test_advert) @@ -899,7 +899,7 @@ class TestSnippetChooserBlockWithCustomPrimaryKey(TestCase): self.assertInHTML('', empty_form_html) self.assertIn('createSnippetChooser("advertwithcustomprimarykey", "tests/advertwithcustomprimarykey");', empty_form_html) - test_advert = AdvertWithCustomPrimaryKey.objects.get(pk='advert_01') + test_advert = AdvertWithCustomPrimaryKey.objects.get(pk='advert/01') test_advert_form_html = block.render_form(test_advert, 'advertwithcustomprimarykey') expected_html = '' % test_advert.pk self.assertInHTML(expected_html, test_advert_form_html) @@ -907,7 +907,7 @@ class TestSnippetChooserBlockWithCustomPrimaryKey(TestCase): def test_form_response(self): block = SnippetChooserBlock(AdvertWithCustomPrimaryKey) - test_advert = AdvertWithCustomPrimaryKey.objects.get(pk='advert_01') + test_advert = AdvertWithCustomPrimaryKey.objects.get(pk='advert/01') value = block.value_from_datadict({'advertwithcustomprimarykey': str(test_advert.pk)}, {}, 'advertwithcustomprimarykey') self.assertEqual(value, test_advert) @@ -918,7 +918,7 @@ class TestSnippetChooserBlockWithCustomPrimaryKey(TestCase): def test_clean(self): required_block = SnippetChooserBlock(AdvertWithCustomPrimaryKey) nonrequired_block = SnippetChooserBlock(AdvertWithCustomPrimaryKey, required=False) - test_advert = AdvertWithCustomPrimaryKey.objects.get(pk='advert_01') + test_advert = AdvertWithCustomPrimaryKey.objects.get(pk='advert/01') self.assertEqual(required_block.clean(test_advert), test_advert) with self.assertRaises(ValidationError): @@ -936,7 +936,7 @@ class TestSnippetChooserPanelWithCustomPrimaryKey(TestCase, WagtailTestUtils): self.advert_text = 'Test advert text' test_snippet = model.objects.create( advertwithcustomprimarykey=AdvertWithCustomPrimaryKey.objects.create( - advert_id="advert_02", + advert_id="advert/02", text=self.advert_text ) ) diff --git a/wagtail/snippets/views/chooser.py b/wagtail/snippets/views/chooser.py index a8b9854deb..ed900490a7 100644 --- a/wagtail/snippets/views/chooser.py +++ b/wagtail/snippets/views/chooser.py @@ -1,6 +1,6 @@ import json -from django.contrib.admin.utils import unquote +from django.contrib.admin.utils import quote, unquote from django.shortcuts import get_object_or_404, render from django.urls import reverse from django.utils.translation import ugettext as _ @@ -78,7 +78,7 @@ def chosen(request, app_label, model_name, pk): 'id': item.pk, 'string': str(item), 'edit_link': reverse('wagtailsnippets:edit', args=( - app_label, model_name, item.pk)) + app_label, model_name, quote(item.pk))) }) return render_modal_workflow( diff --git a/wagtail/snippets/views/snippets.py b/wagtail/snippets/views/snippets.py index da31552719..da4f929c59 100644 --- a/wagtail/snippets/views/snippets.py +++ b/wagtail/snippets/views/snippets.py @@ -146,7 +146,7 @@ def create(request, app_label, model_name): ), buttons=[ messages.button(reverse( - 'wagtailsnippets:edit', args=(app_label, model_name, instance.pk) + 'wagtailsnippets:edit', args=(app_label, model_name, quote(instance.pk)) ), _('Edit')) ] ) diff --git a/wagtail/tests/testapp/fixtures/test.json b/wagtail/tests/testapp/fixtures/test.json index f9e432e304..0ec599bd68 100644 --- a/wagtail/tests/testapp/fixtures/test.json +++ b/wagtail/tests/testapp/fixtures/test.json @@ -1004,7 +1004,7 @@ } }, { - "pk": "advert_01", + "pk": "advert/01", "model": "tests.advertwithcustomprimarykey", "fields": { "text": "test_advert",