Patchwork [1,of,6] sslutil: remove ui from sslkwargs (API)

login
register
mail settings
Submitter Gregory Szorc
Date May 26, 2016, 3:03 a.m.
Message ID <62c63aafbff472307fcf.1464231830@ubuntu-vm-main>
Download mbox | patch
Permalink /patch/15212/
State Accepted
Headers show

Comments

Gregory Szorc - May 26, 2016, 3:03 a.m.
# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 1464230602 25200
#      Wed May 25 19:43:22 2016 -0700
# Node ID 62c63aafbff472307fcf4624d63d691f769b4fb2
# Parent  ee935a6e1ea2894c16e866bca25591aa93fb566d
sslutil: remove ui from sslkwargs (API)

Arguments to sslutil.wrapsocket() are partially determined by
calling sslutil.sslkwargs(). This function receives a ui and
a hostname and determines what settings, if any, need to be
applied when the socket is wrapped.

Both the ui and hostname are passed into wrapsocket(). The
other arguments to wrapsocket() provided by sslkwargs() (ca_certs
and cert_reqs) are not looked at or modified anywhere outside
of sslutil.py. So, sslkwargs() doesn't need to exist as a
separate public API called before wrapsocket().

This commit starts the process of removing external consumers of
sslkwargs() by removing the "ui" key/argument from its return.
All callers now pass the ui argument explicitly.
Yuya Nishihara - May 26, 2016, 2:03 p.m.
On Wed, 25 May 2016 20:03:50 -0700, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc@gmail.com>
> # Date 1464230602 25200
p> #      Wed May 25 19:43:22 2016 -0700
> # Node ID 62c63aafbff472307fcf4624d63d691f769b4fb2
> # Parent  ee935a6e1ea2894c16e866bca25591aa93fb566d
> sslutil: remove ui from sslkwargs (API)

The series looks great, queued with a minor tweak. Thanks.

> --- a/mercurial/mail.py
>      if (starttls or smtps) and verifycert:
>          sslkwargs = sslutil.sslkwargs(ui, mailhost)
> -    else:
> -        # 'ui' is required by sslutil.wrapsocket() and set by sslkwargs()
> -        sslkwargs = {'ui': ui}

This would introduce a temporary name error. I've fixed in flight, but anyway
the end result is the same.

Patch

diff --git a/mercurial/httpconnection.py b/mercurial/httpconnection.py
--- a/mercurial/httpconnection.py
+++ b/mercurial/httpconnection.py
@@ -280,10 +280,11 @@  class http2handler(urlreq.httphandler, u
         kwargs['keyfile'] = keyfile
         kwargs['certfile'] = certfile
 
         kwargs.update(sslutil.sslkwargs(self.ui, host))
 
         con = HTTPConnection(host, port, use_ssl=True,
                              ssl_wrap_socket=sslutil.wrapsocket,
                              ssl_validator=sslutil.validatesocket,
+                             ui=self.ui,
                              **kwargs)
         return con
diff --git a/mercurial/mail.py b/mercurial/mail.py
--- a/mercurial/mail.py
+++ b/mercurial/mail.py
@@ -43,57 +43,61 @@  def _unifiedheaderinit(self, *args, **kw
 
 setattr(email.header.Header, '__init__', _unifiedheaderinit)
 
 class STARTTLS(smtplib.SMTP):
     '''Derived class to verify the peer certificate for STARTTLS.
 
     This class allows to pass any keyword arguments to SSL socket creation.
     '''
-    def __init__(self, sslkwargs, host=None, **kwargs):
+    def __init__(self, ui, sslkwargs, host=None, **kwargs):
         smtplib.SMTP.__init__(self, **kwargs)
+        self._ui = ui
         self._sslkwargs = sslkwargs
         self._host = host
 
     def starttls(self, keyfile=None, certfile=None):
         if not self.has_extn("starttls"):
             msg = "STARTTLS extension not supported by server"
             raise smtplib.SMTPException(msg)
         (resp, reply) = self.docmd("STARTTLS")
         if resp == 220:
             self.sock = sslutil.wrapsocket(self.sock, keyfile, certfile,
+                                           ui=self._ui,
                                            serverhostname=self._host,
                                            **self._sslkwargs)
             self.file = smtplib.SSLFakeFile(self.sock)
             self.helo_resp = None
             self.ehlo_resp = None
             self.esmtp_features = {}
             self.does_esmtp = 0
         return (resp, reply)
 
 class SMTPS(smtplib.SMTP):
     '''Derived class to verify the peer certificate for SMTPS.
 
     This class allows to pass any keyword arguments to SSL socket creation.
     '''
-    def __init__(self, sslkwargs, keyfile=None, certfile=None, host=None,
+    def __init__(self, ui, sslkwargs, keyfile=None, certfile=None, host=None,
                  **kwargs):
         self.keyfile = keyfile
         self.certfile = certfile
         smtplib.SMTP.__init__(self, **kwargs)
         self._host = host
         self.default_port = smtplib.SMTP_SSL_PORT
+        self._ui = ui
         self._sslkwargs = sslkwargs
 
     def _get_socket(self, host, port, timeout):
         if self.debuglevel > 0:
             print('connect:', (host, port), file=sys.stderr)
         new_socket = socket.create_connection((host, port), timeout)
         new_socket = sslutil.wrapsocket(new_socket,
                                         self.keyfile, self.certfile,
+                                        ui=self._ui,
                                         serverhostname=self._host,
                                         **self._sslkwargs)
         self.file = smtplib.SSLFakeFile(new_socket)
         return new_socket
 
 def _smtp(ui):
     '''build an smtp connection and return a function to send mail'''
     local_hostname = ui.config('smtp', 'local_hostname')
@@ -109,24 +113,23 @@  def _smtp(ui):
     verifycert = ui.config('smtp', 'verifycert', 'strict')
     if verifycert not in ['strict', 'loose']:
         if util.parsebool(verifycert) is not False:
             raise error.Abort(_('invalid smtp.verifycert configuration: %s')
                              % (verifycert))
         verifycert = False
     if (starttls or smtps) and verifycert:
         sslkwargs = sslutil.sslkwargs(ui, mailhost)
-    else:
-        # 'ui' is required by sslutil.wrapsocket() and set by sslkwargs()
-        sslkwargs = {'ui': ui}
+
     if smtps:
         ui.note(_('(using smtps)\n'))
-        s = SMTPS(sslkwargs, local_hostname=local_hostname, host=mailhost)
+        s = SMTPS(ui, sslkwargs, local_hostname=local_hostname, host=mailhost)
     elif starttls:
-        s = STARTTLS(sslkwargs, local_hostname=local_hostname, host=mailhost)
+        s = STARTTLS(ui, sslkwargs, local_hostname=local_hostname,
+                     host=mailhost)
     else:
         s = smtplib.SMTP(local_hostname=local_hostname)
     if smtps:
         defaultport = 465
     else:
         defaultport = 25
     mailport = util.getport(ui.config('smtp', 'port', defaultport))
     ui.note(_('sending mail: smtp host %s, port %d\n') %
diff --git a/mercurial/sslutil.py b/mercurial/sslutil.py
--- a/mercurial/sslutil.py
+++ b/mercurial/sslutil.py
@@ -242,17 +242,17 @@  def _defaultcacerts():
 
     return None
 
 def sslkwargs(ui, host):
     """Determine arguments to pass to wrapsocket().
 
     ``host`` is the hostname being connected to.
     """
-    kws = {'ui': ui}
+    kws = {}
 
     # If a host key fingerprint is on file, it is the only thing that matters
     # and CA certs don't come into play.
     hostfingerprint = ui.config('hostfingerprints', host)
     if hostfingerprint:
         return kws
 
     # The code below sets up CA verification arguments. If --insecure is
diff --git a/mercurial/url.py b/mercurial/url.py
--- a/mercurial/url.py
+++ b/mercurial/url.py
@@ -349,18 +349,18 @@  if has_https:
         def connect(self):
             self.sock = _create_connection((self.host, self.port))
 
             host = self.host
             if self.realhostport: # use CONNECT proxy
                 _generic_proxytunnel(self)
                 host = self.realhostport.rsplit(':', 1)[0]
             self.sock = sslutil.wrapsocket(
-                self.sock, self.key_file, self.cert_file, serverhostname=host,
-                **sslutil.sslkwargs(self.ui, host))
+                self.sock, self.key_file, self.cert_file, ui=self.ui,
+                serverhostname=host, **sslutil.sslkwargs(self.ui, host))
             sslutil.validatesocket(self.sock)
 
     class httpshandler(keepalive.KeepAliveHandler, urlreq.httpshandler):
         def __init__(self, ui):
             keepalive.KeepAliveHandler.__init__(self)
             urlreq.httpshandler.__init__(self)
             self.ui = ui
             self.pwmgr = passwordmgr(self.ui)