Support the http.sslVerify and http.sslCAInfo configuration options.
authorJelmer Vernooij <jelmer@jelmer.uk>
Sat, 31 Mar 2018 15:26:56 +0000 (16:26 +0100)
committerJelmer Vernooij <jelmer@jelmer.uk>
Sat, 31 Mar 2018 15:26:56 +0000 (16:26 +0100)
Drop dependency on urllib3[secure]; just depend on certifi.

NEWS
dulwich/client.py
dulwich/tests/test_client.py
setup.py

diff --git a/NEWS b/NEWS
index 10f971ab3750a42f621b0e3c5a6398b651f6d91d..db0045c587b7fd0a994faadcfa8430e405333027 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -5,6 +5,13 @@
   * Add 'dulwich.mailmap' file for reading mailmap files.
     (Jelmer Vernooij)
 
+  * Dulwich no longer depends on urllib3[secure]. Instead,
+    "dulwich[https]" can be used to pull in the necessary
+    dependencies for HTTPS support. (Jelmer Vernooij, #616)
+
+  * Support the `http.sslVerify` and `http.sslCAInfo`
+    configuration options. (Jelmer Vernooij)
+
  API CHANGES
 
   * Index.iterblobs has been renamed to Index.iterobjects.
index f3ca88592df0aded5bcac2a5c54c3869e2d31659..cb865c6d274b14174ca4a2794e9fbf5eacfb77d5 100644 (file)
@@ -60,7 +60,6 @@ except ImportError:
     import urllib.request as urllib2
     import urllib.parse as urlparse
 
-import certifi
 import urllib3
 import urllib3.util
 
@@ -1247,17 +1246,18 @@ def default_user_agent_string():
     return "git/dulwich/%s" % ".".join([str(x) for x in dulwich.__version__])
 
 
-def default_urllib3_manager(config, verify_ssl=True):
+def default_urllib3_manager(config, **override_kwargs):
     """Return `urllib3` connection pool manager.
 
     Honour detected proxy configurations.
 
     :param config: `dulwich.config.ConfigDict` instance with Git configuration.
-    :param verify_ssl: Whether SSL verification is enabled.
+    :param kwargs: Additional arguments for urllib3.ProxyManager
     :return: `urllib3.ProxyManager` instance for proxy configurations,
         `urllib3.PoolManager` otherwise.
     """
     proxy_server = user_agent = None
+    ca_certs = ssl_verify = None
 
     if config is not None:
         try:
@@ -1269,24 +1269,53 @@ def default_urllib3_manager(config, verify_ssl=True):
         except KeyError:
             pass
 
-    ssl_kwargs = {}
-    if verify_ssl:
-        ssl_kwargs.update(cert_reqs="CERT_REQUIRED", ca_certs=certifi.where())
+        # TODO(jelmer): Support per-host settings
+        try:
+            ssl_verify = config.get_boolean(b"http", b"sslVerify")
+        except KeyError:
+            ssl_verify = True
+
+        try:
+            ca_certs = config.get_boolean(b"http", b"sslCAInfo")
+        except KeyError:
+            ca_certs = None
 
     if user_agent is None:
         user_agent = default_user_agent_string()
 
     headers = {"User-agent": user_agent}
 
+    kwargs = {}
+    if ssl_verify is True:
+        kwargs['cert_reqs'] = "CERT_REQUIRED"
+    elif ssl_verify is False:
+        kwargs['cert_reqs'] = 'CERT_NONE'
+    else:
+        # Default to SSL verification
+        kwargs['cert_reqs'] = "CERT_REQUIRED"
+
+    if ca_certs is not None:
+        kwargs['ca_certs'] = ca_certs
+    kwargs.update(override_kwargs)
+
+    # Try really hard to find a SSL certificate path
+    if 'ca_certs' not in kwargs and kwargs.get('cert_reqs') != 'CERT_NONE':
+        try:
+            import certifi
+        except ImportError:
+            pass
+        else:
+            kwargs['ca_certs'] = certifi.where()
+
     if proxy_server is not None:
         # `urllib3` requires a `str` object in both Python 2 and 3, while
         # `ConfigDict` coerces entries to `bytes` on Python 3. Compensate.
         if not isinstance(proxy_server, str):
             proxy_server = proxy_server.decode()
         manager = urllib3.ProxyManager(proxy_server, headers=headers,
-                                       **ssl_kwargs)
+                                       **kwargs)
     else:
-        manager = urllib3.PoolManager(headers=headers, **ssl_kwargs)
+        manager = urllib3.PoolManager(headers=headers, **kwargs)
 
     return manager
 
index eb1ccbeab5420dab40deb046e43016ddcb6754d5..7680bb4743d4ed8ca7d646c45ecc6c8040099485 100644 (file)
@@ -35,7 +35,6 @@ try:
 except ImportError:
     import urllib.parse as urlparse
 
-import certifi
 import urllib3
 
 import dulwich
@@ -961,19 +960,29 @@ class TCPGitClientTests(TestCase):
 
 class DefaultUrllib3ManagerTest(TestCase):
 
-    def assert_verify_ssl(self, manager, assertion=True):
-        pool_keywords = tuple(manager.connection_pool_kw.items())
-        assert_method = self.assertIn if assertion else self.assertNotIn
-        assert_method(('cert_reqs', 'CERT_REQUIRED'), pool_keywords)
-        assert_method(('ca_certs', certifi.where()), pool_keywords)
-
     def test_no_config(self):
         manager = default_urllib3_manager(config=None)
-        self.assert_verify_ssl(manager)
+        pool_keywords = tuple(manager.connection_pool_kw.items())
+        self.assertEqual(manager.connection_pool_kw['cert_reqs'],
+                         'CERT_REQUIRED')
 
     def test_config_no_proxy(self):
         manager = default_urllib3_manager(config=ConfigDict())
-        self.assert_verify_ssl(manager)
+        self.assertNotIsInstance(manager, urllib3.ProxyManager)
+
+    def test_config_ssl(self):
+        config = ConfigDict()
+        config.set(b'http', b'sslVerify', b'true')
+        manager = default_urllib3_manager(config=config)
+        self.assertEqual(manager.connection_pool_kw['cert_reqs'],
+                         'CERT_REQUIRED')
+
+    def test_config_no_ssl(self):
+        config = ConfigDict()
+        config.set(b'http', b'sslVerify', b'false')
+        manager = default_urllib3_manager(config=config)
+        self.assertEqual(manager.connection_pool_kw['cert_reqs'],
+                         'CERT_NONE')
 
     def test_config_proxy(self):
         config = ConfigDict()
@@ -985,11 +994,10 @@ class DefaultUrllib3ManagerTest(TestCase):
         self.assertEqual(manager.proxy.scheme, 'http')
         self.assertEqual(manager.proxy.host, 'localhost')
         self.assertEqual(manager.proxy.port, 3128)
-        self.assert_verify_ssl(manager)
 
     def test_config_no_verify_ssl(self):
-        manager = default_urllib3_manager(config=None, verify_ssl=False)
-        self.assert_verify_ssl(manager, assertion=False)
+        manager = default_urllib3_manager(config=None, cert_reqs="CERT_NONE")
+        self.assertEqual(manager.connection_pool_kw['cert_reqs'], 'CERT_NONE')
 
 
 class SubprocessSSHVendorTests(TestCase):
index b17cb8ee9cdbea779f5b2509097021ee494cd7d9..5b3f22a7a0a335cd5cc0527f1a7b14a65617bb7f 100755 (executable)
--- a/setup.py
+++ b/setup.py
@@ -78,7 +78,7 @@ if has_setuptools:
         'fastimport': ['fastimport'],
         'https': ['urllib3[secure]>=1.21'],
         }
-    setup_kwargs['install_requires'] = ['urllib3[secure]>=1.21']
+    setup_kwargs['install_requires'] = ['urllib3>=1.21', 'certifi']
     setup_kwargs['include_package_data'] = True
     setup_kwargs['test_suite'] = 'dulwich.tests.test_suite'
     setup_kwargs['tests_require'] = tests_require