Commit e85b49d3 authored by Steven Fackler's avatar Steven Fackler
Browse files

Work around the worst of clone bogusness 

SslStream::{clone,try_clone} are inherently broken since the Ssl object
shared by both streams is only going to be talking to one stream. Stuff
like hyper depends on try_clone, so we'll leave it here for now but
minimize the brokenness to "no worse than what it used to be like".
They'll be removed in 0.8.

cc #325
parent 93b388d6

openssl/src/ssl/bio.rs

+9 −29
Original line number Diff line number Diff line
@@ -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
@@ -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

}

openssl/src/ssl/mod.rs

+20 −14
Original line number Diff line number Diff line
@@ -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> {}



impl<S: Clone + Read + Write> Clone for SslStream<S> {

    /// # Deprecated

    ///

    /// This method does not behave as expected and will be removed in a future

    /// release.

    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

        })

    }

}

openssl/src/ssl/tests/mod.rs

+8 −0
Original line number Diff line number Diff line
@@ -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();

}