From f24d56a1ce658a20577517d05bcb92e44e3ffc53 Mon Sep 17 00:00:00 2001 From: Ben Date: Thu, 17 Mar 2016 23:16:39 -0500 Subject: [PATCH] Consolidate logic for changing method during redirects I only moved the code into a function, there was no actual change to the code. I added a few tests to ensure we're doing things correctly. The real point of me doing this is to make it easier to bring back `strict_mode` functionality. For you requests youngsters in the crowd, `strict_mode` followed the spec for redirects meaning the method wouldn't change to a GET. The current code follows the browser convention of changing the method to a GET when doing a 302 redirect. However, lots of servers want you to follow the standards (the nerve!) so I'd like to override the logic. Now that the method changing logic is in `rebuild_method`, I can simply override that function instead of overriding the entire `resolve_redirects` function as suggested by kennethreitz/requests#1325 --- requests/sessions.py | 40 ++++++++++++++++++++++----------------- tests/test_requests.py | 43 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 17 deletions(-) diff --git a/requests/sessions.py b/requests/sessions.py index 639668f2..ba088bf2 100644 --- a/requests/sessions.py +++ b/requests/sessions.py @@ -116,7 +116,6 @@ class SessionRedirectMixin(object): resp.close() url = resp.headers['location'] - method = req.method # Handle redirection without scheme (see: RFC 1808 Section 4) if url.startswith('//'): @@ -140,22 +139,7 @@ class SessionRedirectMixin(object): if resp.is_permanent_redirect and req.url != prepared_request.url: self.redirect_cache[req.url] = prepared_request.url - # http://tools.ietf.org/html/rfc7231#section-6.4.4 - if (resp.status_code == codes.see_other and - method != 'HEAD'): - method = 'GET' - - # Do what the browsers do, despite standards... - # First, turn 302s into GETs. - if resp.status_code == codes.found and method != 'HEAD': - method = 'GET' - - # Second, if a POST is responded to with a 301, turn it into a GET. - # This bizarre behaviour is explained in Issue 1704. - if resp.status_code == codes.moved and method == 'POST': - method = 'GET' - - prepared_request.method = method + self.rebuild_method(prepared_request, resp) # https://github.com/kennethreitz/requests/issues/1084 if resp.status_code not in (codes.temporary_redirect, codes.permanent_redirect): @@ -262,6 +246,28 @@ class SessionRedirectMixin(object): return new_proxies + def rebuild_method(self, prepared_request, response): + """When being redirected we may want to change the method of the request + based on certain specs or browser behavior. + """ + method = prepared_request.method + + # http://tools.ietf.org/html/rfc7231#section-6.4.4 + if response.status_code == codes.see_other and method != 'HEAD': + method = 'GET' + + # Do what the browsers do, despite standards... + # First, turn 302s into GETs. + if response.status_code == codes.found and method != 'HEAD': + method = 'GET' + + # Second, if a POST is responded to with a 301, turn it into a GET. + # This bizarre behaviour is explained in Issue 1704. + if response.status_code == codes.moved and method == 'POST': + method = 'GET' + + prepared_request.method = method + class Session(SessionRedirectMixin): """A Requests session. diff --git a/tests/test_requests.py b/tests/test_requests.py index 01e88da1..01811b8a 100755 --- a/tests/test_requests.py +++ b/tests/test_requests.py @@ -162,6 +162,49 @@ class TestRequests: else: pytest.fail('Expected custom max number of redirects to be respected but was not') + def test_http_301_changes_post_to_get(self, httpbin): + r = requests.post(httpbin('status', '301')) + assert r.status_code == 200 + assert r.request.method == 'GET' + assert r.history[0].status_code == 301 + assert r.history[0].is_redirect + + def test_http_301_doesnt_change_head_to_get(self, httpbin): + r = requests.head(httpbin('status', '301'), allow_redirects=True) + print(r.content) + assert r.status_code == 200 + assert r.request.method == 'HEAD' + assert r.history[0].status_code == 301 + assert r.history[0].is_redirect + + def test_http_302_changes_post_to_get(self, httpbin): + r = requests.post(httpbin('status', '302')) + assert r.status_code == 200 + assert r.request.method == 'GET' + assert r.history[0].status_code == 302 + assert r.history[0].is_redirect + + def test_http_302_doesnt_change_head_to_get(self, httpbin): + r = requests.head(httpbin('status', '302'), allow_redirects=True) + assert r.status_code == 200 + assert r.request.method == 'HEAD' + assert r.history[0].status_code == 302 + assert r.history[0].is_redirect + + def test_http_303_changes_post_to_get(self, httpbin): + r = requests.post(httpbin('status', '303')) + assert r.status_code == 200 + assert r.request.method == 'GET' + assert r.history[0].status_code == 303 + assert r.history[0].is_redirect + + def test_http_303_doesnt_change_head_to_get(self, httpbin): + r = requests.head(httpbin('status', '303'), allow_redirects=True) + assert r.status_code == 200 + assert r.request.method == 'HEAD' + assert r.history[0].status_code == 303 + assert r.history[0].is_redirect + # def test_HTTP_302_ALLOW_REDIRECT_POST(self): # r = requests.post(httpbin('status', '302'), data={'some': 'data'}) # self.assertEqual(r.status_code, 200)