Merge pull request #22 from zopefoundation/fix-redirect-handling

Don't follow redirects for all 30x response status codes.

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)

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:

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')

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):