Patchwork [6,of,6] sslutil: remove redundant check of sslsocket.cipher()

login
register
mail settings
Submitter Gregory Szorc
Date May 15, 2016, 6:57 p.m.
Message ID <7a3aca333baaefd04278.1463338643@ubuntu-vm-main>
Download mbox | patch
Permalink /patch/15123/
State Superseded
Headers show

Comments

Gregory Szorc - May 15, 2016, 6:57 p.m.
# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 1463338249 25200
#      Sun May 15 11:50:49 2016 -0700
# Node ID 7a3aca333baaefd042787a02a5fd4326cc916306
# Parent  21acf5e61add50991012edaa707441548a83986c
sslutil: remove redundant check of sslsocket.cipher()

We are doing this check in both wrapsocket() and validatesocket().

The check was added to the validator in 4bb59919c905 and the commit
message justifies the redundancy with a "might." The check in
wrapsocket() was added in 0cc4ad757c77, which appears to be part of
the same series. I'm going to argue the redundancy isn't needed.

I choose to keep the check in wrapsocket() because it is working
around a bug in Python's wrap_socket() and I feel the check for
the bug should live next to the function call exhibiting the bug.

Patch

diff --git a/mercurial/sslutil.py b/mercurial/sslutil.py
--- a/mercurial/sslutil.py
+++ b/mercurial/sslutil.py
@@ -294,18 +294,16 @@  def sslkwargs(ui, host):
 def validatesocket(sock, strict=False):
     """Validate a socket meets security requiremnets.
 
     The passed socket must have been created with ``wrapsocket()``.
     """
     host = sock._hgstate['hostname']
     ui = sock._hgstate['ui']
 
-    if not sock.cipher(): # work around http://bugs.python.org/issue13721
-        raise error.Abort(_('%s ssl connection error') % host)
     try:
         peercert = sock.getpeercert(True)
         peercert2 = sock.getpeercert()
     except AttributeError:
         raise error.Abort(_('%s ssl connection error') % host)
 
     if not peercert:
         raise error.Abort(_('%s certificate error: '