From 972b3a388ac3af98ba038927c551b92be3a68d62 Mon Sep 17 00:00:00 2001 From: Sean McArthur Date: Tue, 5 May 2015 12:34:36 -0700 Subject: [PATCH] feat(error): add Ssl variant to hyper::Error The errors from openssl were previously boxed into a Box, which lost some specifics and made it difficult to match against. To solve this, an `Ssl` variant is added to the `Error` enum of hyper, and is returned when openssl returns specific errors. Closes #483 BREAKING CHANGE: Adds a variant to `hyper::Error`, which may break any exhaustive matches. --- benches/client.rs | 2 +- src/client/mod.rs | 4 +-- src/client/pool.rs | 2 +- src/error.rs | 31 ++++++++++++++++++---- src/mock.rs | 6 ++--- src/net.rs | 66 ++++++++++++++++++++-------------------------- 6 files changed, 61 insertions(+), 50 deletions(-) diff --git a/benches/client.rs b/benches/client.rs index ab9527994f..9059ec753c 100644 --- a/benches/client.rs +++ b/benches/client.rs @@ -79,7 +79,7 @@ struct MockConnector; impl net::NetworkConnector for MockConnector { type Stream = MockStream; - fn connect(&mut self, _: &str, _: u16, _: &str) -> io::Result { + fn connect(&mut self, _: &str, _: u16, _: &str) -> hyper::Result { Ok(MockStream::new()) } diff --git a/src/client/mod.rs b/src/client/mod.rs index febcb1d7e7..4d1a5be43c 100644 --- a/src/client/mod.rs +++ b/src/client/mod.rs @@ -144,7 +144,7 @@ impl + Send, S: NetworkStream + Send> NetworkConne type Stream = Box; #[inline] fn connect(&mut self, host: &str, port: u16, scheme: &str) - -> io::Result> { + -> ::Result> { Ok(try!(self.0.connect(host, port, scheme)).into()) } } @@ -155,7 +155,7 @@ impl NetworkConnector for Connector { type Stream = Box; #[inline] fn connect(&mut self, host: &str, port: u16, scheme: &str) - -> io::Result> { + -> ::Result> { Ok(try!(self.0.connect(host, port, scheme)).into()) } } diff --git a/src/client/pool.rs b/src/client/pool.rs index 1b3a8dacba..0f570cc353 100644 --- a/src/client/pool.rs +++ b/src/client/pool.rs @@ -98,7 +98,7 @@ impl PoolImpl { impl, S: NetworkStream + Send> NetworkConnector for Pool { type Stream = PooledStream; - fn connect(&mut self, host: &str, port: u16, scheme: &str) -> io::Result> { + fn connect(&mut self, host: &str, port: u16, scheme: &str) -> ::Result> { let key = key(host, port, scheme); let mut locked = self.inner.lock().unwrap(); let mut should_remove = false; diff --git a/src/error.rs b/src/error.rs index bad242c679..99368002b9 100644 --- a/src/error.rs +++ b/src/error.rs @@ -4,11 +4,19 @@ use std::fmt; use std::io::Error as IoError; use httparse; +use openssl::ssl::error::SslError; use url; -use self::Error::{Method, Uri, Version, - Header, Status, Io, - TooLarge}; +use self::Error::{ + Method, + Uri, + Version, + Header, + Status, + Io, + Ssl, + TooLarge +}; /// Result type often returned from methods that can have hyper `Error`s. @@ -29,8 +37,10 @@ pub enum Error { TooLarge, /// An invalid `Status`, such as `1337 ELITE`. Status, - /// An `IoError` that occurred while trying to read or write to a network stream. + /// 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) } impl fmt::Display for Error { @@ -48,13 +58,15 @@ impl StdError for Error { Header => "Invalid Header provided", TooLarge => "Message head is too large", Status => "Invalid Status provided", - Io(_) => "An IoError occurred while connecting to the specified network", + Io(ref e) => e.description(), + Ssl(ref e) => e.description(), } } fn cause(&self) -> Option<&StdError> { match *self { Io(ref error) => Some(error), + Ssl(ref error) => Some(error), Uri(ref error) => Some(error), _ => None, } @@ -73,6 +85,15 @@ impl From for Error { } } +impl From for Error { + fn from(err: SslError) -> Error { + match err { + SslError::StreamError(err) => Io(err), + err => Ssl(err), + } + } +} + impl From for Error { fn from(err: httparse::Error) -> Error { match err { diff --git a/src/mock.rs b/src/mock.rs index cdf9e48d20..9cbd022d07 100644 --- a/src/mock.rs +++ b/src/mock.rs @@ -73,7 +73,7 @@ pub struct MockConnector; impl NetworkConnector for MockConnector { type Stream = MockStream; - fn connect(&mut self, _host: &str, _port: u16, _scheme: &str) -> io::Result { + fn connect(&mut self, _host: &str, _port: u16, _scheme: &str) -> ::Result { Ok(MockStream::new()) } } @@ -88,7 +88,7 @@ macro_rules! mock_connector ( impl ::net::NetworkConnector for $name { type Stream = ::mock::MockStream; - fn connect(&mut self, host: &str, port: u16, scheme: &str) -> ::std::io::Result<::mock::MockStream> { + fn connect(&mut self, host: &str, port: u16, scheme: &str) -> $crate::Result<::mock::MockStream> { use std::collections::HashMap; use std::io::Cursor; debug!("MockStream::connect({:?}, {:?}, {:?})", host, port, scheme); @@ -99,7 +99,7 @@ macro_rules! mock_connector ( let key = format!("{}://{}", scheme, host); // ignore port for now match map.get(&*key) { - Some(res) => Ok(::mock::MockStream { + Some(res) => Ok($crate::mock::MockStream { write: vec![], read: Cursor::new(res.to_string().into_bytes()), }), diff --git a/src/net.rs b/src/net.rs index 2146d413e7..8a393c89fe 100644 --- a/src/net.rs +++ b/src/net.rs @@ -9,7 +9,7 @@ use std::sync::Arc; use openssl::ssl::{Ssl, SslStream, SslContext, SSL_VERIFY_NONE}; use openssl::ssl::SslMethod::Sslv23; -use openssl::ssl::error::{SslError, StreamError, OpenSslErrors, SslSessionClosed}; +use openssl::ssl::error::StreamError as SslIoError; use openssl::x509::X509FileType; use typeable::Typeable; @@ -29,7 +29,7 @@ pub trait NetworkListener: Clone { //fn listen(&mut self, addr: To) -> io::Result; /// Returns an iterator of streams. - fn accept(&mut self) -> io::Result; + fn accept(&mut self) -> ::Result; /// Get the address this Listener ended up listening on. fn local_addr(&mut self) -> io::Result; @@ -47,8 +47,8 @@ pub trait NetworkListener: Clone { pub struct NetworkConnections<'a, N: NetworkListener + 'a>(&'a mut N); impl<'a, N: NetworkListener + 'a> Iterator for NetworkConnections<'a, N> { - type Item = io::Result; - fn next(&mut self) -> Option> { + type Item = ::Result; + fn next(&mut self) -> Option<::Result> { Some(self.0.accept()) } } @@ -58,6 +58,7 @@ pub trait NetworkStream: Read + Write + Any + Send + Typeable { /// Get the remote address of the underlying connection. fn peer_addr(&mut self) -> io::Result; /// This will be called when Stream should no longer be kept alive. + #[inline] fn close(&mut self, _how: Shutdown) -> io::Result<()> { Ok(()) } @@ -68,7 +69,7 @@ pub trait NetworkConnector { /// Type of Stream to create type Stream: Into>; /// Connect to a remote address. - fn connect(&mut self, host: &str, port: u16, scheme: &str) -> io::Result; + fn connect(&mut self, host: &str, port: u16, scheme: &str) -> ::Result; } impl From for Box { @@ -158,22 +159,22 @@ impl Clone for HttpListener { impl HttpListener { /// Start listening to an address over HTTP. - pub fn http(addr: To) -> io::Result { + pub fn http(addr: To) -> ::Result { Ok(HttpListener::Http(try!(TcpListener::bind(addr)))) } /// Start listening to an address over HTTPS. - pub fn https(addr: To, cert: &Path, key: &Path) -> io::Result { - let mut ssl_context = try!(SslContext::new(Sslv23).map_err(lift_ssl_error)); - try!(ssl_context.set_cipher_list("DEFAULT").map_err(lift_ssl_error)); - try!(ssl_context.set_certificate_file(cert, X509FileType::PEM).map_err(lift_ssl_error)); - try!(ssl_context.set_private_key_file(key, X509FileType::PEM).map_err(lift_ssl_error)); + pub fn https(addr: To, cert: &Path, key: &Path) -> ::Result { + let mut ssl_context = try!(SslContext::new(Sslv23)); + try!(ssl_context.set_cipher_list("DEFAULT")); + try!(ssl_context.set_certificate_file(cert, X509FileType::PEM)); + try!(ssl_context.set_private_key_file(key, X509FileType::PEM)); ssl_context.set_verify(SSL_VERIFY_NONE, None); HttpListener::https_with_context(addr, ssl_context) } /// Start listening to an address of HTTPS using the given SslContext - pub fn https_with_context(addr: To, ssl_context: SslContext) -> io::Result { + pub fn https_with_context(addr: To, ssl_context: SslContext) -> ::Result { Ok(HttpListener::Https(try!(TcpListener::bind(addr)), Arc::new(ssl_context))) } } @@ -182,21 +183,20 @@ impl NetworkListener for HttpListener { type Stream = HttpStream; #[inline] - fn accept(&mut self) -> io::Result { - Ok(match *self { - HttpListener::Http(ref mut tcp) => HttpStream::Http(CloneTcpStream(try!(tcp.accept()).0)), + fn accept(&mut self) -> ::Result { + match *self { + HttpListener::Http(ref mut tcp) => Ok(HttpStream::Http(CloneTcpStream(try!(tcp.accept()).0))), HttpListener::Https(ref mut tcp, ref ssl_context) => { let stream = CloneTcpStream(try!(tcp.accept()).0); match SslStream::new_server(&**ssl_context, stream) { - Ok(ssl_stream) => HttpStream::Https(ssl_stream), - Err(StreamError(e)) => { - return Err(io::Error::new(io::ErrorKind::ConnectionAborted, - e)); + Ok(ssl_stream) => Ok(HttpStream::Https(ssl_stream)), + Err(SslIoError(e)) => { + Err(io::Error::new(io::ErrorKind::ConnectionAborted, e).into()) }, - Err(e) => return Err(lift_ssl_error(e)) + Err(e) => Err(e.into()) } } - }) + } } #[inline] @@ -308,9 +308,9 @@ pub type ContextVerifier = Box () + Send>; impl NetworkConnector for HttpConnector { type Stream = HttpStream; - fn connect(&mut self, host: &str, port: u16, scheme: &str) -> io::Result { + fn connect(&mut self, host: &str, port: u16, scheme: &str) -> ::Result { let addr = &(host, port); - match scheme { + Ok(try!(match scheme { "http" => { debug!("http scheme"); Ok(HttpStream::Http(CloneTcpStream(try!(TcpStream::connect(addr))))) @@ -318,30 +318,20 @@ impl NetworkConnector for HttpConnector { "https" => { debug!("https scheme"); let stream = CloneTcpStream(try!(TcpStream::connect(addr))); - let mut context = try!(SslContext::new(Sslv23).map_err(lift_ssl_error)); + let mut context = try!(SslContext::new(Sslv23)); if let Some(ref mut verifier) = self.0 { verifier(&mut context); } - let ssl = try!(Ssl::new(&context).map_err(lift_ssl_error)); - try!(ssl.set_hostname(host).map_err(lift_ssl_error)); - let stream = try!(SslStream::new(&context, stream).map_err(lift_ssl_error)); + let ssl = try!(Ssl::new(&context)); + try!(ssl.set_hostname(host)); + let stream = try!(SslStream::new(&context, stream)); Ok(HttpStream::Https(stream)) }, _ => { Err(io::Error::new(io::ErrorKind::InvalidInput, "Invalid scheme for Http")) } - } - } -} - -fn lift_ssl_error(ssl: SslError) -> io::Error { - debug!("lift_ssl_error: {:?}", ssl); - match ssl { - StreamError(err) => err, - SslSessionClosed => io::Error::new(io::ErrorKind::ConnectionAborted, - "SSL Connection Closed"), - e@OpenSslErrors(..) => io::Error::new(io::ErrorKind::Other, e) + })) } }