Ensured that SSLErrors will be retried
Fixes
Test fixes are due to the fact that SSLErrors are now wrapped in
a MaxRetryError.
MODIFY
test/test_connectionpool.py
test/test_connectionpool.py
@@ -193,18 +193,19 @@ class TestConnectionPool(object):
def _raise(ex):
raise ex()
- def _test(exception, expect):
+ def _test(exception, expect, reason=None):
pool._make_request = lambda *args, **kwargs: _raise(exception)
- with pytest.raises(expect):
+ with pytest.raises(expect) as excinfo:
pool.request('GET', '/')
-
+ if reason is not None:
+ assert isinstance(excinfo.value.reason, reason)
assert pool.pool.qsize() == POOL_SIZE
# Make sure that all of the exceptions return the connection
# to the pool
_test(Empty, EmptyPoolError)
- _test(BaseSSLError, SSLError)
- _test(CertificateError, SSLError)
+ _test(BaseSSLError, MaxRetryError, SSLError)
+ _test(CertificateError, MaxRetryError, SSLError)
# The pool should never be empty, and with these two exceptions
# being raised, a retry will be triggered, but that retry will

MODIFY
test/with_dummyserver/test_https.py
test/with_dummyserver/test_https.py
@@ -36,7 +36,7 @@ from urllib3.exceptions import (
InsecureRequestWarning,
SystemTimeWarning,
InsecurePlatformWarning,
-)
+ MaxRetryError)
from urllib3.packages import six
from urllib3.util.timeout import Timeout
import urllib3.util as util
@@ -171,10 +171,11 @@ class TestHTTPS(HTTPSDummyServerTestCase):
try:
https_pool.request('GET', '/')
self.fail("Didn't raise SSL invalid common name")
- except SSLError as e:
+ except MaxRetryError as e:
+ self.assertIsInstance(e.reason, SSLError)
self.assertTrue(
- "doesn't match" in str(e) or
- "certificate verify failed" in str(e)
+ "doesn't match" in str(e.reason) or
+ "certificate verify failed" in str(e.reason)
)
def test_verified_with_bad_ca_certs(self):
@@ -186,10 +187,11 @@ class TestHTTPS(HTTPSDummyServerTestCase):
try:
https_pool.request('GET', '/')
self.fail("Didn't raise SSL error with bad CA certs")
- except SSLError as e:
- self.assertTrue('certificate verify failed' in str(e),
+ except MaxRetryError as e:
+ self.assertIsInstance(e.reason, SSLError)
+ self.assertTrue('certificate verify failed' in str(e.reason),
"Expected 'certificate verify failed',"
- "instead got: %r" % e)
+ "instead got: %r" % e.reason)
def test_verified_without_ca_certs(self):
# default is cert_reqs=None which is ssl.CERT_NONE
@@ -201,23 +203,26 @@ class TestHTTPS(HTTPSDummyServerTestCase):
https_pool.request('GET', '/')
self.fail("Didn't raise SSL error with no CA certs when"
"CERT_REQUIRED is set")
- except SSLError as e:
+ except MaxRetryError as e:
+ self.assertIsInstance(e.reason, SSLError)
# there is a different error message depending on whether or
# not pyopenssl is injected
- self.assertTrue('No root certificates specified' in str(e) or
- 'certificate verify failed' in str(e) or
- 'invalid certificate chain' in str(e),
+ self.assertTrue('No root certificates specified' in str(e.reason) or
+ 'certificate verify failed' in str(e.reason) or
+ 'invalid certificate chain' in str(e.reason),
"Expected 'No root certificates specified', "
"'certificate verify failed', or "
"'invalid certificate chain', "
- "instead got: %r" % e)
+ "instead got: %r" % e.reason)
def test_no_ssl(self):
pool = HTTPSConnectionPool(self.host, self.port)
pool.ConnectionCls = None
self.addCleanup(pool.close)
self.assertRaises(SSLError, pool._new_conn)
- self.assertRaises(SSLError, pool.request, 'GET', '/')
+ with self.assertRaises(MaxRetryError) as cm:
+ pool.request('GET', '/', retries=0)
+ self.assertIsInstance(cm.exception.reason, SSLError)
def test_unverified_ssl(self):
""" Test that bare HTTPSConnection can connect, make requests """
@@ -324,17 +329,22 @@ class TestHTTPS(HTTPSDummyServerTestCase):
https_pool.assert_fingerprint = 'AA:AA:AA:AA:AA:AAAA:AA:AAAA:AA:' \
'AA:AA:AA:AA:AA:AA:AA:AA:AA'
- self.assertRaises(SSLError, https_pool.request, 'GET', '/')
+ def _test_request():
+ with self.assertRaises(MaxRetryError) as cm:
+ https_pool.request('GET', '/', retries=0)
+ self.assertIsInstance(cm.exception.reason, SSLError)
+
+ _test_request()
https_pool._get_conn()
# Uneven length
https_pool.assert_fingerprint = 'AA:A'
- self.assertRaises(SSLError, https_pool.request, 'GET', '/')
+ _test_request()
https_pool._get_conn()
# Invalid length
https_pool.assert_fingerprint = 'AA'
- self.assertRaises(SSLError, https_pool.request, 'GET', '/')
+ _test_request()
def test_verify_none_and_bad_fingerprint(self):
https_pool = HTTPSConnectionPool('127.0.0.1', self.port,
@@ -344,7 +354,9 @@ class TestHTTPS(HTTPSDummyServerTestCase):
https_pool.assert_fingerprint = 'AA:AA:AA:AA:AA:AAAA:AA:AAAA:AA:' \
'AA:AA:AA:AA:AA:AA:AA:AA:AA'
- self.assertRaises(SSLError, https_pool.request, 'GET', '/')
+ with self.assertRaises(MaxRetryError) as cm:
+ https_pool.request('GET', '/', retries=0)
+ self.assertIsInstance(cm.exception.reason, SSLError)
def test_verify_none_and_good_fingerprint(self):
https_pool = HTTPSConnectionPool('127.0.0.1', self.port,
@@ -510,7 +522,9 @@ class TestHTTPS_TLSv1(HTTPSDummyServerTestCase):
# is an issue with the OpenSSL for Python 2.6 on Windows.
self._pool.cert_reqs = 'CERT_REQUIRED'
- self.assertRaises(SSLError, self._pool.request, 'GET', '/')
+ with self.assertRaises(MaxRetryError) as cm:
+ self._pool.request('GET', '/', retries=0)
+ self.assertIsInstance(cm.exception.reason, SSLError)
self._pool.ca_certs = DEFAULT_CA
self._pool.request('GET', '/')

MODIFY
test/with_dummyserver/test_proxy_poolmanager.py
test/with_dummyserver/test_proxy_poolmanager.py
@@ -82,12 +82,13 @@ class TestHTTPProxyManager(HTTPDummyProxyTestCase):
https_pool = http._new_pool('https', self.https_host,
self.https_port)
try:
- https_pool.request('GET', '/')
+ https_pool.request('GET', '/', retries=0)
self.fail("Didn't raise SSL error with wrong CA")
- except SSLError as e:
- self.assertTrue('certificate verify failed' in str(e),
+ except MaxRetryError as e:
+ self.assertIsInstance(e.reason, SSLError)
+ self.assertTrue('certificate verify failed' in str(e.reason),
"Expected 'certificate verify failed',"
- "instead got: %r" % e)
+ "instead got: %r" % e.reason)
http = proxy_from_url(self.proxy_url, cert_reqs='REQUIRED',
ca_certs=DEFAULT_CA)
@@ -103,10 +104,11 @@ class TestHTTPProxyManager(HTTPDummyProxyTestCase):
https_fail_pool = http._new_pool('https', '127.0.0.1', self.https_port)
try:
- https_fail_pool.request('GET', '/')
+ https_fail_pool.request('GET', '/', retries=0)
self.fail("Didn't raise SSL invalid common name")
- except SSLError as e:
- self.assertTrue("doesn't match" in str(e))
+ except MaxRetryError as e:
+ self.assertIsInstance(e.reason, SSLError)
+ self.assertTrue("doesn't match" in str(e.reason))
def test_redirect(self):
http = proxy_from_url(self.proxy_url)

MODIFY
test/with_dummyserver/test_socketlevel.py
test/with_dummyserver/test_socketlevel.py
@@ -79,7 +79,7 @@ class TestSNI(SocketDummyServerTestCase):
self.addCleanup(pool.close)
try:
pool.request('GET', '/', retries=0)
- except SSLError: # We are violating the protocol
+ except MaxRetryError: # We are violating the protocol
pass
done_receiving.wait()
self.assertTrue(self.host.encode('ascii') in self.buf,
@@ -220,7 +220,7 @@ class TestClientCerts(SocketDummyServerTestCase):
self.addCleanup(pool.close)
try:
pool.request('GET', '/', retries=0)
- except SSLError:
+ except MaxRetryError:
done_receiving.set()
else:
done_receiving.set()
@@ -942,7 +942,9 @@ class TestSSL(SocketDummyServerTestCase):
pool = HTTPSConnectionPool(self.host, self.port)
self.addCleanup(pool.close)
- self.assertRaises(SSLError, pool.request, 'GET', '/', retries=0)
+ with self.assertRaises(MaxRetryError) as cm:
+ pool.request('GET', '/', retries=0)
+ self.assertIsInstance(cm.exception.reason, SSLError)
def test_ssl_read_timeout(self):
timed_out = Event()
@@ -1010,14 +1012,17 @@ class TestSSL(SocketDummyServerTestCase):
assert_fingerprint=fingerprint)
try:
response = pool.urlopen('GET', '/', preload_content=False,
- timeout=Timeout(connect=1, read=0.001))
+ timeout=Timeout(connect=1, read=0.001),
+ retries=0)
response.read()
finally:
pool.close()
- self.assertRaises(SSLError, request)
+ with self.assertRaises(MaxRetryError) as cm:
+ request()
+ self.assertIsInstance(cm.exception.reason, SSLError)
# Should not hang, see https://github.com/shazow/urllib3/issues/529
- self.assertRaises(SSLError, request)
+ self.assertRaises(MaxRetryError, request)
class TestErrorWrapping(SocketDummyServerTestCase):

MODIFY
urllib3/connectionpool.py
urllib3/connectionpool.py
@@ -622,25 +622,14 @@ class HTTPConnectionPool(ConnectionPool, RequestMethods):
# Timed out by queue.
raise EmptyPoolError(self, "No pool connections are available.")
- except (BaseSSLError, CertificateError) as e:
- # Close the connection. If a connection is reused on which there
- # was a Certificate error, the next request will certainly raise
- # another Certificate error.
- clean_exit = False
- raise SSLError(e)
-
- except SSLError:
- # Treat SSLError separately from BaseSSLError to preserve
- # traceback.
- clean_exit = False
- raise
-
- except (TimeoutError, HTTPException, SocketError, ProtocolError) as e:
+ except (TimeoutError, HTTPException, SocketError, ProtocolError,
+ BaseSSLError, SSLError, CertificateError) as e:
# Discard the connection for these exceptions. It will be
- # be replaced during the next _get_conn() call.
+ # replaced during the next _get_conn() call.
clean_exit = False
-
- if isinstance(e, (SocketError, NewConnectionError)) and self.proxy:
+ if isinstance(e, (BaseSSLError, CertificateError)):
+ e = SSLError(e)
+ elif isinstance(e, (SocketError, NewConnectionError)) and self.proxy:
e = ProxyError('Cannot connect to proxy.', e)
elif isinstance(e, (SocketError, HTTPException)):
e = ProtocolError('Connection aborted.', e)

Ensured that SSLErrors will be retried

Fixes #1112

Test fixes are due to the fact that SSLErrors are now wrapped in
a MaxRetryError.