mirror of
https://github.com/kennethreitz/requests.git
synced 2026-06-05 22:50:18 +00:00
Merge pull request #4173 from jeremycline/https-proxy-issue
Pass pool_kwargs rather than updating connection_pool_kw
This commit is contained in:
Cory Benfield
+72
-79
@@ -10,10 +10,6 @@ and maintain connections.
|
||||
|
||||
import os.path
|
||||
import socket
|
||||
try:
|
||||
from threading import RLock
|
||||
except ImportError: # threading is an optional module and may not be present.
|
||||
from dummy_threading import RLock
|
||||
|
||||
from urllib3.poolmanager import PoolManager, proxy_from_url
|
||||
from urllib3.response import HTTPResponse
|
||||
@@ -53,6 +49,67 @@ DEFAULT_RETRIES = 0
|
||||
DEFAULT_POOL_TIMEOUT = None
|
||||
|
||||
|
||||
def _pool_kwargs(verify, cert):
|
||||
"""Create a dictionary of keyword arguments to pass to a
|
||||
:class:`PoolManager <urllib3.poolmanager.PoolManager>` with the
|
||||
necessary SSL configuration.
|
||||
|
||||
:param verify: Whether we should actually verify the certificate;
|
||||
optionally a path to a CA certificate bundle or
|
||||
directory of CA certificates.
|
||||
:param cert: The path to the client certificate and key, if any.
|
||||
This can either be the path to the certificate and
|
||||
key concatenated in a single file, or as a tuple of
|
||||
(cert_file, key_file).
|
||||
"""
|
||||
pool_kwargs = {}
|
||||
if verify:
|
||||
|
||||
cert_loc = None
|
||||
|
||||
# Allow self-specified cert location.
|
||||
if verify is not True:
|
||||
cert_loc = verify
|
||||
|
||||
if not cert_loc:
|
||||
cert_loc = DEFAULT_CA_BUNDLE_PATH
|
||||
|
||||
if not cert_loc or not os.path.exists(cert_loc):
|
||||
raise IOError("Could not find a suitable TLS CA certificate bundle, "
|
||||
"invalid path: {0}".format(cert_loc))
|
||||
|
||||
pool_kwargs['cert_reqs'] = 'CERT_REQUIRED'
|
||||
|
||||
if not os.path.isdir(cert_loc):
|
||||
pool_kwargs['ca_certs'] = cert_loc
|
||||
pool_kwargs['ca_cert_dir'] = None
|
||||
else:
|
||||
pool_kwargs['ca_cert_dir'] = cert_loc
|
||||
pool_kwargs['ca_certs'] = None
|
||||
else:
|
||||
pool_kwargs['cert_reqs'] = 'CERT_NONE'
|
||||
pool_kwargs['ca_certs'] = None
|
||||
pool_kwargs['ca_cert_dir'] = None
|
||||
|
||||
if cert:
|
||||
if not isinstance(cert, basestring):
|
||||
pool_kwargs['cert_file'] = cert[0]
|
||||
pool_kwargs['key_file'] = cert[1]
|
||||
else:
|
||||
pool_kwargs['cert_file'] = cert
|
||||
pool_kwargs['key_file'] = None
|
||||
|
||||
cert_file = pool_kwargs['cert_file']
|
||||
key_file = pool_kwargs['key_file']
|
||||
if cert_file and not os.path.exists(cert_file):
|
||||
raise IOError("Could not find the TLS certificate file, "
|
||||
"invalid path: {0}".format(cert_file))
|
||||
if key_file and not os.path.exists(key_file):
|
||||
raise IOError("Could not find the TLS key file, "
|
||||
"invalid path: {0}".format(key_file))
|
||||
return pool_kwargs
|
||||
|
||||
|
||||
class BaseAdapter(object):
|
||||
"""The Base Transport Adapter"""
|
||||
|
||||
@@ -126,7 +183,6 @@ class HTTPAdapter(BaseAdapter):
|
||||
self._pool_connections = pool_connections
|
||||
self._pool_maxsize = pool_maxsize
|
||||
self._pool_block = pool_block
|
||||
self._pool_kw_lock = RLock()
|
||||
|
||||
self.init_poolmanager(pool_connections, pool_maxsize, block=pool_block)
|
||||
|
||||
@@ -139,7 +195,6 @@ class HTTPAdapter(BaseAdapter):
|
||||
# self.poolmanager uses a lambda function, which isn't pickleable.
|
||||
self.proxy_manager = {}
|
||||
self.config = {}
|
||||
self._pool_kw_lock = RLock()
|
||||
|
||||
for attr, value in state.items():
|
||||
setattr(self, attr, value)
|
||||
@@ -204,65 +259,6 @@ class HTTPAdapter(BaseAdapter):
|
||||
|
||||
return manager
|
||||
|
||||
def _update_poolmanager_ssl_kw(self, verify, cert):
|
||||
"""Update the :class:`PoolManager <urllib3.poolmanager.PoolManager>`
|
||||
connection_pool_kw with the necessary SSL configuration. This method
|
||||
should not be called from user code, and is only exposed for use when
|
||||
subclassing the :class:`HTTPAdapter <requests.adapters.HTTPAdapter>`.
|
||||
|
||||
:param verify: Whether we should actually verify the certificate;
|
||||
optionally a path to a CA certificate bundle or
|
||||
directory of CA certificates.
|
||||
:param cert: The path to the client certificate and key, if any.
|
||||
This can either be the path to the certificate and
|
||||
key concatenated in a single file, or as a tuple of
|
||||
(cert_file, key_file).
|
||||
"""
|
||||
if verify:
|
||||
|
||||
cert_loc = None
|
||||
|
||||
# Allow self-specified cert location.
|
||||
if verify is not True:
|
||||
cert_loc = verify
|
||||
|
||||
if not cert_loc:
|
||||
cert_loc = DEFAULT_CA_BUNDLE_PATH
|
||||
|
||||
if not cert_loc or not os.path.exists(cert_loc):
|
||||
raise IOError("Could not find a suitable TLS CA certificate bundle, "
|
||||
"invalid path: {0}".format(cert_loc))
|
||||
|
||||
self.poolmanager.connection_pool_kw['cert_reqs'] = 'CERT_REQUIRED'
|
||||
|
||||
if not os.path.isdir(cert_loc):
|
||||
self.poolmanager.connection_pool_kw['ca_certs'] = cert_loc
|
||||
self.poolmanager.connection_pool_kw['ca_cert_dir'] = None
|
||||
else:
|
||||
self.poolmanager.connection_pool_kw['ca_cert_dir'] = cert_loc
|
||||
self.poolmanager.connection_pool_kw['ca_certs'] = None
|
||||
else:
|
||||
self.poolmanager.connection_pool_kw['cert_reqs'] = 'CERT_NONE'
|
||||
self.poolmanager.connection_pool_kw['ca_certs'] = None
|
||||
self.poolmanager.connection_pool_kw['ca_cert_dir'] = None
|
||||
|
||||
if cert:
|
||||
if not isinstance(cert, basestring):
|
||||
self.poolmanager.connection_pool_kw['cert_file'] = cert[0]
|
||||
self.poolmanager.connection_pool_kw['key_file'] = cert[1]
|
||||
else:
|
||||
self.poolmanager.connection_pool_kw['cert_file'] = cert
|
||||
self.poolmanager.connection_pool_kw['key_file'] = None
|
||||
|
||||
cert_file = self.poolmanager.connection_pool_kw['cert_file']
|
||||
key_file = self.poolmanager.connection_pool_kw['key_file']
|
||||
if cert_file and not os.path.exists(cert_file):
|
||||
raise IOError("Could not find the TLS certificate file, "
|
||||
"invalid path: {0}".format(cert_file))
|
||||
if key_file and not os.path.exists(key_file):
|
||||
raise IOError("Could not find the TLS key file, "
|
||||
"invalid path: {0}".format(key_file))
|
||||
|
||||
def build_response(self, req, resp):
|
||||
"""Builds a :class:`Response <requests.Response>` object from a urllib3
|
||||
response. This should not be called from user code, and is only exposed
|
||||
@@ -309,21 +305,18 @@ class HTTPAdapter(BaseAdapter):
|
||||
:param proxies: (optional) A Requests-style dictionary of proxies used on this request.
|
||||
:rtype: urllib3.ConnectionPool
|
||||
"""
|
||||
with self._pool_kw_lock:
|
||||
if url.lower().startswith('https'):
|
||||
self._update_poolmanager_ssl_kw(verify, cert)
|
||||
pool_kwargs = _pool_kwargs(verify, cert)
|
||||
proxy = select_proxy(url, proxies)
|
||||
|
||||
proxy = select_proxy(url, proxies)
|
||||
|
||||
if proxy:
|
||||
proxy = prepend_scheme_if_needed(proxy, 'http')
|
||||
proxy_manager = self.proxy_manager_for(proxy)
|
||||
conn = proxy_manager.connection_from_url(url)
|
||||
else:
|
||||
# Only scheme should be lower case
|
||||
parsed = urlparse(url)
|
||||
url = parsed.geturl()
|
||||
conn = self.poolmanager.connection_from_url(url)
|
||||
if proxy:
|
||||
proxy = prepend_scheme_if_needed(proxy, 'http')
|
||||
proxy_manager = self.proxy_manager_for(proxy)
|
||||
conn = proxy_manager.connection_from_url(url, pool_kwargs=pool_kwargs)
|
||||
else:
|
||||
# Only scheme should be lower case
|
||||
parsed = urlparse(url)
|
||||
url = parsed.geturl()
|
||||
conn = self.poolmanager.connection_from_url(url, pool_kwargs=pool_kwargs)
|
||||
|
||||
return conn
|
||||
|
||||
|
||||
@@ -31,6 +31,7 @@ from requests.structures import CaseInsensitiveDict
|
||||
from requests.sessions import SessionRedirectMixin
|
||||
from requests.models import urlencode
|
||||
from requests.hooks import default_hooks
|
||||
from requests.utils import DEFAULT_CA_BUNDLE_PATH
|
||||
|
||||
from .compat import StringIO, u
|
||||
from .utils import override_environ
|
||||
@@ -2765,3 +2766,200 @@ class TestPreparingURLs(object):
|
||||
r = requests.Request('GET', url=input, params=params)
|
||||
p = r.prepare()
|
||||
assert p.url == expected
|
||||
|
||||
|
||||
class TestGetConnection(object):
|
||||
"""
|
||||
Tests for the :meth:`requests.adapters.HTTPAdapter.get_connection` that assert
|
||||
the connections are correctly configured.
|
||||
"""
|
||||
@pytest.mark.parametrize(
|
||||
'proxies, verify, cert, expected',
|
||||
(
|
||||
(
|
||||
{},
|
||||
True,
|
||||
None,
|
||||
{
|
||||
'cert_reqs': 'CERT_REQUIRED',
|
||||
'ca_certs': DEFAULT_CA_BUNDLE_PATH,
|
||||
'ca_cert_dir': None,
|
||||
'cert_file': None,
|
||||
'key_file': None,
|
||||
},
|
||||
),
|
||||
(
|
||||
{},
|
||||
False,
|
||||
None,
|
||||
{
|
||||
'cert_reqs': 'CERT_NONE',
|
||||
'ca_certs': None,
|
||||
'ca_cert_dir': None,
|
||||
'cert_file': None,
|
||||
'key_file': None,
|
||||
},
|
||||
),
|
||||
(
|
||||
{},
|
||||
__file__,
|
||||
None,
|
||||
{
|
||||
'cert_reqs': 'CERT_REQUIRED',
|
||||
'ca_certs': __file__,
|
||||
'ca_cert_dir': None,
|
||||
'cert_file': None,
|
||||
'key_file': None,
|
||||
},
|
||||
),
|
||||
(
|
||||
{},
|
||||
os.path.dirname(__file__),
|
||||
None,
|
||||
{
|
||||
'cert_reqs': 'CERT_REQUIRED',
|
||||
'ca_certs': None,
|
||||
'ca_cert_dir': os.path.dirname(__file__),
|
||||
'cert_file': None,
|
||||
'key_file': None,
|
||||
},
|
||||
),
|
||||
(
|
||||
{},
|
||||
True,
|
||||
None,
|
||||
{
|
||||
'cert_reqs': 'CERT_REQUIRED',
|
||||
'ca_certs': DEFAULT_CA_BUNDLE_PATH,
|
||||
'ca_cert_dir': None,
|
||||
'cert_file': None,
|
||||
'key_file': None,
|
||||
},
|
||||
),
|
||||
(
|
||||
{},
|
||||
True,
|
||||
__file__,
|
||||
{
|
||||
'cert_reqs': 'CERT_REQUIRED',
|
||||
'ca_certs': DEFAULT_CA_BUNDLE_PATH,
|
||||
'ca_cert_dir': None,
|
||||
'cert_file': __file__,
|
||||
'key_file': None,
|
||||
},
|
||||
),
|
||||
(
|
||||
{},
|
||||
True,
|
||||
(__file__, __file__),
|
||||
{
|
||||
'cert_reqs': 'CERT_REQUIRED',
|
||||
'ca_certs': DEFAULT_CA_BUNDLE_PATH,
|
||||
'ca_cert_dir': None,
|
||||
'cert_file': __file__,
|
||||
'key_file': __file__,
|
||||
},
|
||||
),
|
||||
(
|
||||
{},
|
||||
True,
|
||||
(__file__, __file__),
|
||||
{
|
||||
'cert_reqs': 'CERT_REQUIRED',
|
||||
'ca_certs': DEFAULT_CA_BUNDLE_PATH,
|
||||
'ca_cert_dir': None,
|
||||
'cert_file': __file__,
|
||||
'key_file': __file__,
|
||||
},
|
||||
),
|
||||
(
|
||||
{'http': 'http://proxy.example.com', 'https': 'http://proxy.example.com'},
|
||||
True,
|
||||
None,
|
||||
{
|
||||
'cert_reqs': 'CERT_REQUIRED',
|
||||
'ca_certs': DEFAULT_CA_BUNDLE_PATH,
|
||||
'ca_cert_dir': None,
|
||||
'cert_file': None,
|
||||
'key_file': None,
|
||||
},
|
||||
),
|
||||
(
|
||||
{'http': 'http://proxy.example.com', 'https': 'http://proxy.example.com'},
|
||||
os.path.dirname(__file__),
|
||||
None,
|
||||
{
|
||||
'cert_reqs': 'CERT_REQUIRED',
|
||||
'ca_certs': None,
|
||||
'ca_cert_dir': os.path.dirname(__file__),
|
||||
'cert_file': None,
|
||||
'key_file': None,
|
||||
},
|
||||
),
|
||||
(
|
||||
{'http': 'http://proxy.example.com', 'https': 'http://proxy.example.com'},
|
||||
__file__,
|
||||
None,
|
||||
{
|
||||
'cert_reqs': 'CERT_REQUIRED',
|
||||
'ca_certs': __file__,
|
||||
'ca_cert_dir': None,
|
||||
'cert_file': None,
|
||||
'key_file': None,
|
||||
},
|
||||
),
|
||||
(
|
||||
{'http': 'http://proxy.example.com', 'https': 'http://proxy.example.com'},
|
||||
True,
|
||||
__file__,
|
||||
{
|
||||
'cert_reqs': 'CERT_REQUIRED',
|
||||
'ca_certs': DEFAULT_CA_BUNDLE_PATH,
|
||||
'ca_cert_dir': None,
|
||||
'cert_file': __file__,
|
||||
'key_file': None,
|
||||
},
|
||||
),
|
||||
(
|
||||
{'http': 'http://proxy.example.com', 'https': 'http://proxy.example.com'},
|
||||
True,
|
||||
(__file__, __file__),
|
||||
{
|
||||
'cert_reqs': 'CERT_REQUIRED',
|
||||
'ca_certs': DEFAULT_CA_BUNDLE_PATH,
|
||||
'ca_cert_dir': None,
|
||||
'cert_file': __file__,
|
||||
'key_file': __file__,
|
||||
},
|
||||
),
|
||||
)
|
||||
)
|
||||
def test_get_https_connection(self, proxies, verify, cert, expected):
|
||||
"""Assert connections are configured correctly."""
|
||||
adapter = requests.adapters.HTTPAdapter()
|
||||
connection = adapter.get_connection(
|
||||
'https://example.com', proxies=proxies, verify=verify, cert=cert)
|
||||
actual_config = {}
|
||||
for key, value in connection.__dict__.items():
|
||||
if key in expected:
|
||||
actual_config[key] = value
|
||||
assert actual_config == expected
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
'verify, cert',
|
||||
(
|
||||
('a/path/that/does/not/exist', None),
|
||||
(True, 'a/path/that/does/not/exist'),
|
||||
(True, (__file__, 'a/path/that/does/not/exist')),
|
||||
(True, ('a/path/that/does/not/exist', __file__)),
|
||||
)
|
||||
)
|
||||
def test_cert_files_missing(self, verify, cert):
|
||||
"""
|
||||
Assert an IOError is raised when one of the certificate files or
|
||||
directories can't be found.
|
||||
"""
|
||||
adapter = requests.adapters.HTTPAdapter()
|
||||
with pytest.raises(IOError) as excinfo:
|
||||
adapter.get_connection('https://example.com', verify=verify, cert=cert)
|
||||
excinfo.match('invalid path: a/path/that/does/not/exist')
|
||||
|
||||
Reference in New Issue
Block a user