fixing redirects for non-GET/HEAD/POST methods

requests/sessions.py

@@ -157,11 +157,13 @@ class SessionRedirectMixin(object):
             if response.is_permanent_redirect and request.url != prepared_request.url:
                 self.redirect_cache[request.url] = prepared_request.url
 
+            old_method = prepared_request.method
             self.rebuild_method(prepared_request, response)
+            new_method = prepared_request.method
 
-            # 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 old_method != new_method and new_method == 'GET':
                 # https://github.com/kennethreitz/requests/issues/3490
                 purged_headers = ('Content-Length', 'Content-Type', 'Transfer-Encoding')
                 for header in purged_headers:
@@ -289,14 +291,12 @@ class SessionRedirectMixin(object):
         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

tests/test_requests.py

@@ -234,12 +234,52 @@ class TestRequests:
 
     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_301_doesnt_change_non_post_to_get(self, httpbin):
+        r = requests.patch(httpbin('redirect-to'),
+                           data='test body',
+                           params={'url': 'patch', 'status_code': '301'})
+        assert r.status_code == 200
+        assert r.request.method == 'PATCH'
+        assert r.history[0].status_code == 301
+        assert r.history[0].is_redirect
+        assert r.request.body == 'test body'
+        assert r.json()['data'] == 'test body'
+
+    @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
+
+        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
@@ -254,6 +294,36 @@ class TestRequests:
         assert r.history[0].status_code == 302
         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_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
+
+        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
@@ -268,6 +338,33 @@ class TestRequests:
         assert r.history[0].status_code == 303
         assert r.history[0].is_redirect
 
+    @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')]