Remove Authorization headers regardless of case on cross-origin redirects (#1511)

6 files changed, 39 insertions, 5 deletions

diff --git a/CHANGES.rst b/CHANGES.rst
index 8ef3f153..4e9a1abe 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -8,6 +8,8 @@ dev (master)
 
 * Upgraded ``urllib3.utils.parse_url()`` to be RFC 3986 compliant. (Pull #1487)
 
+* Remove Authorization header regardless of case when redirecting to cross-site. (Issue #1510)
+
 * ... [Short description of non-trivial change.] (Issue #)
 
 
diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt
index 71813f95..02b24fc7 100644
--- a/CONTRIBUTORS.txt
+++ b/CONTRIBUTORS.txt
@@ -272,5 +272,8 @@ In chronological order:
 * Justin Bramley <https://github.com/jbramleycl>
   * Add ability to handle multiple Content-Encodings
 
+* Katsuhiko YOSHIDA <https://github.com/kyoshidajp>
+  * Remove Authorization header regardless of case when redirecting to cross-site
+
 * [Your name or handle] <[email or website]>
   * [Brief summary of your changes]
diff --git a/src/urllib3/poolmanager.py b/src/urllib3/poolmanager.py
index 70393249..a13149b1 100644
--- a/src/urllib3/poolmanager.py
+++ b/src/urllib3/poolmanager.py
@@ -7,6 +7,7 @@ from ._collections import RecentlyUsedContainer
 from .connectionpool import HTTPConnectionPool, HTTPSConnectionPool
 from .connectionpool import port_by_scheme
 from .exceptions import LocationValueError, MaxRetryError, ProxySchemeUnknown
+from .packages import six
 from .packages.six.moves.urllib.parse import urljoin
 from .request import RequestMethods
 from .util.url import parse_url
@@ -342,8 +343,10 @@ class PoolManager(RequestMethods):
         # conn.is_same_host() which may use socket.gethostbyname() in the future.
         if (retries.remove_headers_on_redirect
                 and not conn.is_same_host(redirect_location)):
-            for header in retries.remove_headers_on_redirect:
-                kw['headers'].pop(header, None)
+            headers = list(six.iterkeys(kw['headers']))
+            for header in headers:
+                if header.lower() in retries.remove_headers_on_redirect:
+                    kw['headers'].pop(header, None)
 
         try:
             retries = retries.increment(method, url, response=response, _pool=conn)
diff --git a/src/urllib3/util/retry.py b/src/urllib3/util/retry.py
index e7d0abd6..02429ee8 100644
--- a/src/urllib3/util/retry.py
+++ b/src/urllib3/util/retry.py
@@ -179,7 +179,8 @@ class Retry(object):
         self.raise_on_status = raise_on_status
         self.history = history or tuple()
         self.respect_retry_after_header = respect_retry_after_header
-        self.remove_headers_on_redirect = remove_headers_on_redirect
+        self.remove_headers_on_redirect = frozenset([
+            h.lower() for h in remove_headers_on_redirect])
 
     def new(self, **kw):
         params = dict(
diff --git a/test/test_retry.py b/test/test_retry.py
index b4119b10..886a2214 100644
--- a/test/test_retry.py
+++ b/test/test_retry.py
@@ -253,9 +253,9 @@ class TestRetry(object):
     def test_retry_default_remove_headers_on_redirect(self):
         retry = Retry()
 
-        assert list(retry.remove_headers_on_redirect) == ['Authorization']
+        assert list(retry.remove_headers_on_redirect) == ['authorization']
 
     def test_retry_set_remove_headers_on_redirect(self):
         retry = Retry(remove_headers_on_redirect=['X-API-Secret'])
 
-        assert list(retry.remove_headers_on_redirect) == ['X-API-Secret']
+        assert list(retry.remove_headers_on_redirect) == ['x-api-secret']
diff --git a/test/with_dummyserver/test_poolmanager.py b/test/with_dummyserver/test_poolmanager.py
index 2a13722c..de0b9172 100644
--- a/test/with_dummyserver/test_poolmanager.py
+++ b/test/with_dummyserver/test_poolmanager.py
@@ -123,6 +123,17 @@ class TestPoolManager(HTTPDummyServerTestCase):
 
         self.assertNotIn('Authorization', data)
 
+        r = http.request('GET', '%s/redirect' % self.base_url,
+                         fields={'target': '%s/headers' % self.base_url_alt},
+                         headers={'authorization': 'foo'})
+
+        self.assertEqual(r.status, 200)
+
+        data = json.loads(r.data.decode('utf-8'))
+
+        self.assertNotIn('authorization', data)
+        self.assertNotIn('Authorization', data)
+
     def test_redirect_cross_host_no_remove_headers(self):
         http = PoolManager()
         self.addCleanup(http.clear)
@@ -155,6 +166,20 @@ class TestPoolManager(HTTPDummyServerTestCase):
         self.assertNotIn('X-API-Secret', data)
         self.assertEqual(data['Authorization'], 'bar')
 
+        r = http.request('GET', '%s/redirect' % self.base_url,
+                         fields={'target': '%s/headers' % self.base_url_alt},
+                         headers={'x-api-secret': 'foo',
+                                  'authorization': 'bar'},
+                         retries=Retry(remove_headers_on_redirect=['X-API-Secret']))
+
+        self.assertEqual(r.status, 200)
+
+        data = json.loads(r.data.decode('utf-8'))
+
+        self.assertNotIn('x-api-secret', data)
+        self.assertNotIn('X-API-Secret', data)
+        self.assertEqual(data['Authorization'], 'bar')
+
     def test_raise_on_redirect(self):
         http = PoolManager()
         self.addCleanup(http.clear)