Fix #1955: Do not use original request in redirect

The original request was never being properly overriden in resolve_redirects.
As such being having a POST request respond with a 303 would generate a GET
request. If the GET request encountered another redirect to something like a
307, then it would use the original request and generate another POST request.

There are two parts to this fix:

- The fix itself
- The test infrastructure to ensure it does not regress because HTTPBin is
  insufficient

diff --git a/requests/sessions.py b/requests/sessions.py
index 425db22ca6fbe6e422b1e6dd61f9185915adf372..fc5b0ffa18c43a870589de130a1cd03f8046fdff 100644 (file)
--- a/requests/sessions.py
+++ b/requests/sessions.py
@@ -168,8 +168,11 @@ class SessionRedirectMixin(object):
             if new_auth is not None:
                 prepared_request.prepare_auth(new_auth)
 
+            # Override the original request.
+            req = prepared_request
+
             resp = self.send(
-                prepared_request,
+                req,
                 stream=stream,
                 timeout=timeout,
                 verify=verify,

diff --git a/test_requests.py b/test_requests.py
index 17de84911bb32db1df00aef1224f3bc917864e92..0b3c106b926451452e5da76a2dc29dbe296c8de0 100755 (executable)
--- a/test_requests.py
+++ b/test_requests.py
@@ -8,6 +8,7 @@ import json
 import os
 import pickle
 import unittest
+import collections
 
 import requests
 import pytest
@@ -18,6 +19,7 @@ from requests.compat import (
 from requests.cookies import cookiejar_from_dict, morsel_to_cookie
 from requests.exceptions import InvalidURL, MissingSchema
 from requests.structures import CaseInsensitiveDict
+from requests.sessions import SessionRedirectMixin
 
 try:
     import StringIO
@@ -1187,5 +1189,64 @@ class TestTimeout:
             assert 'Read timed out' in e.args[0].args[0]
 
 
+SendCall = collections.namedtuple('SendCall', ('args', 'kwargs'))
+
+
+class RedirectSession(SessionRedirectMixin):
+    def __init__(self, order_of_redirects):
+        self.redirects = order_of_redirects
+        self.calls = []
+        self.max_redirects = 30
+        self.cookies = {}
+        self.trust_env = False
+
+    def send(self, *args, **kwargs):
+        self.calls.append(SendCall(args, kwargs))
+        return self.build_response()
+
+    def build_response(self):
+        request = self.calls[-1].args[0]
+        r = requests.Response()
+
+        try:
+            r.status_code = int(self.redirects.pop(0))
+        except IndexError:
+            r.status_code = 200
+
+        r.headers = CaseInsensitiveDict({'Location': '/'})
+        r.raw = self._build_raw()
+        r.request = request
+        return r
+
+    def _build_raw(self):
+        string = StringIO.StringIO('')
+        setattr(string, 'release_conn', lambda *args: args)
+        return string
+
+
+class TestRedirects:
+    default_keyword_args = {
+        'stream': False,
+        'verify': True,
+        'cert': None,
+        'timeout': None,
+        'allow_redirects': False,
+        'proxies': None,
+    }
+
+    def test_requests_are_updated_each_time(self):
+        session = RedirectSession([303, 307])
+        prep = requests.Request('POST', 'http://httpbin.org/post').prepare()
+        r0 = session.send(prep)
+        assert r0.request.method == 'POST'
+        assert session.calls[-1] == SendCall((r0.request,), {})
+        redirect_generator = session.resolve_redirects(r0, prep)
+        for response in redirect_generator:
+            assert response.request.method == 'GET'
+            send_call = SendCall((response.request,),
+                                 TestRedirects.default_keyword_args)
+            assert session.calls[-1] == send_call
+
+
 if __name__ == '__main__':
     unittest.main()