From eaaea9460a505daa4639eb84dd61973c5ab4dd85 Mon Sep 17 00:00:00 2001 From: David Glick Date: Wed, 1 Feb 2017 17:34:26 +0100 Subject: [PATCH] Don't follow redirects for all 30x response status codes. Some 30x codes do not indicate redirects, such as 304. This follows redirects for the same status codes that mechanize did (301, 302, 303, 307) --- CHANGES.rst | 3 ++- src/zope/testbrowser/browser.py | 4 +++- src/zope/testbrowser/ftests/wsgitestapp.py | 5 ++--- src/zope/testbrowser/tests/test_wsgi.py | 24 ++++++++++++++++++++++ 4 files changed, 31 insertions(+), 5 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 3566a0a..5153809 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -5,7 +5,8 @@ CHANGES 5.2 (unreleased) ---------------- -- Nothing changed yet. +- Fixed browser to only follow redirects for HTTP statuses + 301, 302, 303, and 307; not other 30x statuses such as 304. 5.1 (2017-01-31) diff --git a/src/zope/testbrowser/browser.py b/src/zope/testbrowser/browser.py index ea944d2..ec1c323 100644 --- a/src/zope/testbrowser/browser.py +++ b/src/zope/testbrowser/browser.py @@ -55,6 +55,8 @@ def __init__(self, *args): _allowed = set(['localhost', '127.0.0.1']) _allowed.update(_allowed_2nd_level) +REDIRECTS = (301, 302, 303, 307) + class TestbrowserApp(webtest.TestApp): _last_fragment = "" @@ -275,7 +277,7 @@ def _processRequest(self, url, make_request): self._history.add(self._response) resp = make_request(reqargs) remaining_redirects = 100 # infinite loops protection - while 300 <= resp.status_int < 400 and remaining_redirects: + while resp.status_int in REDIRECTS and remaining_redirects: remaining_redirects -= 1 url = urlparse.urljoin(url, resp.headers['location']) with self._preparedRequest(url) as reqargs: diff --git a/src/zope/testbrowser/ftests/wsgitestapp.py b/src/zope/testbrowser/ftests/wsgitestapp.py index 50ef2f4..5162308 100644 --- a/src/zope/testbrowser/ftests/wsgitestapp.py +++ b/src/zope/testbrowser/ftests/wsgitestapp.py @@ -177,8 +177,7 @@ def echo_one(req): def set_status(req): status = req.params.get('status') + body = req.params.get('body', 'Just set a status of %s' % status) if status: - resp = Response('Just set a status of %s' % status) - resp.status = int(status) - return resp + return Response(body, status=int(status)) return Response('Everything fine') diff --git a/src/zope/testbrowser/tests/test_wsgi.py b/src/zope/testbrowser/tests/test_wsgi.py index 27894fd..f310aa1 100644 --- a/src/zope/testbrowser/tests/test_wsgi.py +++ b/src/zope/testbrowser/tests/test_wsgi.py @@ -96,6 +96,30 @@ def test_redirect(self): self.assertEquals(browser.headers.get('status'), '200 OK') self.assertEquals(browser.url, 'http://localhost/set_status.html') + def test_non_redirecting_30x_status(self): + app = WSGITestApplication() + browser = zope.testbrowser.wsgi.Browser(wsgi_app=app) + + # These statuses should redirect + for status in (301, 302, 303, 307): + browser.open('http://localhost/redirect.html?%s' % urlencode({ + 'to': 'http://localhost/set_status.html', + 'type': status, + })) + self.assertEqual(browser.url, 'http://localhost/set_status.html') + self.assertEqual(browser.headers['status'], '200 OK') + + # These should not + for status in (300, 304, 305, 306): + url = 'http://localhost/set_status.html?%s' % urlencode({ + 'status': status, + 'body': '', + }) + browser.open(url) + self.assertEqual(url, browser.url) + status_code = browser.headers['status'].split()[0] + self.assertEqual(status_code, str(status)) + # See https://github.com/zopefoundation/zope.testbrowser/pull/4#issuecomment-24302778 # noqa # # def test_no_redirect(self):