Skip to content

Commit

Permalink
Fix http status of the response
Browse files Browse the repository at this point in the history
The http status of the response is changed from 301 (Moved Permanently)
to 302 (Found) for GET requests and to 307 (Temporary Redirect) for
other request methods because nothing prevents the URL to be reused in
the future.

This was inspired by plone/plone.rest#76

Refs #8
  • Loading branch information
ale-rt committed Feb 14, 2020
1 parent 8466ac5 commit 717ca99
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 10 deletions.
1 change: 1 addition & 0 deletions news/8.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
The http status of the response is changed from 301 (Moved Permanently) to 302 (Found) for GET requests and to 307 (Temporary Redirect) for other request methods because nothing prevents the URL to be reused in the future. [ale-rt]
12 changes: 11 additions & 1 deletion plone/app/redirector/browser.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,17 @@ def attempt_redirect(self):
# some analytics programs might use this info to track
if query_string:
url += "?" + query_string
self.request.response.redirect(url, status=301, lock=1)

# Answer GET requests with 302 (Found). Every other method will be answered
# with 307 (Temporary Redirect), which instructs the client to NOT
# switch the method (if the original request was a POST, it should
# re-POST to the new URL from the Location header).
if self.request.method.upper() == "GET":
status = 302
else:
status = 307

self.request.response.redirect(url, status=status, lock=1)
return True

def find_redirect_if_view(self, old_path_elements, storage):
Expand Down
18 changes: 9 additions & 9 deletions plone/app/redirector/tests/test_view.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ def test_attempt_redirect_with_known_url(self):
self.storage.add(fp + '/foo', fp + '/bar')
view = self.view(self.portal, fu + '/foo')
self.assertEqual(True, view.attempt_redirect())
self.assertEqual(301, self.request.response.getStatus())
self.assertEqual(302, self.request.response.getStatus())
self.assertEqual(fu + '/bar',
self.request.response.getHeader('location'))

Expand All @@ -48,7 +48,7 @@ def test_attempt_redirect_with_known_url_and_template(self):
self.storage.add(fp + '/foo', fp + '/bar')
view = self.view(self.portal, fu + '/foo/view')
self.assertEqual(True, view.attempt_redirect())
self.assertEqual(301, self.request.response.getStatus())
self.assertEqual(302, self.request.response.getStatus())
self.assertEqual(fu + '/bar/view',
self.request.response.getHeader('location'))

Expand All @@ -58,29 +58,29 @@ def test_attempt_redirect_with_known_url_and_view_with_part(self):
self.storage.add(fp + '/foo', fp + '/bar')
view = self.view(self.portal, fu + '/foo/@@view/part')
self.assertEqual(True, view.attempt_redirect())
self.assertEqual(301, self.request.response.getStatus())
self.assertEqual(302, self.request.response.getStatus())
self.assertEqual(fu + '/bar/@@view/part',
self.request.response.getHeader('location'))

def test_attempt_redirect_with_unknown_url(self):
fu = self.folder.absolute_url()
view = self.view(self.portal, fu + '/foo')
self.assertEqual(False, view.attempt_redirect())
self.assertNotEqual(301, self.request.response.getStatus())
self.assertNotEqual(302, self.request.response.getStatus())

def test_attempt_redirect_with_unknown_url_with_illegal_characters(self):
fu = self.folder.absolute_url()
view = self.view(self.portal, fu + '+Länder')
self.assertEqual(False, view.attempt_redirect())
self.assertNotEqual(301, self.request.response.getStatus())
self.assertNotEqual(302, self.request.response.getStatus())

def test_attempt_redirect_with_quoted_url(self):
fp = '/'.join(self.folder.getPhysicalPath())
fu = self.folder.absolute_url()
self.storage.add(fp + '/foo', fp + '/bar')
view = self.view(self.portal, fu + '/foo/baz%20quux')
self.assertEqual(True, view.attempt_redirect())
self.assertEqual(301, self.request.response.getStatus())
self.assertEqual(302, self.request.response.getStatus())
self.assertEqual(fu + '/bar/baz%20quux',
self.request.response.getHeader('location'))

Expand All @@ -90,7 +90,7 @@ def test_attempt_redirect_with_query_string(self):
self.storage.add(fp + '/foo?blah=blah', fp + '/bar')
view = self.view(self.portal, fu + '/foo', 'blah=blah')
self.assertEqual(True, view.attempt_redirect())
self.assertEqual(301, self.request.response.getStatus())
self.assertEqual(302, self.request.response.getStatus())
self.assertEqual(fu + '/bar',
self.request.response.getHeader('location'))

Expand All @@ -100,7 +100,7 @@ def test_attempt_redirect_appending_query_string(self):
self.storage.add(fp + '/foo', fp + '/bar')
view = self.view(self.portal, fu + '/foo', 'blah=blah')
self.assertEqual(True, view.attempt_redirect())
self.assertEqual(301, self.request.response.getStatus())
self.assertEqual(302, self.request.response.getStatus())
self.assertEqual(fu + '/bar?blah=blah',
self.request.response.getHeader('location'))

Expand All @@ -111,7 +111,7 @@ def test_attempt_redirect_with_external_url(self):
'http://otherhost' + fp + '/bar%20qux corge')
view = self.view(self.portal, fu + '/foo')
self.assertEqual(True, view.attempt_redirect())
self.assertEqual(301, self.request.response.getStatus())
self.assertEqual(302, self.request.response.getStatus())
self.assertEqual('http://otherhost' + fp + '/bar%20qux%20corge',
self.request.response.getHeader('location'))

Expand Down

0 comments on commit 717ca99

Please sign in to comment.