Skip to content

Commit

Permalink
feat(ssl): redesign SSL usage
Browse files Browse the repository at this point in the history
BREAKING CHANGE: Server::https was changed to allow any implementation
  of Ssl. Server in general was also changed. HttpConnector no longer
  uses SSL; using HttpsConnector instead.
  • Loading branch information
seanmonstar committed Jun 20, 2015
1 parent e689f20 commit 53bba6e
Show file tree
Hide file tree
Showing 17 changed files with 339 additions and 375 deletions.
2 changes: 2 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ matrix:
env: FEATURES="--features nightly"
- rust: beta
- rust: stable
- rust: stable
env: FEATURES="--no-default-features"

sudo: false

Expand Down
23 changes: 17 additions & 6 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,22 +12,33 @@ authors = ["Sean McArthur <sean.monstar@gmail.com>",
keywords = ["http", "hyper", "hyperium"]

[dependencies]
cookie = "0.1"
httparse = "0.1"
log = "0.3"
mime = "0.0.11"
mime = "0.0.12"
num_cpus = "0.2"
openssl = "0.6"
rustc-serialize = "0.3"
time = "0.1"
unicase = "0.1"
url = "0.2"
traitobject = "0.0.1"
typeable = "0.1"
solicit = "0.2"
unicase = "0.1"
url = "0.2"

[dependencies.cookie]
version = "0.1"
default-features = false

[dependencies.openssl]
version = "0.6"
optional = true

[dependencies.solicit]
version = "0.3"
default-features = false

[dev-dependencies]
env_logger = "*"

[features]
default = ["ssl"]
ssl = ["openssl", "cookie/secure", "solicit/openssl"]
nightly = []
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ fn hello(_: Request, res: Response<Fresh>) {
}

fn main() {
Server::http(hello).listen("127.0.0.1:3000").unwrap();
Server::http("127.0.0.1:3000").unwrap().handle(hello);
}
```

Expand Down
4 changes: 2 additions & 2 deletions examples/hello.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ fn hello(_: Request, res: Response) {

fn main() {
env_logger::init().unwrap();
let _listening = hyper::Server::http(hello)
.listen("127.0.0.1:3000").unwrap();
let _listening = hyper::Server::http("127.0.0.1:3000").unwrap()
.handle(hello);
println!("Listening on http://127.0.0.1:3000");
}
4 changes: 2 additions & 2 deletions examples/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ fn echo(mut req: Request, mut res: Response) {

fn main() {
env_logger::init().unwrap();
let server = Server::http(echo);
let _guard = server.listen("127.0.0.1:1337").unwrap();
let server = Server::http("127.0.0.1:1337").unwrap();
let _guard = server.handle(echo);
println!("Listening on http://127.0.0.1:1337");
}
36 changes: 1 addition & 35 deletions src/client/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ use url::ParseError as UrlError;
use header::{Headers, Header, HeaderFormat};
use header::{ContentLength, Location};
use method::Method;
use net::{NetworkConnector, NetworkStream, ContextVerifier};
use net::{NetworkConnector, NetworkStream};
use {Url};
use Error;

Expand Down Expand Up @@ -116,11 +116,6 @@ impl Client {
}
}

/// Set the SSL verifier callback for use with OpenSSL.
pub fn set_ssl_verifier(&mut self, verifier: ContextVerifier) {
self.protocol.set_ssl_verifier(verifier);
}

/// Set the RedirectPolicy.
pub fn set_redirect_policy(&mut self, policy: RedirectPolicy) {
self.redirect_policy = policy;
Expand Down Expand Up @@ -417,8 +412,6 @@ 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 @@ -464,31 +457,4 @@ mod tests {
let res = client.get("http://127.0.0.1").send().unwrap();
assert_eq!(res.headers.get(), Some(&Server("mock2".to_owned())));
}

/// 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."),
};
}
}
33 changes: 6 additions & 27 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, ContextVerifier};
use net::{NetworkConnector, NetworkStream, DefaultConnector};

/// The `NetworkConnector` that behaves as a connection pool used by hyper's `Client`.
pub struct Pool<C: NetworkConnector> {
Expand Down Expand Up @@ -58,11 +58,11 @@ impl<'a> From<&'a str> for Scheme {
}
}

impl Pool<HttpConnector> {
/// Creates a `Pool` with an `HttpConnector`.
impl Pool<DefaultConnector> {
/// Creates a `Pool` with a `DefaultConnector`.
#[inline]
pub fn new(config: Config) -> Pool<HttpConnector> {
Pool::with_connector(config, HttpConnector(None))
pub fn new(config: Config) -> Pool<DefaultConnector> {
Pool::with_connector(config, DefaultConnector::default())
}
}

Expand Down Expand Up @@ -119,10 +119,6 @@ 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 @@ -181,9 +177,8 @@ impl<S> Drop for PooledStream<S> {
#[cfg(test)]
mod tests {
use std::net::Shutdown;
use mock::{MockConnector, ChannelMockConnector};
use mock::{MockConnector};
use net::{NetworkConnector, NetworkStream};
use std::sync::mpsc;

use super::{Pool, key};

Expand Down Expand Up @@ -220,20 +215,4 @@ mod tests {
let locked = pool.inner.lock().unwrap();
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`"),
};
}
}
4 changes: 2 additions & 2 deletions src/client/request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use url::Url;
use method::{self, Method};
use header::Headers;
use header::Host;
use net::{NetworkStream, NetworkConnector, HttpConnector, Fresh, Streaming};
use net::{NetworkStream, NetworkConnector, DefaultConnector, Fresh, Streaming};
use version;
use client::{Response, get_host_and_port};

Expand Down Expand Up @@ -66,7 +66,7 @@ impl Request<Fresh> {

/// Create a new client request.
pub fn new(method: method::Method, url: Url) -> ::Result<Request<Fresh>> {
let mut conn = HttpConnector(None);
let mut conn = DefaultConnector::default();
Request::with_connector(method, url, &mut conn)
}

Expand Down
27 changes: 17 additions & 10 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@ use std::io::Error as IoError;
use std::str::Utf8Error;

use httparse;
use openssl::ssl::error::SslError;
use url;
use solicit::http::HttpError as Http2Error;

#[cfg(feature = "openssl")]
use openssl::ssl::error::SslError;

use self::Error::{
Method,
Uri,
Expand Down Expand Up @@ -43,8 +45,8 @@ pub enum Error {
Status,
/// An `io::Error` that occurred while trying to read or write to a network stream.
Io(IoError),
/// An error from the `openssl` library.
Ssl(SslError),
/// An error from a SSL library.
Ssl(Box<StdError + Send + Sync>),
/// An HTTP/2-specific error, coming from the `solicit` library.
Http2(Http2Error),
/// Parsing a field as string failed
Expand Down Expand Up @@ -89,7 +91,7 @@ impl StdError for Error {
fn cause(&self) -> Option<&StdError> {
match *self {
Io(ref error) => Some(error),
Ssl(ref error) => Some(error),
Ssl(ref error) => Some(&**error),
Uri(ref error) => Some(error),
Http2(ref error) => Some(error),
_ => None,
Expand All @@ -109,11 +111,12 @@ impl From<url::ParseError> for Error {
}
}

#[cfg(feature = "openssl")]
impl From<SslError> for Error {
fn from(err: SslError) -> Error {
match err {
SslError::StreamError(err) => Io(err),
err => Ssl(err),
err => Ssl(Box::new(err)),
}
}
}
Expand Down Expand Up @@ -149,7 +152,6 @@ mod tests {
use std::error::Error as StdError;
use std::io;
use httparse;
use openssl::ssl::error::SslError;
use solicit::http::HttpError as Http2Error;
use url;
use super::Error;
Expand Down Expand Up @@ -192,12 +194,8 @@ mod tests {

from_and_cause!(io::Error::new(io::ErrorKind::Other, "other") => Io(..));
from_and_cause!(url::ParseError::EmptyHost => Uri(..));
from_and_cause!(SslError::SslSessionClosed => Ssl(..));
from_and_cause!(Http2Error::UnknownStreamId => Http2(..));

from!(SslError::StreamError(io::Error::new(io::ErrorKind::Other, "ssl negotiation")) => Io(..));


from!(httparse::Error::HeaderName => Header);
from!(httparse::Error::HeaderName => Header);
from!(httparse::Error::HeaderValue => Header);
Expand All @@ -207,4 +205,13 @@ mod tests {
from!(httparse::Error::TooManyHeaders => TooLarge);
from!(httparse::Error::Version => Version);
}

#[cfg(feature = "openssl")]
#[test]
fn test_from_ssl() {
use openssl::ssl::error::SslError;

from!(SslError::StreamError(io::Error::new(io::ErrorKind::Other, "ssl negotiation")) => Io(..));
from_and_cause!(SslError::SslSessionClosed => Ssl(..));
}
}
4 changes: 2 additions & 2 deletions src/header/common/set_cookie.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,13 +168,13 @@ fn test_fmt() {
fn cookie_jar() {
let jar = CookieJar::new(b"secret");
let cookie = Cookie::new("foo".to_owned(), "bar".to_owned());
jar.encrypted().add(cookie);
jar.add(cookie);

let cookies = SetCookie::from_cookie_jar(&jar);

let mut new_jar = CookieJar::new(b"secret");
cookies.apply_to_cookie_jar(&mut new_jar);

assert_eq!(jar.encrypted().find("foo"), new_jar.encrypted().find("foo"));
assert_eq!(jar.find("foo"), new_jar.find("foo"));
assert_eq!(jar.iter().collect::<Vec<Cookie>>(), new_jar.iter().collect::<Vec<Cookie>>());
}
15 changes: 1 addition & 14 deletions src/http/h1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use Error;
use header::{Headers, ContentLength, TransferEncoding};
use header::Encoding::Chunked;
use method::{Method};
use net::{NetworkConnector, NetworkStream, ContextVerifier};
use net::{NetworkConnector, NetworkStream};
use status::StatusCode;
use version::HttpVersion;
use version::HttpVersion::{Http10, Http11};
Expand Down Expand Up @@ -264,11 +264,6 @@ impl Protocol for Http11Protocol {

Ok(Box::new(Http11Message::with_stream(stream)))
}

#[inline]
fn set_ssl_verifier(&mut self, verifier: ContextVerifier) {
self.connector.set_ssl_verifier(verifier);
}
}

impl Http11Protocol {
Expand All @@ -292,10 +287,6 @@ impl<C: NetworkConnector<Stream=S> + Send + Sync, S: NetworkStream + Send> Netwo
-> ::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 + Sync>);
Expand All @@ -307,10 +298,6 @@ 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);
}
}


Expand Down
9 changes: 2 additions & 7 deletions src/http/h2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use http::{
ResponseHead,
RawStatus,
};
use net::{NetworkStream, NetworkConnector, ContextVerifier};
use net::{NetworkStream, NetworkConnector};
use net::{HttpConnector, HttpStream};
use url::Url;
use header::Headers;
Expand Down Expand Up @@ -133,11 +133,6 @@ impl<C, S> Protocol for Http2Protocol<C, S> where C: NetworkConnector<Stream=S>

Ok(Box::new(Http2Message::with_client(client)))
}

#[inline]
fn set_ssl_verifier(&mut self, verifier: ContextVerifier) {
self.connector.set_ssl_verifier(verifier)
}
}

/// Represents an HTTP/2 request, described by a `RequestHead` and the body of the request.
Expand Down Expand Up @@ -387,7 +382,7 @@ impl<S> HttpMessage for Http2Message<S> where S: CloneableStream {
/// (which produces an `HttpStream` for the underlying transport layer).
#[inline]
pub fn new_protocol() -> Http2Protocol<HttpConnector, HttpStream> {
Http2Protocol::with_connector(HttpConnector(None))
Http2Protocol::with_connector(HttpConnector)
}

#[cfg(test)]
Expand Down
Loading

0 comments on commit 53bba6e

Please sign in to comment.