Merge branch 'release-v0.7.4' into releasev0.7.4

5 files changed, 43 insertions, 49 deletions

diff --git a/openssl/Cargo.toml b/openssl/Cargo.toml
index 30121fe2..9c56623e 100644
--- a/openssl/Cargo.toml
+++ b/openssl/Cargo.toml
@@ -1,11 +1,11 @@
 [package]
 name = "openssl"
-version = "0.7.3"
+version = "0.7.4"
 authors = ["Steven Fackler <[email protected]>"]
 license = "Apache-2.0"
 description = "OpenSSL bindings"
 repository = "https://github.com/sfackler/rust-openssl"
-documentation = "https://sfackler.github.io/rust-openssl/doc/v0.7.3/openssl"
+documentation = "https://sfackler.github.io/rust-openssl/doc/v0.7.4/openssl"
 readme = "../README.md"
 keywords = ["crypto", "tls", "ssl", "dtls"]
 build = "build.rs"
@@ -29,8 +29,8 @@ pkcs5_pbkdf2_hmac = ["openssl-sys/pkcs5_pbkdf2_hmac"]
 bitflags = ">= 0.2, < 0.4"
 lazy_static = "0.1"
 libc = "0.2"
-openssl-sys = { version = "0.7.3", path = "../openssl-sys" }
-openssl-sys-extras = { version = "0.7.3", path = "../openssl-sys-extras" }
+openssl-sys = { version = "0.7.4", path = "../openssl-sys" }
+openssl-sys-extras = { version = "0.7.4", path = "../openssl-sys-extras" }
 
 [build-dependencies]
 gcc = "0.3"
diff --git a/openssl/src/lib.rs b/openssl/src/lib.rs
index 942589e7..88b67d97 100644
--- a/openssl/src/lib.rs
+++ b/openssl/src/lib.rs
@@ -1,4 +1,4 @@
-#![doc(html_root_url="https://sfackler.github.io/rust-openssl/doc/v0.7.3")]
+#![doc(html_root_url="https://sfackler.github.io/rust-openssl/doc/v0.7.4")]
 
 #[macro_use]
 extern crate bitflags;
diff --git a/openssl/src/ssl/bio.rs b/openssl/src/ssl/bio.rs
index ef63d146..a361ae81 100644
--- a/openssl/src/ssl/bio.rs
+++ b/openssl/src/ssl/bio.rs
@@ -6,35 +6,20 @@ use std::io::prelude::*;
 use std::mem;
 use std::slice;
 use std::ptr;
+use std::sync::Arc;
 
 use ssl::error::SslError;
 
 // "rust"
 const NAME: [c_char; 5] = [114, 117, 115, 116, 0];
 
-// we use this after removing the stream from the BIO so that we don't have to
-// worry about freeing the heap allocated BIO_METHOD after freeing the BIO.
-static DESTROY_METHOD: BIO_METHOD = BIO_METHOD {
-    type_: BIO_TYPE_NONE,
-    name: &NAME[0],
-    bwrite: None,
-    bread: None,
-    bputs: None,
-    bgets: None,
-    ctrl: None,
-    create: None,
-    destroy: Some(destroy),
-    callback_ctrl: None,
-};
-
 pub struct StreamState<S> {
     pub stream: S,
     pub error: Option<io::Error>,
 }
 
-pub fn new<S: Read + Write>(stream: S) -> Result<(*mut BIO, Box<BIO_METHOD>), SslError> {
-
-    let method = Box::new(BIO_METHOD {
+pub fn new<S: Read + Write>(stream: S) -> Result<(*mut BIO, Arc<BIO_METHOD>), SslError> {
+    let method = Arc::new(BIO_METHOD {
         type_: BIO_TYPE_NONE,
         name: &NAME[0],
         bwrite: Some(bwrite::<S>),
@@ -43,7 +28,7 @@ pub fn new<S: Read + Write>(stream: S) -> Result<(*mut BIO, Box<BIO_METHOD>), Ss
         bgets: None,
         ctrl: Some(ctrl::<S>),
         create: Some(create),
-        destroy: None, // covered in the replacement BIO_METHOD
+        destroy: Some(destroy::<S>),
         callback_ctrl: None,
     });
 
@@ -66,14 +51,6 @@ pub unsafe fn take_error<S>(bio: *mut BIO) -> Option<io::Error> {
     state.error.take()
 }
 
-pub unsafe fn take_stream<S>(bio: *mut BIO) -> S {
-    let state: Box<StreamState<S>> = Box::from_raw((*bio).ptr as *mut _);
-    (*bio).ptr = ptr::null_mut();
-    (*bio).method = &DESTROY_METHOD as *const _ as *mut _;
-    (*bio).init = 0;
-    state.stream
-}
-
 pub unsafe fn get_ref<'a, S: 'a>(bio: *mut BIO) -> &'a S {
     let state: &'a StreamState<S> = mem::transmute((*bio).ptr);
     &state.stream
@@ -124,7 +101,7 @@ unsafe extern "C" fn bread<S: Read>(bio: *mut BIO, buf: *mut c_char, len: c_int)
 fn retriable_error(err: &io::Error) -> bool {
     match err.kind() {
         io::ErrorKind::WouldBlock | io::ErrorKind::NotConnected => true,
-        _ => false
+        _ => false,
     }
 }
 
@@ -159,11 +136,14 @@ unsafe extern "C" fn create(bio: *mut BIO) -> c_int {
     1
 }
 
-unsafe extern "C" fn destroy(bio: *mut BIO) -> c_int {
+unsafe extern "C" fn destroy<S>(bio: *mut BIO) -> c_int {
     if bio.is_null() {
         return 0;
     }
 
-    assert!((*bio).ptr.is_null());
+    assert!(!(*bio).ptr.is_null());
+    Box::<StreamState<S>>::from_raw((*bio).ptr as *mut _);
+    (*bio).ptr = ptr::null_mut();
+    (*bio).init = 0;
     1
 }
diff --git a/openssl/src/ssl/mod.rs b/openssl/src/ssl/mod.rs
index d529347f..955f10fd 100644
--- a/openssl/src/ssl/mod.rs
+++ b/openssl/src/ssl/mod.rs
@@ -10,7 +10,7 @@ use std::str;
 use std::net;
 use std::path::Path;
 use std::ptr;
-use std::sync::{Once, ONCE_INIT, Mutex};
+use std::sync::{Once, ONCE_INIT, Mutex, Arc};
 use std::cmp;
 use std::any::Any;
 #[cfg(any(feature = "npn", feature = "alpn"))]
@@ -778,6 +778,7 @@ impl Drop for Ssl {
 }
 
 impl Clone for Ssl {
+    /// # Deprecated
     fn clone(&self) -> Ssl {
         unsafe { rust_SSL_clone(self.ssl) };
         Ssl { ssl: self.ssl }
@@ -1003,23 +1004,22 @@ make_LibSslError! {
 /// A stream wrapper which handles SSL encryption for an underlying stream.
 pub struct SslStream<S> {
     ssl: Ssl,
-    _method: Box<ffi::BIO_METHOD>, // :(
+    _method: Arc<ffi::BIO_METHOD>, // NOTE: this *must* be after the Ssl field so things drop right
     _p: PhantomData<S>,
 }
 
 unsafe impl<S: Send> Send for SslStream<S> {}
 
+/// # Deprecated
+///
+/// This method does not behave as expected and will be removed in a future
+/// release.
 impl<S: Clone + Read + Write> Clone for SslStream<S> {
     fn clone(&self) -> SslStream<S> {
-        let stream = self.get_ref().clone();
-        Self::new_base(self.ssl.clone(), stream)
-    }
-}
-
-impl<S> Drop for SslStream<S> {
-    fn drop(&mut self) {
-        unsafe {
-            let _ = bio::take_stream::<S>(self.ssl.get_raw_rbio());
+        SslStream {
+            ssl: self.ssl.clone(),
+            _method: self._method.clone(),
+            _p: PhantomData,
         }
     }
 }
@@ -1232,10 +1232,16 @@ impl<S> SslStream<S> {
 }
 
 impl SslStream<::std::net::TcpStream> {
-    /// Like `TcpStream::try_clone`.
+    /// # Deprecated
+    ///
+    /// This method does not behave as expected and will be removed in a future
+    /// release.
     pub fn try_clone(&self) -> io::Result<SslStream<::std::net::TcpStream>> {
-        let stream = try!(self.get_ref().try_clone());
-        Ok(Self::new_base(self.ssl.clone(), stream))
+        Ok(SslStream {
+            ssl: self.ssl.clone(),
+            _method: self._method.clone(),
+            _p: PhantomData,
+        })
     }
 }
 
diff --git a/openssl/src/ssl/tests/mod.rs b/openssl/src/ssl/tests/mod.rs
index 7ed8fc3b..af3c005e 100644
--- a/openssl/src/ssl/tests/mod.rs
+++ b/openssl/src/ssl/tests/mod.rs
@@ -949,3 +949,11 @@ fn test_read_nonblocking() {
     assert!(bytes_read >= 5);
     assert_eq!(&input_buffer[..5], b"HTTP/");
 }
+
+#[test]
+fn broken_try_clone_doesnt_crash() {
+    let context = SslContext::new(SslMethod::Sslv23).unwrap();
+    let inner = TcpStream::connect("example.com:443").unwrap();
+    let stream1 = SslStream::connect(&context, inner).unwrap();
+    let _stream2 = stream1.try_clone().unwrap();
+}