From 8fd54fd71c0cdc724c4c1772bc9c544adf1ac4a5 Mon Sep 17 00:00:00 2001 From: Hillary Jeffrey Date: Wed, 17 Oct 2018 12:50:48 -0400 Subject: [PATCH] Catch redirects that omit a destination link or point to a page with no routable URL (#4836) Fixes #4815 --- CHANGELOG.txt | 1 + CONTRIBUTORS.rst | 1 + docs/releases/2.4.rst | 1 + wagtail/contrib/redirects/middleware.py | 3 +++ wagtail/contrib/redirects/models.py | 4 +++- wagtail/contrib/redirects/tests.py | 15 +++++++++++++++ wagtail/tests/testapp/fixtures/test.json | 19 ++++++++++++++++++- 7 files changed, 42 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.txt b/CHANGELOG.txt index 3f652a1d8f..1bffe1ef15 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -17,6 +17,7 @@ Changelog * Fix: Additional fields on custom document models now show on the multiple document upload view (Robert Rollins, Sergey Fedoseev) * Fix: Help text is partially hidden when using a combination of BooleanField and FieldPanel in page model (Dzianis Sheka) * Fix: Allow custom logos of any height in the admin menu (Meteor0id) + * Fix: Redirects now return 404 when destination is unspecified or a page with no site (Hillary Jeffrey) 2.3 LTS (23.10.2018) diff --git a/CONTRIBUTORS.rst b/CONTRIBUTORS.rst index 279c1c9cf9..748deb51c9 100644 --- a/CONTRIBUTORS.rst +++ b/CONTRIBUTORS.rst @@ -333,6 +333,7 @@ Contributors * Fedor Selitsky * Seb Brown * Noah B Johnson +* Hillary Jeffrey Translators =========== diff --git a/docs/releases/2.4.rst b/docs/releases/2.4.rst index b7d2db8f42..a57437e34f 100644 --- a/docs/releases/2.4.rst +++ b/docs/releases/2.4.rst @@ -41,6 +41,7 @@ Bug fixes * Document chooser now displays more useful help message when there are no documents in Wagtail document library (gmmoraes, Stas Rudakou) * Allow custom logos of any height in the admin menu (Meteor0id) * Users without the edit permission no longer see "Edit" links in list of pages waiting for moderation (Justin Focus, Fedor Selitsky) + * Redirects now return 404 when destination is unspecified or a page with no site (Hillary Jeffrey) Upgrade considerations diff --git a/wagtail/contrib/redirects/middleware.py b/wagtail/contrib/redirects/middleware.py index 164e82dc21..c2eaba23a2 100644 --- a/wagtail/contrib/redirects/middleware.py +++ b/wagtail/contrib/redirects/middleware.py @@ -57,6 +57,9 @@ class RedirectMiddleware(MiddlewareMixin): if redirect is None: return response + if redirect.link is None: + return response + if redirect.is_permanent: return http.HttpResponsePermanentRedirect(redirect.link) else: diff --git a/wagtail/contrib/redirects/models.py b/wagtail/contrib/redirects/models.py index 6897d144d3..e29cf9569b 100644 --- a/wagtail/contrib/redirects/models.py +++ b/wagtail/contrib/redirects/models.py @@ -34,9 +34,11 @@ class Redirect(models.Model): def link(self): if self.redirect_page: return self.redirect_page.url - else: + elif self.redirect_link: return self.redirect_link + return None + def get_is_permanent_display(self): if self.is_permanent: return _("permanent") diff --git a/wagtail/contrib/redirects/tests.py b/wagtail/contrib/redirects/tests.py index 8547136402..3116a66346 100644 --- a/wagtail/contrib/redirects/tests.py +++ b/wagtail/contrib/redirects/tests.py @@ -176,6 +176,21 @@ class TestRedirects(TestCase): response = self.client.get('/xmas/', HTTP_HOST='localhost') self.assertEqual(response.status_code, 404) + def test_redirect_without_page_or_link_target(self): + models.Redirect.objects.create(old_path='/xmas/', redirect_link='') + + # the redirect has been created but has no target and should 404 + response = self.client.get('/xmas/', HTTP_HOST='localhost') + self.assertEqual(response.status_code, 404) + + def test_redirect_to_page_without_site(self): + siteless_page = Page.objects.get(url_path='/does-not-exist/') + models.Redirect.objects.create(old_path='/xmas', redirect_page=siteless_page) + + # the redirect's destination page doesn't have a site so the redirect should 404 + response = self.client.get('/xmas/', HTTP_HOST='localhost') + self.assertEqual(response.status_code, 404) + def test_duplicate_redirects_when_match_is_for_generic(self): contact_page = Page.objects.get(url_path='/home/contact-us/') site = Site.objects.create(hostname='other.example.com', port=80, root_page=contact_page) diff --git a/wagtail/tests/testapp/fixtures/test.json b/wagtail/tests/testapp/fixtures/test.json index e5420b56fc..468b57412b 100644 --- a/wagtail/tests/testapp/fixtures/test.json +++ b/wagtail/tests/testapp/fixtures/test.json @@ -5,7 +5,7 @@ "fields": { "title": "Root", "draft_title": "Root", - "numchild": 1, + "numchild": 2, "show_in_menus": false, "live": true, "depth": 1, @@ -594,6 +594,23 @@ } }, +{ + "pk": 20, + "model": "wagtailcore.page", + "fields": { + "title": "This page doesn't get served", + "draft_title": "This page doesn't get served", + "numchild": 0, + "show_in_menus": false, + "live": true, + "depth": 2, + "content_type": ["wagtailcore", "page"], + "path": "00010002", + "url_path": "/does-not-exist/", + "slug": "does-not-exist" + } +}, + { "pk": 1,