diff --git a/AUTHORS.rst b/AUTHORS.rst index d011fccd..611d8063 100644 --- a/AUTHORS.rst +++ b/AUTHORS.rst @@ -171,3 +171,4 @@ Patches and Suggestions - Nate Prewitt (`@nateprewitt `_) - Maik Himstedt - Michael Hunsinger +- Jeremy Cline (`@jeremycline `_) diff --git a/requests/adapters.py b/requests/adapters.py index 885ffdc3..1032d2e4 100644 --- a/requests/adapters.py +++ b/requests/adapters.py @@ -10,6 +10,10 @@ 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 .models import Response from .packages.urllib3.poolmanager import PoolManager, proxy_from_url @@ -119,6 +123,7 @@ 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) @@ -131,6 +136,7 @@ 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) @@ -195,17 +201,21 @@ class HTTPAdapter(BaseAdapter): return manager - def cert_verify(self, conn, url, verify, cert): - """Verify a SSL certificate. This method should not be called from user - code, and is only exposed for use when subclassing the - :class:`HTTPAdapter `. + def _update_poolmanager_ssl_kw(self, verify, cert): + """Update the :class:`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 `. - :param conn: The urllib3 connection object associated with the cert. - :param url: The requested URL. - :param verify: Whether we should actually verify the certificate. - :param cert: The SSL certificate to verify. + :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 url.lower().startswith('https') and verify: + if verify: cert_loc = None @@ -219,23 +229,25 @@ class HTTPAdapter(BaseAdapter): if not cert_loc: raise Exception("Could not find a suitable SSL CA certificate bundle.") - conn.cert_reqs = 'CERT_REQUIRED' + self.poolmanager.connection_pool_kw['cert_reqs'] = 'CERT_REQUIRED' if not os.path.isdir(cert_loc): - conn.ca_certs = cert_loc + self.poolmanager.connection_pool_kw['ca_certs'] = cert_loc + self.poolmanager.connection_pool_kw['ca_cert_dir'] = None else: - conn.ca_cert_dir = cert_loc + self.poolmanager.connection_pool_kw['ca_cert_dir'] = cert_loc + self.poolmanager.connection_pool_kw['ca_certs'] = None else: - conn.cert_reqs = 'CERT_NONE' - conn.ca_certs = None - conn.ca_cert_dir = None + 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): - conn.cert_file = cert[0] - conn.key_file = cert[1] + self.poolmanager.connection_pool_kw['cert_file'] = cert[0] + self.poolmanager.connection_pool_kw['key_file'] = cert[1] else: - conn.cert_file = cert + self.poolmanager.connection_pool_kw['cert_file'] = cert def build_response(self, req, resp): """Builds a :class:`Response ` object from a urllib3 @@ -274,7 +286,7 @@ class HTTPAdapter(BaseAdapter): return response - def get_connection(self, url, proxies=None): + def get_connection(self, url, proxies=None, verify=None, cert=None): """Returns a urllib3 connection for the given URL. This should not be called from user code, and is only exposed for use when subclassing the :class:`HTTPAdapter `. @@ -283,17 +295,21 @@ class HTTPAdapter(BaseAdapter): :param proxies: (optional) A Requests-style dictionary of proxies used on this request. :rtype: requests.packages.urllib3.ConnectionPool """ - proxy = select_proxy(url, proxies) + with self._pool_kw_lock: + if url.lower().startswith('https'): + self._update_poolmanager_ssl_kw(verify, cert) - 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) + 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) return conn @@ -386,10 +402,8 @@ class HTTPAdapter(BaseAdapter): :param proxies: (optional) The proxies dictionary to apply to the request. :rtype: requests.Response """ + conn = self.get_connection(request.url, proxies, verify, cert) - conn = self.get_connection(request.url, proxies) - - self.cert_verify(conn, request.url, verify, cert) url = self.request_url(request, proxies) self.add_headers(request) diff --git a/tests/test_requests.py b/tests/test_requests.py index d8d7ea54..8fc89046 100755 --- a/tests/test_requests.py +++ b/tests/test_requests.py @@ -14,6 +14,7 @@ import warnings import io import requests import pytest +import pytest_httpbin from requests.adapters import HTTPAdapter from requests.auth import HTTPDigestAuth, _basic_auth_str from requests.compat import ( @@ -1487,6 +1488,34 @@ class TestRequests: resp.close() assert resp.raw.closed + def test_updating_ca_cert(self, httpbin_secure): + """Assert that requests use the latest configured CA certificates.""" + session = requests.session() + session.verify = pytest_httpbin.certs.where() + session.get(httpbin_secure('/')) + session.verify = True + with pytest.raises(requests.exceptions.SSLError) as e: + session.get(httpbin_secure('/')) + assert 'certificate verify failed' in str(e) + + def test_updating_client_cert(self, httpbin_secure): + """Assert that requests use the latest configured client certificates.""" + ca_file = pytest_httpbin.certs.where() + cert_dir = os.path.dirname(ca_file) + # All we need is a valid certificate and key to make a request. httpbin_secure + # won't check the signature or subject name, so it's okay that these happen to + # be the server's certificate and key. + cert = os.path.join(cert_dir, 'cert.pem') + key = os.path.join(cert_dir, 'key.pem') + session = requests.session() + session.verify = ca_file + resp = session.get(httpbin_secure('/')) + resp_with_cert = session.get(httpbin_secure('/'), cert=(cert, key)) + assert resp_with_cert.raw._pool.cert_file == cert + assert resp_with_cert.raw._pool.key_file == key + assert resp.raw._pool is not resp_with_cert.raw._pool + + class TestCaseInsensitiveDict: @pytest.mark.parametrize(