diff --git a/3.0-HISTORY.rst b/3.0-HISTORY.rst index 1956a5d1..34bcfaa3 100644 --- a/3.0-HISTORY.rst +++ b/3.0-HISTORY.rst @@ -1,6 +1,9 @@ 3.0.0 (2016-xx-xx) ++++++++++++++++++ +- Relax how Requests strips bodies from redirects. 3.0.0 only supports body + removal on 301/302 POST redirects and all 303 redirects. + - Remove support for non-string/bytes parameters in ``_basic_auth_str``. - Prevent ``Session.merge_environment`` from erroneously setting the diff --git a/requests/sessions.py b/requests/sessions.py index f6d39827..ec315e64 100644 --- a/requests/sessions.py +++ b/requests/sessions.py @@ -157,11 +157,11 @@ class SessionRedirectMixin(object): if response.is_permanent_redirect and request.url != prepared_request.url: self.redirect_cache[request.url] = prepared_request.url - self.rebuild_method(prepared_request, response) + method_changed = self.rebuild_method(prepared_request, response) - # https://github.com/kennethreitz/requests/issues/1084 - if response.status_code not in (codes.temporary_redirect, - codes.permanent_redirect): + # https://github.com/kennethreitz/requests/issues/2590 + # If method is changed to GET we need to remove body and associated headers. + if method_changed and prepared_request.method == 'GET': # https://github.com/kennethreitz/requests/issues/3490 purged_headers = ('Content-Length', 'Content-Type', 'Transfer-Encoding') for header in purged_headers: @@ -282,24 +282,26 @@ class SessionRedirectMixin(object): 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. + + :rtype bool: + :return: boolean expressing if the method changed during rebuild. """ - method = prepared_request.method + method = original_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': + # If a POST is responded to with a 301 or 302, turn it into a GET. This has + # become a common pattern in browsers and was introduced into later versions + # of HTTP RFCs. While some browsers transform other methods to GET, little of + # that has been standardized. For that reason, we're using curl as a model + # which only supports POST->GET. + if response.status_code in (codes.found, codes.moved) and method == 'POST': method = 'GET' prepared_request.method = method + return method != original_method class Session(SessionRedirectMixin): diff --git a/tests/test_requests.py b/tests/test_requests.py index fdb0fa6a..336f68dc 100755 --- a/tests/test_requests.py +++ b/tests/test_requests.py @@ -225,48 +225,92 @@ 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' + @pytest.mark.parametrize( + 'method, body, expected', ( + ('GET', None, 'GET'), + ('HEAD', None, 'HEAD'), + ('POST', 'test', 'GET'), + ('PUT', 'put test', 'PUT'), + ('PATCH', 'patch test', 'PATCH'), + ('DELETE', '', 'DELETE') + ) + ) + def test_http_301_for_redirectable_methods(self, httpbin, method, body, expected): + """Tests all methods except OPTIONS for expected redirect behaviour. + + OPTIONS responses can behave differently depending on the server, so + we don't have anything uniform to test except how httpbin responds + to them. For that reason they aren't included here. + """ + params = {'url': '/%s' % expected.lower(), 'status_code': '301'} + r = requests.request(method, httpbin('redirect-to'), data=body, params=params) + + assert r.request.url == httpbin(expected.lower()) + assert r.request.method == expected 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 + if expected in ('GET', 'HEAD'): + assert r.request.body is None + else: + assert r.json()['data'] == body - 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' + @pytest.mark.parametrize( + 'method, body, expected', ( + ('GET', None, 'GET'), + ('HEAD', None, 'HEAD'), + ('POST', 'test', 'GET'), + ('PUT', 'put test', 'PUT'), + ('PATCH', 'patch test', 'PATCH'), + ('DELETE', '', 'DELETE') + ) + ) + def test_http_302_for_redirectable_methods(self, httpbin, method, body, expected): + """Tests all methods except OPTIONS for expected redirect behaviour. + + OPTIONS responses can behave differently depending on the server, so + we don't have anything uniform to test except how httpbin responds + to them. For that reason they aren't included here. + """ + params = {'url': '/%s' % expected.lower()} + r = requests.request(method, httpbin('redirect-to'), data=body, params=params) + + assert r.request.url == httpbin(expected.lower()) + assert r.request.method == expected 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 + if expected in ('GET', 'HEAD'): + assert r.request.body is None + else: + assert r.json()['data'] == body - 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' + @pytest.mark.parametrize( + 'method, body, expected', ( + ('GET', None, 'GET'), + ('HEAD', None, 'HEAD'), + ('POST', 'test', 'GET'), + ('PUT', 'put test', 'GET'), + ('PATCH', 'patch test', 'GET'), + ('DELETE', '', 'GET') + ) + ) + def test_http_303_for_redirectable_methods(self, httpbin, method, body, expected): + """Tests all methods except OPTIONS for expected redirect behaviour. + + OPTIONS responses can behave differently depending on the server, so + we don't have anything uniform to test except how httpbin responds + to them. For that reason they aren't included here. + """ + params = {'url': '/%s' % expected.lower(), 'status_code': '303'} + r = requests.request(method, httpbin('redirect-to'), data=body, params=params) + + assert r.request.url == httpbin(expected.lower()) + assert r.request.method == expected 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 + assert r.request.body is None def test_multiple_location_headers(self, httpbin): headers = [('Location', 'http://example.com'),