Switch to Retry.remove_headers_on_redirect
Default to Authorization header.
Allow different settings on the Retry object.
Remove logic from ConnectionPool.
MODIFY
CHANGES.rst
CHANGES.rst
@@ -18,9 +18,9 @@ dev (master)
* Lazily load `uuid` to boost performance on imports (Pull #1270)
-* Stop forwarding the ``Authentication`` HTTP header on a cross-host
- redirect unless disabled by setting ``forward_auth_headers_across_hosts``
- to True on ``Retry`` objects. (Issue #1316)
+* Allow providing a list of headers to strip from requests when redirecting
+ to a different host. Defaults to the ``Authorization`` header. Different
+ headers can be set via ``Retry.remove_headers_on_redirect``. (Issue #1316)
* ... [Short description of non-trivial change.] (Issue #)

MODIFY
test/appengine/test_gae_manager.py
test/appengine/test_gae_manager.py
@@ -176,10 +176,6 @@ class TestGAERetry(test_connectionpool.TestRetry):
test_retry_redirect_history = None
test_multi_redirect_history = None
- # GAE doesn't have the assert_same_host parameter in urlopen()
- test_redirect_cross_host_forward_auth_headers = None
- test_redirect_cross_host_no_forward_auth_headers = None
-
@pytest.mark.usefixtures('testbed')
class TestGAERetryAfter(test_connectionpool.TestRetryAfter):

MODIFY
test/test_retry.py
test/test_retry.py
@@ -249,3 +249,13 @@ class TestRetry(object):
retry = Retry()
with pytest.raises(ReadTimeoutError):
retry.increment(method='POST', error=error)
+
+ def test_retry_default_remove_headers_on_redirect(self):
+ retry = Retry()
+
+ 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']

MODIFY
test/with_dummyserver/test_connectionpool.py
test/with_dummyserver/test_connectionpool.py
@@ -5,14 +5,11 @@ import sys
import unittest
import time
import warnings
-import json
import mock
from .. import (
- TARPIT_HOST,
- VALID_SOURCE_ADDRESSES,
- INVALID_SOURCE_ADDRESSES
+ TARPIT_HOST, VALID_SOURCE_ADDRESSES, INVALID_SOURCE_ADDRESSES,
)
from ..port_helpers import find_unused_port
from urllib3 import (
@@ -746,8 +743,6 @@ class TestRetry(HTTPDummyServerTestCase):
self.pool = HTTPConnectionPool(self.host, self.port)
self.addCleanup(self.pool.close)
- self.base_url_alt = 'http://%s:%d' % (self.host_alt, self.port)
-
def test_max_retry(self):
try:
r = self.pool.request('GET', '/redirect',
@@ -866,35 +861,6 @@ class TestRetry(HTTPDummyServerTestCase):
actual = [(history.status, history.redirect_location) for history in r.retries.history]
self.assertEqual(actual, expected)
- def test_redirect_cross_host_no_forward_auth_headers(self):
- url = '/redirect?target=%s/headers' % self.base_url_alt
- headers = {'Authentication': 'foo'}
-
- resp = self.pool.urlopen('GET', url, headers=headers, assert_same_host=False)
-
- self.assertEqual(resp.status, 200)
-
- data = json.loads(resp.data.decode('utf-8'))
-
- self.assertNotIn('Authentication', data)
-
- def test_redirect_cross_host_forward_auth_headers(self):
- url = '/redirect?target=%s/headers' % self.base_url_alt
- headers = {'Authentication': 'foo'}
- retry = Retry(redirect=2, forward_auth_headers_across_hosts=True)
-
- resp = self.pool.urlopen(
- 'GET', url, headers=headers,
- assert_same_host=False,
- retries=retry
- )
-
- self.assertEqual(resp.status, 200)
-
- data = json.loads(resp.data.decode('utf-8'))
-
- self.assertEqual(data['Authentication'], 'foo')
-
class TestRetryAfter(HTTPDummyServerTestCase):
def setUp(self):

MODIFY
test/with_dummyserver/test_poolmanager.py
test/with_dummyserver/test_poolmanager.py
@@ -109,34 +109,51 @@ class TestPoolManager(HTTPDummyServerTestCase):
except MaxRetryError:
pass
- def test_redirect_cross_host_strip_auth_headers(self):
+ def test_redirect_cross_host_remove_headers(self):
http = PoolManager()
self.addCleanup(http.clear)
r = http.request('GET', '%s/redirect' % self.base_url,
fields={'target': '%s/headers' % self.base_url_alt},
- headers={'Authentication': 'foo'})
+ headers={'Authorization': 'foo'})
self.assertEqual(r.status, 200)
data = json.loads(r.data.decode('utf-8'))
- self.assertNotIn('Authentication', data)
+ self.assertNotIn('Authorization', data)
- def test_redirect_cross_host_forward_auth_headers(self):
+ def test_redirect_cross_host_no_remove_headers(self):
http = PoolManager()
self.addCleanup(http.clear)
r = http.request('GET', '%s/redirect' % self.base_url,
fields={'target': '%s/headers' % self.base_url_alt},
- headers={'Authentication': 'foo'},
- retries=Retry(forward_auth_headers_across_hosts=True))
+ headers={'Authorization': 'foo'},
+ retries=Retry(remove_headers_on_redirect=[]))
self.assertEqual(r.status, 200)
data = json.loads(r.data.decode('utf-8'))
- self.assertEqual(data['Authentication'], 'foo')
+ self.assertEqual(data['Authorization'], 'foo')
+
+ def test_redirect_cross_host_set_removed_headers(self):
+ http = PoolManager()
+ self.addCleanup(http.clear)
+
+ 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.assertEqual(data['Authorization'], 'bar')
def test_raise_on_redirect(self):
http = PoolManager()

MODIFY
urllib3/connectionpool.py
urllib3/connectionpool.py
@@ -697,13 +697,6 @@ class HTTPConnectionPool(ConnectionPool, RequestMethods):
# drain and return the connection to the pool before recursing
drain_and_release_conn(response)
- # Strip the Authentication header if redirecting to a new host
- # and we don't want to forward that header across hosts.
- if (not retries.forward_auth_headers_across_hosts
- and 'Authentication' in headers
- and not self.is_same_host(redirect_location)):
- headers.pop('Authentication')
-
retries.sleep_for_retry(response)
log.debug("Redirecting %s -> %s", url, redirect_location)
return self.urlopen(

MODIFY
urllib3/poolmanager.py
urllib3/poolmanager.py
@@ -338,12 +338,12 @@ class PoolManager(RequestMethods):
if not isinstance(retries, Retry):
retries = Retry.from_int(retries, redirect=redirect)
- # Strip the Authentication header if redirecting to a new host
- # and we don't want to forward that header across hosts.
- if (not retries.forward_auth_headers_across_hosts
- and 'Authentication' in headers
+ # Strip headers marked as unsafe to forward to the redirected location.
+ if (retries.remove_headers_on_redirect
and not conn.is_same_host(redirect_location)):
- headers.pop('Authentication')
+ for header in retries.remove_headers_on_redirect:
+ if header in headers:
+ headers.pop(header)
try:
retries = retries.increment(method, url, response=response, _pool=conn)

MODIFY
urllib3/util/retry.py
urllib3/util/retry.py
@@ -139,10 +139,10 @@ class Retry(object):
Whether to respect Retry-After header on status codes defined as
:attr:`Retry.RETRY_AFTER_STATUS_CODES` or not.
- :param bool forward_auth_headers_across_hosts:
- Whether to forward Authentication headers if a response is received
- that redirects to a different host than the original request.
- Defaults to False.
+ :param iterable remove_headers_on_redirect:
+ Sequence of headers to remove from the request when a response
+ indicating a redirect is returned before firing off the redirected
+ request.
"""
DEFAULT_METHOD_WHITELIST = frozenset([
@@ -157,7 +157,7 @@ class Retry(object):
method_whitelist=DEFAULT_METHOD_WHITELIST, status_forcelist=None,
backoff_factor=0, raise_on_redirect=True, raise_on_status=True,
history=None, respect_retry_after_header=True,
- forward_auth_headers_across_hosts=False):
+ remove_headers_on_redirect=None):
self.total = total
self.connect = connect
@@ -168,6 +168,10 @@ class Retry(object):
redirect = 0
raise_on_redirect = False
+ if remove_headers_on_redirect is None:
+ remove_headers_on_redirect = ['Authorization']
+ remove_headers_on_redirect = set(remove_headers_on_redirect)
+
self.redirect = redirect
self.status_forcelist = status_forcelist or set()
self.method_whitelist = method_whitelist
@@ -176,8 +180,7 @@ 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.forward_auth_headers_across_hosts = \
- forward_auth_headers_across_hosts
+ self.remove_headers_on_redirect = remove_headers_on_redirect
def new(self, **kw):
params = dict(
@@ -189,7 +192,7 @@ class Retry(object):
raise_on_redirect=self.raise_on_redirect,
raise_on_status=self.raise_on_status,
history=self.history,
- forward_auth_headers_across_hosts=self.forward_auth_headers_across_hosts
+ remove_headers_on_redirect=self.remove_headers_on_redirect
)
params.update(kw)
return type(self)(**params)

Switch to Retry.remove_headers_on_redirect

- Default to Authorization header.
- Allow different settings on the Retry object.
- Remove logic from ConnectionPool.