Merge pull request #132 from pquentin/merge-from-master-2019-10-05

RatanShreshtha · web-flow · commit 2c915e38a2ba · 2019-10-18T16:17:43.000+05:30
Merge from master (October 5th, 2019)
diff --git a/_travis/install.sh b/_travis/install.sh
@@ -29,9 +29,6 @@ if [[ "$(uname -s)" == 'Darwin' ]]; then
 
     install_mac_python $MACPYTHON
 
-    # Enable TLS 1.3 on macOS
-    sudo defaults write /Library/Preferences/com.apple.networkd tcp_connect_enable_tls13 1
-
     # Install Nox
     python3 -m pip install nox
 
diff --git a/src/urllib3/contrib/_securetransport/bindings.py b/src/urllib3/contrib/_securetransport/bindings.py
@@ -415,6 +415,7 @@ class SecurityConst(object):
     kTLSProtocol1 = 4
     kTLSProtocol11 = 7
     kTLSProtocol12 = 8
+    # SecureTransport does not support TLS 1.3 even if there's a constant for it
     kTLSProtocol13 = 10
     kTLSProtocolMaxSupported = 999
 
diff --git a/src/urllib3/contrib/securetransport.py b/src/urllib3/contrib/securetransport.py
@@ -144,13 +144,10 @@
 ]
 
 # Basically this is simple: for PROTOCOL_SSLv23 we turn it into a low of
-# TLSv1 and a high of TLSv1.3. For everything else, we pin to that version.
-# TLSv1 to 1.2 are supported on macOS 10.8+ and TLSv1.3 is macOS 10.13+
+# TLSv1 and a high of TLSv1.2. For everything else, we pin to that version.
+# TLSv1 to 1.2 are supported on macOS 10.8+
 _protocol_to_min_max = {
-    util.PROTOCOL_TLS: (
-        SecurityConst.kTLSProtocol1,
-        SecurityConst.kTLSProtocolMaxSupported,
-    )
+    util.PROTOCOL_TLS: (SecurityConst.kTLSProtocol1, SecurityConst.kTLSProtocol12)
 }
 
 if hasattr(ssl, "PROTOCOL_SSLv2"):
@@ -488,15 +485,7 @@ def handshake(
         result = Security.SSLSetProtocolVersionMin(self.context, min_version)
         _assert_no_error(result)
 
-        # TLS 1.3 isn't necessarily enabled by the OS
-        # so we have to detect when we error out and try
-        # setting TLS 1.3 if it's allowed. kTLSProtocolMaxSupported
-        # was added in macOS 10.13 along with kTLSProtocol13.
         result = Security.SSLSetProtocolVersionMax(self.context, max_version)
-        if result != 0 and max_version == SecurityConst.kTLSProtocolMaxSupported:
-            result = Security.SSLSetProtocolVersionMax(
-                self.context, SecurityConst.kTLSProtocol12
-            )
         _assert_no_error(result)
 
         # If there's a trust DB, we need to use it. We do that by telling
@@ -707,7 +696,7 @@ def version(self):
         )
         _assert_no_error(result)
         if protocol.value == SecurityConst.kTLSProtocol13:
-            return "TLSv1.3"
+            raise ssl.SSLError("SecureTransport does not support TLS 1.3")
         elif protocol.value == SecurityConst.kTLSProtocol12:
             return "TLSv1.2"
         elif protocol.value == SecurityConst.kTLSProtocol11:
diff --git a/test/contrib/test_securetransport.py b/test/contrib/test_securetransport.py
@@ -31,9 +31,8 @@ def teardown_module():
         pass
 
 
-# Currently TLSv1.3 doesn't work with SecureTransport despite
-# Apple previously documenting support. See:
-# https://github.com/python-trio/trio/issues/1165#issuecomment-526563135
+# SecureTransport does not support TLSv1.3
+# https://github.com/urllib3/urllib3/issues/1674
 from ..with_dummyserver.test_https import (  # noqa: F401
     TestHTTPS,
     TestHTTPS_TLSv1,
diff --git a/test/test_connectionpool.py b/test/test_connectionpool.py
@@ -450,7 +450,6 @@ def __call__(self, *args, **kwargs):
 
         def _test(exception, *args):
             with HTTPConnectionPool(host="localhost", maxsize=1, block=True) as pool:
-
                 # Verify that the request succeeds after two attempts, and that the
                 # connection is left on the response object, instead of being
                 # released back into the pool.
diff --git a/test/test_no_ssl.py b/test/test_no_ssl.py
@@ -65,14 +65,14 @@ def pop(self):
 
 class TestWithoutSSL(object):
     @classmethod
-    def setup_class(self):
+    def setup_class(cls):
         sys.modules.pop("ssl", None)
         sys.modules.pop("_ssl", None)
 
         module_stash.stash()
         sys.meta_path.insert(0, ssl_blocker)
 
-    def teardown_class(self):
+    def teardown_class(cls):
         sys.meta_path.remove(ssl_blocker)
         module_stash.pop()
 
diff --git a/test/test_proxymanager.py b/test/test_proxymanager.py
@@ -7,7 +7,6 @@ class TestProxyManager(object):
     def test_proxy_headers(self):
         url = "http://pypi.org/project/urllib3/"
         with ProxyManager("http://something:1234") as p:
-
             # Verify default headers
             default_headers = {"Accept": "*/*", "Host": "pypi.org"}
             headers = p._set_proxy_headers(url)
diff --git a/test/with_dummyserver/test_connectionpool.py b/test/with_dummyserver/test_connectionpool.py
@@ -319,29 +319,29 @@ def test_socket_options(self):
         """Test that connections accept socket options."""
         # This test needs to be here in order to be run. socket.create_connection actually tries to
         # connect to the host provided so we need a dummyserver to be running.
-        pool = HTTPConnectionPool(
+        with HTTPConnectionPool(
             self.host,
             self.port,
             socket_options=[(socket.SOL_SOCKET, socket.SO_KEEPALIVE, 1)],
-        )
-        conn = pool._new_conn()
-        conn.connect()
-        s = conn._sock
-        using_keepalive = s.getsockopt(socket.SOL_SOCKET, socket.SO_KEEPALIVE) > 0
-        assert using_keepalive
-        s.close()
+        ) as pool:
+            conn = pool._new_conn()
+            conn.connect()
+            s = conn._sock
+            using_keepalive = s.getsockopt(socket.SOL_SOCKET, socket.SO_KEEPALIVE) > 0
+            assert using_keepalive
+            s.close()
 
     def test_disable_default_socket_options(self):
         """Test that passing None disables all socket options."""
         # This test needs to be here in order to be run. socket.create_connection actually tries
         # to connect to the host provided so we need a dummyserver to be running.
-        pool = HTTPConnectionPool(self.host, self.port, socket_options=None)
-        conn = pool._new_conn()
-        conn.connect()
-        s = conn._sock
-        using_nagle = s.getsockopt(socket.IPPROTO_TCP, socket.TCP_NODELAY) == 0
-        assert using_nagle
-        s.close()
+        with HTTPConnectionPool(self.host, self.port, socket_options=None) as pool:
+            conn = pool._new_conn()
+            conn.connect()
+            s = conn._sock
+            using_nagle = s.getsockopt(socket.IPPROTO_TCP, socket.TCP_NODELAY) == 0
+            assert using_nagle
+            s.close()
 
     def test_defaults_are_applied(self):
         """Test that modifying the default socket options works."""
@@ -372,12 +372,12 @@ def test_defaults_are_applied(self):
     def test_connection_error_retries(self):
         """ ECONNREFUSED error should raise a connection error, with retries """
         port = find_unused_port()
-        pool = HTTPConnectionPool(self.host, port)
-        try:
-            pool.request("GET", "/", retries=Retry(connect=3))
-            self.fail("Should have failed with a connection error.")
-        except MaxRetryError as e:
-            assert type(e.reason) == NewConnectionError
+        with HTTPConnectionPool(self.host, port) as pool:
+            try:
+                pool.request("GET", "/", retries=Retry(connect=3))
+                self.fail("Should have failed with a connection error.")
+            except MaxRetryError as e:
+                assert type(e.reason) == NewConnectionError
 
     def test_timeout_success(self):
         timeout = Timeout(connect=3, read=5, total=None)
@@ -395,12 +395,12 @@ def test_timeout_success(self):
             pool.request("GET", "/")
 
     def test_bad_connect(self):
-        pool = HTTPConnectionPool("badhost.invalid", self.port)
-        try:
-            pool.request("GET", "/", retries=5)
-            self.fail("should raise timeout exception here")
-        except MaxRetryError as e:
-            assert type(e.reason) == NewConnectionError
+        with HTTPConnectionPool("badhost.invalid", self.port) as pool:
+            try:
+                pool.request("GET", "/", retries=5)
+                self.fail("should raise timeout exception here")
+            except MaxRetryError as e:
+                assert type(e.reason) == NewConnectionError
 
     def test_keepalive(self):
         with HTTPConnectionPool(self.host, self.port, block=True, maxsize=1) as pool:
@@ -567,59 +567,58 @@ def test_lazy_load_twice(self):
         # This test is sad and confusing. Need to figure out what's
         # going on with partial reads and socket reuse.
 
-        pool = HTTPConnectionPool(
+        with HTTPConnectionPool(
             self.host, self.port, block=True, maxsize=1, timeout=2
-        )
-
-        payload_size = 1024 * 2
-        first_chunk = 512
+        ) as pool:
+            payload_size = 1024 * 2
+            first_chunk = 512
 
-        boundary = "foo"
+            boundary = "foo"
 
-        req_data = {"count": "a" * payload_size}
-        resp_data = encode_multipart_formdata(req_data, boundary=boundary)[0]
+            req_data = {"count": "a" * payload_size}
+            resp_data = encode_multipart_formdata(req_data, boundary=boundary)[0]
 
-        req2_data = {"count": "b" * payload_size}
-        resp2_data = encode_multipart_formdata(req2_data, boundary=boundary)[0]
+            req2_data = {"count": "b" * payload_size}
+            resp2_data = encode_multipart_formdata(req2_data, boundary=boundary)[0]
 
-        r1 = pool.request(
-            "POST",
-            "/echo",
-            fields=req_data,
-            multipart_boundary=boundary,
-            preload_content=False,
-        )
-
-        first_data = r1.read(first_chunk)
-        assert len(first_data) > 0
-        assert first_data == resp_data[: len(first_data)]
-
-        try:
-            r2 = pool.request(
+            r1 = pool.request(
                 "POST",
                 "/echo",
-                fields=req2_data,
+                fields=req_data,
                 multipart_boundary=boundary,
                 preload_content=False,
-                pool_timeout=0.001,
             )
 
-            # This branch should generally bail here, but maybe someday it will
-            # work? Perhaps by some sort of magic. Consider it a TODO.
+            first_data = r1.read(first_chunk)
+            assert len(first_data) > 0
+            assert first_data == resp_data[: len(first_data)]
+
+            try:
+                r2 = pool.request(
+                    "POST",
+                    "/echo",
+                    fields=req2_data,
+                    multipart_boundary=boundary,
+                    preload_content=False,
+                    pool_timeout=0.001,
+                )
 
-            second_data = r2.read(first_chunk)
-            assert len(second_data) > 0
-            assert second_data == resp2_data[: len(second_data)]
+                # This branch should generally bail here, but maybe someday it will
+                # work? Perhaps by some sort of magic. Consider it a TODO.
 
-            assert r1.read() == resp_data[len(first_data) :]
-            assert r2.read() == resp2_data[len(second_data) :]
-            assert pool.num_requests == 2
+                second_data = r2.read(first_chunk)
+                assert len(second_data) > 0
+                assert second_data == resp2_data[: len(second_data)]
 
-        except EmptyPoolError:
-            assert r1.read() == resp_data[len(first_data) :]
-            assert pool.num_requests == 1
+                assert r1.read() == resp_data[len(first_data) :]
+                assert r2.read() == resp2_data[len(second_data) :]
+                assert pool.num_requests == 2
 
-        assert pool.num_connections == 1
+            except EmptyPoolError:
+                assert r1.read() == resp_data[len(first_data) :]
+                assert pool.num_requests == 1
+
+            assert pool.num_connections == 1
 
     def test_for_double_release(self):
         MAXSIZE = 5
@@ -653,11 +652,11 @@ def test_for_double_release(self):
 
     def test_connections_arent_released(self):
         MAXSIZE = 5
-        pool = HTTPConnectionPool(self.host, self.port, maxsize=MAXSIZE)
-        assert pool.pool.qsize() == MAXSIZE
+        with HTTPConnectionPool(self.host, self.port, maxsize=MAXSIZE) as pool:
+            assert pool.pool.qsize() == MAXSIZE
 
-        pool.request("GET", "/", preload_content=False)
-        assert pool.pool.qsize() == MAXSIZE - 1
+            pool.request("GET", "/", preload_content=False)
+            assert pool.pool.qsize() == MAXSIZE - 1
 
     def test_dns_error(self):
         pool = HTTPConnectionPool(
@@ -684,11 +683,11 @@ def test_source_address(self):
 
     def test_source_address_error(self):
         for addr in INVALID_SOURCE_ADDRESSES:
-            pool = HTTPConnectionPool(
+            with HTTPConnectionPool(
                 self.host, self.port, source_address=addr, retries=False
-            )
-            with pytest.raises(NewConnectionError):
-                pool.request("GET", "/source_address?{0}".format(addr))
+            ) as pool:
+                with pytest.raises(NewConnectionError):
+                    pool.request("GET", "/source_address?{0}".format(addr))
 
     def test_stream_keepalive(self):
         x = 2
diff --git a/test/with_dummyserver/test_https.py b/test/with_dummyserver/test_https.py
diff --git a/test/with_dummyserver/test_poolmanager.py b/test/with_dummyserver/test_poolmanager.py
diff --git a/test/with_dummyserver/test_proxy_poolmanager.py b/test/with_dummyserver/test_proxy_poolmanager.py
diff --git a/test/with_dummyserver/test_socketlevel.py b/test/with_dummyserver/test_socketlevel.py
