From b644af0ec7f3cd92d71f6a266b1888e56975b4c1 Mon Sep 17 00:00:00 2001
From: Cory Benfield <lukasaoz@gmail.com>
Date: Wed, 21 Oct 2015 09:23:08 +0100
Subject: [PATCH] Make sure we build environment settings properly.

---
 requests/sessions.py | 31 ++++++++++++++++----------
 test_requests.py     | 52 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 72 insertions(+), 11 deletions(-)

diff --git a/requests/sessions.py b/requests/sessions.py
index b11bdb6d..dde462f0 100644
--- a/requests/sessions.py
+++ b/requests/sessions.py
@@ -618,24 +618,33 @@ class Session(SessionRedirectMixin):
 
     def merge_environment_settings(self, url, proxies, stream, verify, cert):
         """Check the environment and merge it with some settings."""
-        # Gather clues from the surrounding environment.
-        if self.trust_env:
-            # Set environment's proxies.
-            env_proxies = get_environ_proxies(url) or {}
-            for (k, v) in env_proxies.items():
-                proxies.setdefault(k, v)
+        # Merge all the kwargs except for proxies.
+        stream = merge_setting(stream, self.stream)
+        verify = merge_setting(verify, self.verify)
+        cert = merge_setting(cert, self.cert)
 
+        # Gather clues from the surrounding environment.
+        # We do this after merging the Session values to make sure we don't
+        # accidentally exclude them.
+        if self.trust_env:
             # Look for requests environment configuration and be compatible
             # with cURL.
             if verify is True or verify is None:
                 verify = (os.environ.get('REQUESTS_CA_BUNDLE') or
                           os.environ.get('CURL_CA_BUNDLE'))
 
-        # Merge all the kwargs.
-        proxies = merge_setting(proxies, self.proxies)
-        stream = merge_setting(stream, self.stream)
-        verify = merge_setting(verify, self.verify)
-        cert = merge_setting(cert, self.cert)
+        # Now we handle proxies.
+        # Proxies need to be built up backwards. This is because None values
+        # can delete proxy information, which can then be re-added by a more
+        # specific layer. So we begin by getting the environment's proxies,
+        # then add the Session, then add the request.
+        env_proxies = {}
+
+        if self.trust_env:
+            env_proxies = get_environ_proxies(url) or {}
+
+        new_proxies = merge_setting(self.proxies, env_proxies)
+        proxies = merge_setting(proxies, new_proxies)
 
         return {'verify': verify, 'proxies': proxies, 'stream': stream,
                 'cert': cert}
diff --git a/test_requests.py b/test_requests.py
index 45afd517..9e2b05d5 100755
--- a/test_requests.py
+++ b/test_requests.py
@@ -68,6 +68,21 @@ def httpsbin_url(httpbin_secure):
     return inner
 
 
+class SendRecordingAdapter(HTTPAdapter):
+    """
+    A basic subclass of the HTTPAdapter that records the arguments used to
+    ``send``.
+    """
+    def __init__(self, *args, **kwargs):
+        super(SendRecordingAdapter, self).__init__(*args, **kwargs)
+
+        self.send_calls = []
+
+    def send(self, *args, **kwargs):
+        self.send_calls.append((args, kwargs))
+        return super(SendRecordingAdapter, self).send(*args, **kwargs)
+
+
 # Requests to this URL should always fail with a connection timeout (nothing
 # listening on that port)
 TARPIT = "http://10.255.255.1"
@@ -1141,6 +1156,43 @@ class TestRequests:
         next(r.iter_lines())
         assert len(list(r.iter_lines())) == 3
 
+    def test_environment_comes_after_session(self):
+        """The Session arguments should come before environment arguments."""
+        # We get proxies from the environment and verify from the argument.
+        s = requests.Session()
+        a = SendRecordingAdapter()
+        s.mount('http://', a)
+
+        # Both of these arguments are safe fallbacks that we can easily
+        # detect, but which will allow the request to succeed.
+        s.verify = False
+        s.proxies = {'http': None}
+
+        old_proxy = os.environ.get('HTTP_PROXY')
+        old_bundle = os.environ.get('REQUESTS_CA_BUNDLE')
+
+        try:
+            os.environ['HTTP_PROXY'] = '10.10.10.10:3128'
+            os.environ['REQUESTS_CA_BUNDLE'] = '/path/to/nowhere'
+
+            s.get(httpbin('get'), timeout=5)
+        finally:
+            if old_proxy is not None:
+                os.environ['HTTP_PROXY'] = old_proxy
+            else:
+                del os.environ['HTTP_PROXY']
+
+            if old_bundle is not None:
+                os.environ['REQUESTS_CA_BUNDLE'] = old_bundle
+            else:
+                del os.environ['REQUESTS_CA_BUNDLE']
+
+        call = a.send_calls[0]
+        assert call[1]['verify'] == False
+
+        proxies = call[1]['proxies']
+        with pytest.raises(KeyError):
+            proxies['http']
 
 class TestContentEncodingDetection: