Skip to content

Commit

Permalink
Restore HTTPResponse.redirect behaviour of not raising an exception. (
Browse files Browse the repository at this point in the history
#114)

This should fix testbrowser related failures that suddenly saw
Redirect exceptions being raised, rather than the redirects just
being followed. This refs #106.
  • Loading branch information
hannosch committed May 5, 2017
1 parent 50ce4db commit 23f094f
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 18 deletions.
2 changes: 2 additions & 0 deletions CHANGES.rst
Expand Up @@ -11,6 +11,8 @@ https://zope.readthedocs.io/en/2.13/CHANGES.html
Bugs Fixed
++++++++++

- Restore `HTTPResponse.redirect` behaviour of not raising an exception.

Features Added
++++++++++++++

Expand Down
36 changes: 36 additions & 0 deletions src/Testing/tests/test_testbrowser.py
Expand Up @@ -42,6 +42,13 @@ def __call__(self, REQUEST):
raise ValueError('dummy')


class RedirectStub(Item):
"""This is a stub, causing a redirect."""

def __call__(self, REQUEST):
return REQUEST.RESPONSE.redirect('/redirected')


class TestTestbrowser(FunctionalTestCase):

def test_auth(self):
Expand Down Expand Up @@ -86,6 +93,7 @@ def test_cookies(self):
def test_handle_errors_true(self):
self.folder._setObject('stub', ExceptionStub())
browser = Browser()

with self.assertRaises(HTTPError):
browser.open('http://localhost/test_folder_1_/stub')
self.assertTrue(browser.headers['status'].startswith('500'))
Expand All @@ -94,10 +102,20 @@ def test_handle_errors_true(self):
browser.open('http://localhost/nothing-is-here')
self.assertTrue(browser.headers['status'].startswith('404'))

def test_handle_errors_true_redirect(self):
self.folder._setObject('redirect', RedirectStub())
browser = Browser()

with self.assertRaises(HTTPError):
browser.open('http://localhost/test_folder_1_/redirect')
self.assertTrue(browser.headers['status'].startswith('404'))
self.assertEqual(browser.url, 'http://localhost/redirected')

def test_handle_errors_false(self):
self.folder._setObject('stub', ExceptionStub())
browser = Browser()
browser.handleErrors = False

with self.assertRaises(ValueError):
browser.open('http://localhost/test_folder_1_/stub')
self.assertTrue(browser.contents is None)
Expand All @@ -106,6 +124,15 @@ def test_handle_errors_false(self):
browser.open('http://localhost/nothing-is-here')
self.assertTrue(browser.contents is None)

def test_handle_errors_false_redirect(self):
self.folder._setObject('redirect', RedirectStub())
browser = Browser()
browser.handleErrors = False

with self.assertRaises(NotFound):
browser.open('http://localhost/test_folder_1_/redirect')
self.assertTrue(browser.contents is None)

def test_raise_http_errors_false(self):
self.folder._setObject('stub', ExceptionStub())
browser = Browser()
Expand All @@ -117,6 +144,15 @@ def test_raise_http_errors_false(self):
browser.open('http://localhost/nothing-is-here')
self.assertTrue(browser.headers['status'].startswith('404'))

def test_raise_http_errors_false_redirect(self):
self.folder._setObject('redirect', RedirectStub())
browser = Browser()
browser.raiseHttpErrors = False

browser.open('http://localhost/test_folder_1_/redirect')
self.assertTrue(browser.headers['status'].startswith('404'))
self.assertEqual(browser.url, 'http://localhost/redirected')

def test_headers_camel_case(self):
# The Zope2 response mungs headers so they come out
# in camel case. We should do the same.
Expand Down
27 changes: 9 additions & 18 deletions src/ZPublisher/HTTPResponse.py
Expand Up @@ -174,6 +174,15 @@ def __init__(self,
self.stdout = stdout
self.stderr = stderr

def redirect(self, location, status=302, lock=0):
"""Cause a redirection without raising an error"""
if isinstance(location, HTTPRedirection):
status = location.getStatus()
location = str(location)
self.setStatus(status, lock=lock)
self.setHeader('Location', location)
return location

def retry(self):
""" Return a cloned response object to be used in a retry attempt.
"""
Expand Down Expand Up @@ -798,15 +807,6 @@ def unauthorized(self):
m = m + '\nNo Authorization header found.'
raise Unauthorized(m)

def redirect(self, location, status=302, lock=0):
"""Cause a redirection without raising an error"""
if isinstance(location, HTTPRedirection):
status = location.getStatus()
location = str(location)
self.setStatus(status, lock=lock)
self.setHeader('Location', location)
return location

def _setBCIHeaders(self, t, tb):
try:
# Try to capture exception info for bci calls
Expand Down Expand Up @@ -1009,15 +1009,6 @@ def unauthorized(self):
exc.detail = 'No Authorization header found.'
raise exc

def redirect(self, location, status=302, lock=0):
"""Cause a redirection."""
if isinstance(location, HTTPRedirection):
raise location

exc = Redirect(str(location))
exc.setStatus(status)
raise exc

def exception(self, fatal=0, info=None, abort=1):
if isinstance(info, tuple) and len(info) == 3:
t, v, tb = info
Expand Down

0 comments on commit 23f094f

Please sign in to comment.