Skip to content

Commit

Permalink
Merge pull request #3757 from nateprewitt/2590_redirected_bodies
Browse files Browse the repository at this point in the history
301/302 redirects for non-POST with body
Lukasa authored Dec 12, 2016
2 parents f554ec7 + 354ac7e commit 0b4302b
Showing 3 changed files with 93 additions and 44 deletions.
3 changes: 3 additions & 0 deletions 3.0-HISTORY.rst
Original file line number Diff line number Diff line change
@@ -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
28 changes: 15 additions & 13 deletions requests/sessions.py
Original file line number Diff line number Diff line change
@@ -157,11 +157,11 @@ def resolve_redirects(self, response, stream=False, timeout=None,
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 @@ def rebuild_proxies(self, prepared_request, 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.
: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):
106 changes: 75 additions & 31 deletions tests/test_requests.py
Original file line number Diff line number Diff line change
@@ -225,49 +225,93 @@ def test_HTTP_302_TOO_MANY_REDIRECTS_WITH_PARAMS(self, httpbin):
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
@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.
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'
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_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
if expected in ('GET', 'HEAD'):
assert r.request.body is None
else:
assert r.json()['data'] == body

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'
@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_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
if expected in ('GET', 'HEAD'):
assert r.request.body is None
else:
assert r.json()['data'] == body

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

assert r.request.body is None

def test_multiple_location_headers(self, httpbin):
headers = [('Location', 'http://example.com'),
('Location', 'https://example.com/1')]

0 comments on commit 0b4302b

Please sign in to comment.