Merge pull request #472 from sfackler/alpn-test

Fix test_alpn_server_select_none

3 files changed, 100 insertions, 54 deletions

diff --git a/openssl-sys/src/lib.rs b/openssl-sys/src/lib.rs
index 300ed056..026d6e6b 100644
--- a/openssl-sys/src/lib.rs
+++ b/openssl-sys/src/lib.rs
@@ -318,6 +318,18 @@ pub unsafe fn SSL_set_tlsext_host_name(s: *mut SSL, name: *mut c_char) -> c_long
              name as *mut c_void)
 }
 
+pub fn ERR_GET_LIB(l: c_ulong) -> c_int {
+    ((l >> 24) & 0x0FF) as c_int
+}
+
+pub fn ERR_GET_FUNC(l: c_ulong) -> c_int {
+    ((l >> 12) & 0xFFF) as c_int
+}
+
+pub fn ERR_GET_REASON(l: c_ulong) -> c_int {
+    (l & 0xFFF) as c_int
+}
+
 extern {
     pub fn ASN1_INTEGER_set(dest: *mut ASN1_INTEGER, value: c_long) -> c_int;
     pub fn ASN1_STRING_type_new(ty: c_int) -> *mut ASN1_STRING;
diff --git a/openssl/src/error.rs b/openssl/src/error.rs
index f54d7bda..4dd219af 100644
--- a/openssl/src/error.rs
+++ b/openssl/src/error.rs
@@ -76,39 +76,79 @@ impl Error {
     }
 
     /// Returns the raw OpenSSL error code for this error.
-    pub fn error_code(&self) -> c_ulong {
+    pub fn code(&self) -> c_ulong {
         self.0
     }
 
-    /// Returns the name of the library reporting the error.
-    pub fn library(&self) -> &'static str {
-        get_lib(self.0)
+    /// Returns the name of the library reporting the error, if available.
+    pub fn library(&self) -> Option<&'static str> {
+        unsafe {
+            let cstr = ffi::ERR_lib_error_string(self.0);
+            if cstr.is_null() {
+                return None;
+            }
+            let bytes = CStr::from_ptr(cstr as *const _).to_bytes();
+            Some(str::from_utf8(bytes).unwrap())
+        }
     }
 
     /// Returns the name of the function reporting the error.
-    pub fn function(&self) -> &'static str {
-        get_func(self.0)
+    pub fn function(&self) -> Option<&'static str> {
+        unsafe {
+            let cstr = ffi::ERR_func_error_string(self.0);
+            if cstr.is_null() {
+                return None;
+            }
+            let bytes = CStr::from_ptr(cstr as *const _).to_bytes();
+            Some(str::from_utf8(bytes).unwrap())
+        }
     }
 
     /// Returns the reason for the error.
-    pub fn reason(&self) -> &'static str {
-        get_reason(self.0)
+    pub fn reason(&self) -> Option<&'static str> {
+        unsafe {
+            let cstr = ffi::ERR_reason_error_string(self.0);
+            if cstr.is_null() {
+                return None;
+            }
+            let bytes = CStr::from_ptr(cstr as *const _).to_bytes();
+            Some(str::from_utf8(bytes).unwrap())
+        }
     }
 }
 
 impl fmt::Debug for Error {
     fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
-        fmt.debug_struct("Error")
-           .field("library", &self.library())
-           .field("function", &self.function())
-           .field("reason", &self.reason())
-           .finish()
+        let mut builder = fmt.debug_struct("Error");
+        builder.field("code", &self.code());
+        if let Some(library) = self.library() {
+            builder.field("library", &library);
+        }
+        if let Some(function) = self.function() {
+            builder.field("function", &function);
+        }
+        if let Some(reason) = self.reason() {
+            builder.field("reason", &reason);
+        }
+        builder.finish()
     }
 }
 
 impl fmt::Display for Error {
     fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
-        fmt.write_str(&self.reason())
+        try!(write!(fmt, "error:{:08X}", self.0));
+        match self.library() {
+            Some(l) => try!(write!(fmt, ":{}", l)),
+            None => try!(write!(fmt, ":lib({})", ffi::ERR_GET_LIB(self.0))),
+        }
+        match self.function() {
+            Some(f) => try!(write!(fmt, ":{}", f)),
+            None => try!(write!(fmt, ":func({})", ffi::ERR_GET_FUNC(self.0))),
+        }
+        match self.reason() {
+            Some(r) => write!(fmt, ":{}", r),
+            None => write!(fmt, ":reason({})", ffi::ERR_GET_FUNC(self.0)),
+        }
     }
 }
 
@@ -117,31 +157,3 @@ impl error::Error for Error {
         "An OpenSSL error"
     }
 }
-
-fn get_lib(err: c_ulong) -> &'static str {
-    unsafe {
-        let cstr = ffi::ERR_lib_error_string(err);
-        assert!(!cstr.is_null(), "bad lib: {}", err);
-        let bytes = CStr::from_ptr(cstr as *const _).to_bytes();
-        str::from_utf8(bytes).unwrap()
-    }
-}
-
-fn get_func(err: c_ulong) -> &'static str {
-    unsafe {
-        let cstr = ffi::ERR_func_error_string(err);
-        assert!(!cstr.is_null(), "bad func: {}", err);
-        let bytes = CStr::from_ptr(cstr as *const _).to_bytes();
-        str::from_utf8(bytes).unwrap()
-    }
-}
-
-fn get_reason(err: c_ulong) -> &'static str {
-    unsafe {
-        let cstr = ffi::ERR_reason_error_string(err);
-        assert!(!cstr.is_null(), "bad reason: {}", err);
-        let bytes = CStr::from_ptr(cstr as *const _).to_bytes();
-        str::from_utf8(bytes).unwrap()
-    }
-}
-
diff --git a/openssl/src/ssl/tests/mod.rs b/openssl/src/ssl/tests/mod.rs
index b3500105..ce1ba8ca 100644
--- a/openssl/src/ssl/tests/mod.rs
+++ b/openssl/src/ssl/tests/mod.rs
@@ -726,10 +726,7 @@ fn test_alpn_server_advertise_multiple() {
 /// Test that Servers supporting ALPN don't report a protocol when none of their protocols match
 /// the client's reported protocol.
 #[test]
-#[cfg(feature = "openssl-102")]
-// TODO: not sure why this test is failing on OpenSSL 1.1.0, may be related to
-//       something about SSLv3 though?
-#[cfg_attr(ossl110, ignore)]
+#[cfg(all(feature = "openssl-102", ossl102))]
 fn test_alpn_server_select_none() {
     let listener = TcpListener::bind("127.0.0.1:0").unwrap();
     let localhost = listener.local_addr().unwrap();
@@ -753,21 +750,46 @@ fn test_alpn_server_select_none() {
     let mut ctx = SslContext::new(Tls).unwrap();
     ctx.set_verify(SSL_VERIFY_PEER);
     ctx.set_alpn_protocols(&[b"http/2"]);
-    match ctx.set_CA_file(&Path::new("test/root-ca.pem")) {
-        Ok(_) => {}
-        Err(err) => panic!("Unexpected error {:?}", err),
-    }
+    ctx.set_CA_file(&Path::new("test/root-ca.pem")).unwrap();
     // Now connect to the socket and make sure the protocol negotiation works...
     let stream = TcpStream::connect(localhost).unwrap();
-    let stream = match SslStream::connect(&ctx, stream) {
-        Ok(stream) => stream,
-        Err(err) => panic!("Expected success, got {:?}", err),
-    };
+    let stream = SslStream::connect(&ctx, stream).unwrap();
 
     // Since the protocols from the server and client don't overlap at all, no protocol is selected
     assert_eq!(None, stream.ssl().selected_alpn_protocol());
 }
 
+// In 1.1.0, ALPN negotiation failure is a fatal error
+#[test]
+#[cfg(all(feature = "openssl-102", ossl110))]
+fn test_alpn_server_select_none() {
+    let listener = TcpListener::bind("127.0.0.1:0").unwrap();
+    let localhost = listener.local_addr().unwrap();
+    // We create a different context instance for the server...
+    let listener_ctx = {
+        let mut ctx = SslContext::new(Tls).unwrap();
+        ctx.set_verify(SSL_VERIFY_PEER);
+        ctx.set_alpn_protocols(&[b"http/1.1", b"spdy/3.1"]);
+        assert!(ctx.set_certificate_file(&Path::new("test/cert.pem"), X509FileType::PEM)
+                   .is_ok());
+        ctx.set_private_key_file(&Path::new("test/key.pem"), X509FileType::PEM)
+            .unwrap();
+        ctx
+    };
+    // Have the listener wait on the connection in a different thread.
+    thread::spawn(move || {
+        let (stream, _) = listener.accept().unwrap();
+        assert!(SslStream::accept(&listener_ctx, stream).is_err());
+    });
+
+    let mut ctx = SslContext::new(Tls).unwrap();
+    ctx.set_verify(SSL_VERIFY_PEER);
+    ctx.set_alpn_protocols(&[b"http/2"]);
+    ctx.set_CA_file(&Path::new("test/root-ca.pem")).unwrap();
+    // Now connect to the socket and make sure the protocol negotiation works...
+    let stream = TcpStream::connect(localhost).unwrap();
+    assert!(SslStream::connect(&ctx, stream).is_err());
+}
 
 #[cfg(test)]
 mod dtlsv1 {