Patchwork [03,of,11] sslutil: move code examining _canloaddefaultcerts out of _defaultcacerts

login
register
mail settings
Submitter Gregory Szorc
Date May 5, 2016, 7:53 a.m.
Message ID <2864851cadedd9fb9603.1462434800@ubuntu-vm-main>
Download mbox | patch
Permalink /patch/14895/
State Accepted
Headers show

Comments

Gregory Szorc - May 5, 2016, 7:53 a.m.
# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 1462430314 25200
#      Wed May 04 23:38:34 2016 -0700
# Node ID 2864851cadedd9fb960368d408ce6fce039c78a8
# Parent  c681048bf0680635752b51e34c6be45e19e9192b
sslutil: move code examining _canloaddefaultcerts out of _defaultcacerts

Before, the return of _defaultcacerts() was 1 of 3 types. This was
difficult to read. Make it return a path or None.

We had to update hghave.py in the same patch because it was also
looking at this internal function. I wasted dozens of minutes
trying to figure out why tests were failing until I found the
code in hghave.py...

Patch

diff --git a/mercurial/sslutil.py b/mercurial/sslutil.py
--- a/mercurial/sslutil.py
+++ b/mercurial/sslutil.py
@@ -217,24 +217,23 @@  def _plainapplepython():
     """
     if sys.platform != 'darwin' or util.mainfrozen() or not sys.executable:
         return False
     exe = os.path.realpath(sys.executable).lower()
     return (exe.startswith('/usr/bin/python') or
             exe.startswith('/system/library/frameworks/python.framework/'))
 
 def _defaultcacerts():
-    """return path to CA certificates; None for system's store; ! to disable"""
+    """return path to default CA certificates or None."""
     if _plainapplepython():
         dummycert = os.path.join(os.path.dirname(__file__), 'dummycert.pem')
         if os.path.exists(dummycert):
             return dummycert
-    if _canloaddefaultcerts:
-        return None
-    return '!'
+
+    return None
 
 def sslkwargs(ui, host):
     """Determine arguments to pass to wrapsocket().
 
     ``host`` is the hostname being connected to.
     """
     kws = {'ui': ui}
 
@@ -257,18 +256,22 @@  def sslkwargs(ui, host):
             raise error.Abort(_('could not find web.cacerts: %s') % cacerts)
 
         kws.update({'ca_certs': cacerts,
                     'cert_reqs': ssl.CERT_REQUIRED})
         return kws
 
     # No CAs in config. See if we can load defaults.
     cacerts = _defaultcacerts()
-    if cacerts and cacerts != '!':
+    if cacerts:
         ui.debug('using %s to enable OS X system CA\n' % cacerts)
+    else:
+        if not _canloaddefaultcerts:
+            cacerts = '!'
+
     ui.setconfig('web', 'cacerts', cacerts, 'defaultcacerts')
 
     if cacerts != '!':
         kws.update({'ca_certs': cacerts,
                     'cert_reqs': ssl.CERT_REQUIRED,
                     })
     return kws
 
diff --git a/tests/hghave.py b/tests/hghave.py
--- a/tests/hghave.py
+++ b/tests/hghave.py
@@ -411,17 +411,17 @@  def has_sslcontext():
         ssl.SSLContext
         return True
     except (ImportError, AttributeError):
         return False
 
 @check("defaultcacerts", "can verify SSL certs by system's CA certs store")
 def has_defaultcacerts():
     from mercurial import sslutil
-    return sslutil._defaultcacerts() != '!'
+    return sslutil._defaultcacerts() or sslutil._canloaddefaultcerts
 
 @check("windows", "Windows")
 def has_windows():
     return os.name == 'nt'
 
 @check("system-sh", "system() uses sh")
 def has_system_sh():
     return os.name != 'nt'