Merge pull request #518 from mlalic/fix-client-ssl-verifier

fix(client): keep the underlying connector when setting an SSL verifier

Author: seanmonstar

benches/client.rs

@@ -8,7 +8,7 @@ use std::fmt;
 use std::io::{self, Read, Write, Cursor};
 use std::net::SocketAddr;
 
-use hyper::net;
+use hyper::net::{self, ContextVerifier};
 
 static README: &'static [u8] = include_bytes!("../README.md");
 
@@ -83,6 +83,9 @@ impl net::NetworkConnector for MockConnector {
         Ok(MockStream::new())
     }
 
+    fn set_ssl_verifier(&mut self, _verifier: ContextVerifier) {
+        // pass
+    }
 }
 
 #[bench]

src/client/mod.rs

@@ -41,7 +41,7 @@ use url::ParseError as UrlError;
 use header::{Headers, Header, HeaderFormat};
 use header::{ContentLength, Location};
 use method::Method;
-use net::{NetworkConnector, NetworkStream, HttpConnector, ContextVerifier};
+use net::{NetworkConnector, NetworkStream, ContextVerifier};
 use status::StatusClass::Redirection;
 use {Url};
 use Error;
@@ -85,10 +85,7 @@ impl Client {
 
     /// Set the SSL verifier callback for use with OpenSSL.
     pub fn set_ssl_verifier(&mut self, verifier: ContextVerifier) {
-        self.connector = with_connector(Pool::with_connector(
-            Default::default(),
-            HttpConnector(Some(verifier))
-        ));
+        self.connector.set_ssl_verifier(verifier);
     }
 
     /// Set the RedirectPolicy.
@@ -147,6 +144,10 @@ impl<C: NetworkConnector<Stream=S> + Send, S: NetworkStream + Send> NetworkConne
         -> ::Result<Box<NetworkStream + Send>> {
         Ok(try!(self.0.connect(host, port, scheme)).into())
     }
+    #[inline]
+    fn set_ssl_verifier(&mut self, verifier: ContextVerifier) {
+        self.0.set_ssl_verifier(verifier);
+    }
 }
 
 struct Connector(Box<NetworkConnector<Stream=Box<NetworkStream + Send>> + Send>);
@@ -158,6 +159,10 @@ impl NetworkConnector for Connector {
         -> ::Result<Box<NetworkStream + Send>> {
         Ok(try!(self.0.connect(host, port, scheme)).into())
     }
+    #[inline]
+    fn set_ssl_verifier(&mut self, verifier: ContextVerifier) {
+        self.0.set_ssl_verifier(verifier);
+    }
 }
 
 /// Options for an individual Request.
@@ -399,6 +404,8 @@ mod tests {
     use header::Server;
     use super::{Client, RedirectPolicy};
     use url::Url;
+    use mock::ChannelMockConnector;
+    use std::sync::mpsc::{self, TryRecvError};
 
     mock_connector!(MockRedirectPolicy {
         "http://127.0.0.1" =>       "HTTP/1.1 301 Redirect\r\n\
@@ -445,4 +452,30 @@ mod tests {
         assert_eq!(res.headers.get(), Some(&Server("mock2".to_string())));
     }
 
+    /// Tests that the `Client::set_ssl_verifier` method does not drop the
+    /// old connector, but rather delegates the change to the connector itself.
+    #[test]
+    fn test_client_set_ssl_verifer() {
+        let (tx, rx) = mpsc::channel();
+        let mut client = Client::with_connector(ChannelMockConnector::new(tx));
+
+        client.set_ssl_verifier(Box::new(|_| {}));
+
+        // Make sure that the client called the `set_ssl_verifier` method
+        match rx.try_recv() {
+            Ok(meth) => {
+                assert_eq!(meth, "set_ssl_verifier");
+            },
+            _ => panic!("Expected a call to `set_ssl_verifier`"),
+        };
+        // Now make sure that no other method was called, as well as that
+        // the connector is still alive (i.e. wasn't dropped by the client).
+        match rx.try_recv() {
+            Err(TryRecvError::Empty) => {},
+            Err(TryRecvError::Disconnected) => {
+                panic!("Expected the connector to still be alive.");
+            },
+            Ok(_) => panic!("Did not expect any more method calls."),
+        };
+    }
 }

src/client/pool.rs

@@ -5,7 +5,7 @@ use std::io::{self, Read, Write};
 use std::net::{SocketAddr, Shutdown};
 use std::sync::{Arc, Mutex};
 
-use net::{NetworkConnector, NetworkStream, HttpConnector};
+use net::{NetworkConnector, NetworkStream, HttpConnector, ContextVerifier};
 
 /// The `NetworkConnector` that behaves as a connection pool used by hyper's `Client`.
 pub struct Pool<C: NetworkConnector> {
@@ -120,6 +120,10 @@ impl<C: NetworkConnector<Stream=S>, S: NetworkStream + Send> NetworkConnector fo
             pool: self.inner.clone()
         })
     }
+    #[inline]
+    fn set_ssl_verifier(&mut self, verifier: ContextVerifier) {
+        self.connector.set_ssl_verifier(verifier);
+    }
 }
 
 /// A Stream that will try to be returned to the Pool when dropped.
@@ -185,8 +189,9 @@ impl<S> Drop for PooledStream<S> {
 #[cfg(test)]
 mod tests {
     use std::net::Shutdown;
-    use mock::MockConnector;
+    use mock::{MockConnector, ChannelMockConnector};
     use net::{NetworkConnector, NetworkStream};
+    use std::sync::mpsc;
 
     use super::{Pool, key};
 
@@ -224,5 +229,19 @@ mod tests {
         assert_eq!(locked.conns.len(), 0);
     }
 
+    /// Tests that the `Pool::set_ssl_verifier` method sets the SSL verifier of
+    /// the underlying `Connector` instance that it uses.
+    #[test]
+    fn test_set_ssl_verifier_delegates_to_connector() {
+        let (tx, rx) = mpsc::channel();
+        let mut pool = Pool::with_connector(
+            Default::default(), ChannelMockConnector::new(tx));
+
+        pool.set_ssl_verifier(Box::new(|_| { }));
 
+        match rx.try_recv() {
+            Ok(meth) => assert_eq!(meth, "set_ssl_verifier"),
+            _ => panic!("Expected a call to `set_ssl_verifier`"),
+        };
+    }
 }

src/mock.rs

@@ -1,8 +1,9 @@
 use std::fmt;
 use std::io::{self, Read, Write, Cursor};
 use std::net::SocketAddr;
+use std::sync::mpsc::Sender;
 
-use net::{NetworkStream, NetworkConnector};
+use net::{NetworkStream, NetworkConnector, ContextVerifier};
 
 pub struct MockStream {
     pub read: Cursor<Vec<u8>>,
@@ -76,6 +77,39 @@ impl NetworkConnector for MockConnector {
     fn connect(&mut self, _host: &str, _port: u16, _scheme: &str) -> ::Result<MockStream> {
         Ok(MockStream::new())
     }
+
+    fn set_ssl_verifier(&mut self, _verifier: ContextVerifier) {
+        // pass
+    }
+}
+
+/// A mock implementation of the `NetworkConnector` trait that keeps track of all calls to its
+/// methods by sending corresponding messages onto a channel.
+///
+/// Otherwise, it behaves the same as `MockConnector`.
+pub struct ChannelMockConnector {
+    calls: Sender<String>,
+}
+
+impl ChannelMockConnector {
+    pub fn new(calls: Sender<String>) -> ChannelMockConnector {
+        ChannelMockConnector { calls: calls }
+    }
+}
+
+impl NetworkConnector for ChannelMockConnector {
+    type Stream = MockStream;
+    #[inline]
+    fn connect(&mut self, _host: &str, _port: u16, _scheme: &str)
+            -> ::Result<MockStream> {
+        self.calls.send("connect".into()).unwrap();
+        Ok(MockStream::new())
+    }
+
+    #[inline]
+    fn set_ssl_verifier(&mut self, _verifier: ContextVerifier) {
+        self.calls.send("set_ssl_verifier".into()).unwrap();
+    }
 }
 
 /// new connectors must be created if you wish to intercept requests.
@@ -107,6 +141,9 @@ macro_rules! mock_connector (
                 }
             }
 
+            fn set_ssl_verifier(&mut self, _verifier: ::net::ContextVerifier) {
+                // pass
+            }
         }
 
     )

src/net.rs

@@ -70,6 +70,9 @@ pub trait NetworkConnector {
     type Stream: Into<Box<NetworkStream + Send>>;
     /// Connect to a remote address.
     fn connect(&mut self, host: &str, port: u16, scheme: &str) -> ::Result<Self::Stream>;
+    /// Sets the given `ContextVerifier` to be used when verifying the SSL context
+    /// on the establishment of a new connection.
+    fn set_ssl_verifier(&mut self, verifier: ContextVerifier);
 }
 
 impl<T: NetworkStream + Send> From<T> for Box<NetworkStream + Send> {
@@ -344,12 +347,15 @@ impl NetworkConnector for HttpConnector {
             }
         }))
     }
+    fn set_ssl_verifier(&mut self, verifier: ContextVerifier) {
+        self.0 = Some(verifier);
+    }
 }
 
 #[cfg(test)]
 mod tests {
     use mock::MockStream;
-    use super::NetworkStream;
+    use super::{NetworkStream, HttpConnector, NetworkConnector};
 
     #[test]
     fn test_downcast_box_stream() {
@@ -371,4 +377,12 @@ mod tests {
 
     }
 
+    #[test]
+    fn test_http_connector_set_ssl_verifier() {
+        let mut connector = HttpConnector(None);
+
+        connector.set_ssl_verifier(Box::new(|_| {}));
+
+        assert!(connector.0.is_some());
+    }
 }