Rev 6463: Feedback from mgz. in file:///home/vila/src/bzr/bugs/920455-ssl-defaults/

[Vincent Ladeuil]

At file:///home/vila/src/bzr/bugs/920455-ssl-defaults/

------------------------------------------------------------
revno: 6463
revision-id: v.ladeuil+lp at free.fr-20120131163653-j5z45vjfx8a6h2d0
parent: v.ladeuil+lp at free.fr-20120131142358-6rntqjsvni2cqjuf
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: 920455-ssl-defaults
timestamp: Tue 2012-01-31 17:36:53 +0100
message:
  Feedback from mgz.
-------------- next part --------------
=== modified file 'bzrlib/errors.py'
--- a/bzrlib/errors.py	2012-01-20 13:21:01 +0000
+++ b/bzrlib/errors.py	2012-01-31 16:36:53 +0000
@@ -1756,7 +1756,8 @@
 
 class ConfigOptionValueError(BzrError):
 
-    _fmt = """Bad value "%(value)s" for option "%(name)s"."""
+    _fmt = ('Bad value "%(value)s" for option "%(name)s".\n'
+            'See ``bzr help %(name)s``')
 
     def __init__(self, name, value):
         BzrError.__init__(self, name=name, value=value)

=== modified file 'bzrlib/tests/test_https_urllib.py'
--- a/bzrlib/tests/test_https_urllib.py	2012-01-31 11:39:29 +0000
+++ b/bzrlib/tests/test_https_urllib.py	2012-01-31 16:36:53 +0000
@@ -53,9 +53,18 @@
         self.assertEquals(path, stack.get('ssl.ca_certs'))
 
     def test_specified_doesnt_exist(self):
-        path = os.path.join(self.test_dir, "nonexisting.pem")
-        stack = self.get_stack("ssl.ca_certs = %s\n" % path)
-        self.assertRaises(ConfigOptionValueError, stack.get, 'ssl.ca_certs')
+        stack = self.get_stack('')
+        # Disable the default value mechanism to force the behavior we want
+        self.overrideAttr(_urllib2_wrappers.opt_ssl_ca_certs, 'default',
+                          os.path.join(self.test_dir, u"nonexisting.pem"))
+        self.warnings = []
+        def warning(*args):
+            self.warnings.append(args[0] % args[1:])
+        self.overrideAttr(trace, 'warning', warning)
+        self.assertEquals(None, stack.get('ssl.ca_certs'))
+        self.assertLength(1, self.warnings)
+        self.assertContainsRe(self.warnings[0],
+                              "is not valid for \"ssl.ca_certs\"")
 
 
 class CertReqsConfigTests(TestCaseInTempDir):

=== modified file 'bzrlib/transport/http/_urllib2_wrappers.py'
--- a/bzrlib/transport/http/_urllib2_wrappers.py	2012-01-31 14:23:58 +0000
+++ b/bzrlib/transport/http/_urllib2_wrappers.py	2012-01-31 16:36:53 +0000
@@ -126,9 +126,14 @@
 opt_ssl_ca_certs = config.Option('ssl.ca_certs',
         from_unicode=ca_certs_from_store,
         default=default_ca_certs,
-        invalid='error',
+        invalid='warning',
         help="""\
 Path to certification authority certificates to trust.
+
+This should be a valid path to a bundle containing all root Certificate
+Authorities used to verify an https server certificate.
+
+Use ssl.cert_reqs=none to disable certificate verification.
 """)
 
 opt_ssl_cert_reqs = config.Option('ssl.cert_reqs',
@@ -466,7 +471,7 @@
         config_stack = config.GlobalStack()
         cert_reqs = config_stack.get('ssl.cert_reqs')
         if cert_reqs == ssl.CERT_NONE:
-            trace.warning("not checking SSL certificates for %s: %d",
+            trace.warning("Not checking SSL certificate for %s: %d",
                 self.host, self.port)
             ca_certs = None
         else:
@@ -476,19 +481,18 @@
                 ca_certs = self.ca_certs
             if ca_certs is None:
                 trace.warning(
-                    "no valid trusted SSL CA certificates file set. See "
+                    "No valid trusted SSL CA certificates file set. See "
                     "'bzr help ssl.ca_certs' for more information on setting "
-                    "trusted CA's.")
+                    "trusted CAs.")
         try:
             ssl_sock = ssl.wrap_socket(self.sock, self.key_file, self.cert_file,
                 cert_reqs=cert_reqs, ca_certs=ca_certs)
         except ssl.SSLError, e:
-            if e.errno != ssl.SSL_ERROR_SSL:
-                raise
-            trace.note(
-                "To disable SSL certificate verification, use "
-                "-Ossl.cert_reqs=none. See ``bzr help ssl.ca_certs`` for "
-                "more information on specifying trusted CA certificates.")
+            if e.errno == ssl.SSL_ERROR_SSL:
+                trace.note(
+                    "To disable SSL certificate verification, use "
+                    "-Ossl.cert_reqs=none. See ``bzr help ssl.ca_certs`` for "
+                    "more information on specifying trusted CAs.")
             raise
         if cert_reqs == ssl.CERT_REQUIRED:
             peer_cert = ssl_sock.getpeercert()