Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
May 10, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion benches/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand Down Expand Up @@ -83,6 +83,9 @@ impl net::NetworkConnector for MockConnector {
Ok(MockStream::new())
}

fn set_ssl_verifier(&mut self, _verifier: ContextVerifier) {
// pass
}
}

#[bench]
Expand Down
43 changes: 38 additions & 5 deletions src/client/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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>);
Expand All @@ -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.
Expand Down Expand Up @@ -401,6 +406,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\
Expand Down Expand Up @@ -447,4 +454,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."),
};
}
}
23 changes: 21 additions & 2 deletions src/client/pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down Expand Up @@ -119,6 +119,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.
Expand Down Expand Up @@ -184,8 +188,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};

Expand Down Expand Up @@ -223,5 +228,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`"),
};
}
}
39 changes: 38 additions & 1 deletion src/mock.rs
Original file line number Diff line number Diff line change
@@ -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>>,
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -107,6 +141,9 @@ macro_rules! mock_connector (
}
}

fn set_ssl_verifier(&mut self, _verifier: ::net::ContextVerifier) {
// pass
}
}

)
Expand Down
16 changes: 15 additions & 1 deletion src/net.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down Expand Up @@ -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() {
Expand All @@ -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());
}
}