From dacde211c3bc095cc536b7bfc2078187d29eeeaf Mon Sep 17 00:00:00 2001
From: Cody P Schafer <dev@codyps.com>
Date: Mon, 18 Jan 2016 15:30:52 -0500
Subject: [PATCH 1/2] ssl: fix refcounting of SslContext when set_ssl_context
 is used

Additionally impl Clone for SslContext to both allow us to use it &
allow external users to take advantage of SslContext's internal
refcount.

Maintain the existing signature for set_ssl_context(), but
add inline comments recommending changing it.

Fixes #333
---
 openssl/src/ssl/mod.rs | 31 ++++++++++++++++++++++++++++---
 1 file changed, 28 insertions(+), 3 deletions(-)

diff --git a/openssl/src/ssl/mod.rs b/openssl/src/ssl/mod.rs
index 9ea8a27b..7366cc4a 100644
--- a/openssl/src/ssl/mod.rs
+++ b/openssl/src/ssl/mod.rs
@@ -481,6 +481,8 @@ fn wrap_ssl_result(res: c_int) -> Result<(), SslError> {
 }
 
 /// An SSL context object
+///
+/// Internally ref-counted, use `.clone()` in the same way as Rc and Arc.
 pub struct SslContext {
     ctx: *mut ffi::SSL_CTX,
 }
@@ -488,6 +490,12 @@ pub struct SslContext {
 unsafe impl Send for SslContext {}
 unsafe impl Sync for SslContext {}
 
+impl Clone for SslContext {
+    fn clone(&self) -> Self {
+        unsafe { SslContext::new_ref(self.ctx) }
+    }
+}
+
 // TODO: add useful info here
 impl fmt::Debug for SslContext {
     fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
@@ -502,6 +510,12 @@ impl Drop for SslContext {
 }
 
 impl SslContext {
+    // Create a new SslContext given an existing ref, and incriment ref-count appropriately.
+    unsafe fn new_ref(ctx: *mut ffi::SSL_CTX) -> SslContext {
+        rust_SSL_CTX_clone(ctx);
+        SslContext { ctx: ctx }
+    }
+
     /// Creates a new SSL context.
     pub fn new(method: SslMethod) -> Result<SslContext, SslError> {
         init();
@@ -955,16 +969,27 @@ impl Ssl {
     }
 
     /// change the context corresponding to the current connection
+    ///
+    /// Returns a clone of the SslContext @ctx (ie: the new context). The old context is freed.
     pub fn set_ssl_context(&self, ctx: &SslContext) -> SslContext {
-        SslContext { ctx: unsafe { ffi::SSL_set_SSL_CTX(self.ssl, ctx.ctx) } }
+        // If duplication of @ctx's cert fails, this returns NULL. This _appears_ to only occur on
+        // allocation failures (meaning panicing is probably appropriate), but it might be nice to
+        // propogate the error.
+        assert!(unsafe { ffi::SSL_set_SSL_CTX(self.ssl, ctx.ctx) } != ptr::null_mut());
+
+        // FIXME: we return this reference here for compatibility, but it isn't actually required.
+        // This should be removed when a api-incompatabile version is to be released.
+        //
+        // ffi:SSL_set_SSL_CTX() returns copy of the ctx pointer passed to it, so it's easier for
+        // us to do the clone directly.
+        ctx.clone()
     }
 
     /// obtain the context corresponding to the current connection
     pub fn get_ssl_context(&self) -> SslContext {
         unsafe {
             let ssl_ctx = ffi::SSL_get_SSL_CTX(self.ssl);
-            rust_SSL_CTX_clone(ssl_ctx);
-            SslContext { ctx: ssl_ctx }
+            SslContext::new_ref(ssl_ctx)
         }
     }
 }

From d1825c7a86717faf518e0abd08f0f5c1e94c0ca8 Mon Sep 17 00:00:00 2001
From: Cody P Schafer <dev@codyps.com>
Date: Mon, 18 Jan 2016 16:39:27 -0500
Subject: [PATCH 2/2] openssl/ssl/context: test that we are refcounting
 correctly

Not a perfect test, on failure it _might_ exit with this output:

Process didn't exit successfully:
`/home/cody/g/rust-openssl/openssl/target/debug/openssl-8e712036e3aac4fe`
(signal: 11)

But unclear if we can do any better.
---
 openssl/src/ssl/tests/mod.rs | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/openssl/src/ssl/tests/mod.rs b/openssl/src/ssl/tests/mod.rs
index a763f496..f5a42536 100644
--- a/openssl/src/ssl/tests/mod.rs
+++ b/openssl/src/ssl/tests/mod.rs
@@ -1045,3 +1045,16 @@ fn flush_panic() {
     let mut stream = SslStream::connect(&ctx, stream).unwrap();
     let _ = stream.flush();
 }
+
+#[test]
+fn refcount_ssl_context() {
+    let ssl = {
+        let ctx = SslContext::new(SslMethod::Sslv23).unwrap();
+        ssl::Ssl::new(&ctx).unwrap()
+    };
+
+    {
+        let new_ctx_a = SslContext::new(SslMethod::Sslv23).unwrap();
+        let _new_ctx_b = ssl.set_ssl_context(&new_ctx_a);
+    }
+}