From a3b9a34be9dccfc42df65b2403bc0c6666fd3392 Mon Sep 17 00:00:00 2001 From: Cory Benfield Date: Wed, 10 Feb 2016 09:49:24 +0000 Subject: [PATCH 1/6] Revert "We do not require square brackets for httplib." This reverts commit 019dbaeadb6318d98c78f2c874e2d49c06ebda15. --- urllib3/connectionpool.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/urllib3/connectionpool.py b/urllib3/connectionpool.py index 5a9d766a82..6f6e9053cd 100644 --- a/urllib3/connectionpool.py +++ b/urllib3/connectionpool.py @@ -69,7 +69,8 @@ def __init__(self, host, port=None): if not host: raise LocationValueError("No host specified.") - self.host = host + # httplib doesn't like it when we include brackets in ipv6 addresses + self.host = host.strip('[]') self.port = port def __str__(self): From 9689f4096fb9ef44c100eadac1b2ea59c553b008 Mon Sep 17 00:00:00 2001 From: Cory Benfield Date: Wed, 10 Feb 2016 09:50:39 +0000 Subject: [PATCH 2/6] Revert "Better explanation." This reverts commit fd4c7d769653a443b1fdb54da4bc8c71ba09bfdf. --- urllib3/connection.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/urllib3/connection.py b/urllib3/connection.py index e9ab8a3655..6a58b7de06 100644 --- a/urllib3/connection.py +++ b/urllib3/connection.py @@ -301,11 +301,9 @@ def connect(self): ) # In case the hostname is an IPv6 address, strip the square - # brackets from it before using it to validate. This is because - # a certificate with an IPv6 address in it won't have square - # brackets around that address. Sadly, match_hostname won't do this - # for us: it expects the plain host part without any extra work - # that might have been done to make it palatable to httplib. + # brackets from it before using it to validate. Non IPv6 addresses + # should not start/end with square brackets, so this should be + # safe. asserted_hostname = self.assert_hostname or hostname asserted_hostname = asserted_hostname.strip('[]') match_hostname(cert, asserted_hostname) From becbfa501648604fbcb8352225fdaa47213f1fc6 Mon Sep 17 00:00:00 2001 From: Cory Benfield Date: Wed, 10 Feb 2016 09:50:43 +0000 Subject: [PATCH 3/6] Revert "Explain what the fix for #760 is doing." This reverts commit 16f0bd2ed8bb67dc76db42d4c691ecceffb4c443. --- urllib3/connection.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/urllib3/connection.py b/urllib3/connection.py index 6a58b7de06..4cd1513b8f 100644 --- a/urllib3/connection.py +++ b/urllib3/connection.py @@ -299,11 +299,6 @@ def connect(self): 'for details.)'.format(hostname)), SubjectAltNameWarning ) - - # In case the hostname is an IPv6 address, strip the square - # brackets from it before using it to validate. Non IPv6 addresses - # should not start/end with square brackets, so this should be - # safe. asserted_hostname = self.assert_hostname or hostname asserted_hostname = asserted_hostname.strip('[]') match_hostname(cert, asserted_hostname) From 95275687c805d8129ab4d10f6000410dd0303023 Mon Sep 17 00:00:00 2001 From: Cory Benfield Date: Wed, 10 Feb 2016 09:50:49 +0000 Subject: [PATCH 4/6] Revert "Strip square brackets from hostnames before verifying" This reverts commit cc2d86fc13688e573721ee7a7571cb5315ef2017. --- urllib3/connection.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/urllib3/connection.py b/urllib3/connection.py index 4cd1513b8f..f951fce230 100644 --- a/urllib3/connection.py +++ b/urllib3/connection.py @@ -299,9 +299,7 @@ def connect(self): 'for details.)'.format(hostname)), SubjectAltNameWarning ) - asserted_hostname = self.assert_hostname or hostname - asserted_hostname = asserted_hostname.strip('[]') - match_hostname(cert, asserted_hostname) + match_hostname(cert, self.assert_hostname or hostname) self.is_verified = (resolved_cert_reqs == ssl.CERT_REQUIRED or self.assert_fingerprint is not None) From 9d345c357c3e1f741fc5d1b240845d517ba15530 Mon Sep 17 00:00:00 2001 From: Cory Benfield Date: Wed, 10 Feb 2016 10:03:03 +0000 Subject: [PATCH 5/6] Defend against square bracket IPv6 problems. --- urllib3/connectionpool.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/urllib3/connectionpool.py b/urllib3/connectionpool.py index 6f6e9053cd..3a7a28efaa 100644 --- a/urllib3/connectionpool.py +++ b/urllib3/connectionpool.py @@ -70,6 +70,11 @@ def __init__(self, host, port=None): raise LocationValueError("No host specified.") # httplib doesn't like it when we include brackets in ipv6 addresses + # Specifically, if we include brackets but also pass the port then + # httplib crazily doubles up the square brackets on the Host header. + # Instead, we need to make sure we never pass ``None`` as the port. + # However, for backward compatibility reasons we can't actually + # *assert* that. self.host = host.strip('[]') self.port = port @@ -830,6 +835,7 @@ def connection_from_url(url, **kw): >>> r = conn.request('GET', '/') """ scheme, host, port = get_host(url) + port = port or port_by_scheme.get(scheme, 80) if scheme == 'https': return HTTPSConnectionPool(host, port=port, **kw) else: From 1966d2810248201038c2dcb1416450967aca875a Mon Sep 17 00:00:00 2001 From: Cory Benfield Date: Thu, 18 Feb 2016 11:24:55 +0000 Subject: [PATCH 6/6] Add test coverage for now-uncovered branch. --- test/test_connectionpool.py | 49 +++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/test/test_connectionpool.py b/test/test_connectionpool.py index ee37913d41..8fbf06b706 100644 --- a/test/test_connectionpool.py +++ b/test/test_connectionpool.py @@ -4,6 +4,7 @@ connection_from_url, HTTPConnection, HTTPConnectionPool, + HTTPSConnectionPool, ) from urllib3.util.timeout import Timeout from urllib3.packages.ssl_match_hostname import CertificateError @@ -74,6 +75,54 @@ def test_same_host(self): c = connection_from_url(b) self.assertFalse(c.is_same_host(a), "%s =? %s" % (b, a)) + def test_same_host_no_port(self): + # This test was introduced in #801 to deal with the fact that urllib3 + # never initializes ConnectionPool objects with port=None. + same_host_http = [ + ('google.com', '/'), + ('google.com', 'http://google.com/'), + ('google.com', 'http://google.com'), + ('google.com', 'http://google.com/abra/cadabra'), + # Test comparison using default ports + ('google.com', 'http://google.com:80/abracadabra'), + ] + same_host_https = [ + ('google.com', '/'), + ('google.com', 'https://google.com/'), + ('google.com', 'https://google.com'), + ('google.com', 'https://google.com/abra/cadabra'), + # Test comparison using default ports + ('google.com', 'https://google.com:443/abracadabra'), + ] + + for a, b in same_host_http: + c = HTTPConnectionPool(a) + self.assertTrue(c.is_same_host(b), "%s =? %s" % (a, b)) + for a, b in same_host_https: + c = HTTPSConnectionPool(a) + self.assertTrue(c.is_same_host(b), "%s =? %s" % (a, b)) + + not_same_host_http = [ + ('google.com', 'https://google.com/'), + ('yahoo.com', 'http://google.com/'), + ('google.com', 'https://google.net/'), + ] + not_same_host_https = [ + ('google.com', 'http://google.com/'), + ('yahoo.com', 'https://google.com/'), + ('google.com', 'https://google.net/'), + ] + + for a, b in not_same_host_http: + c = HTTPConnectionPool(a) + self.assertFalse(c.is_same_host(b), "%s =? %s" % (a, b)) + c = HTTPConnectionPool(b) + self.assertFalse(c.is_same_host(a), "%s =? %s" % (b, a)) + for a, b in not_same_host_https: + c = HTTPSConnectionPool(a) + self.assertFalse(c.is_same_host(b), "%s =? %s" % (a, b)) + c = HTTPSConnectionPool(b) + self.assertFalse(c.is_same_host(a), "%s =? %s" % (b, a)) def test_max_connections(self): pool = HTTPConnectionPool(host='localhost', maxsize=1, block=True)