diff --git a/components/net/http_loader.rs b/components/net/http_loader.rs index 95578ba2774..5f3a1ebe330 100644 --- a/components/net/http_loader.rs +++ b/components/net/http_loader.rs @@ -21,6 +21,10 @@ use hyper::Error as HttpError; use hyper::client::Request; use hyper::header::StrictTransportSecurity; use hyper::header::{AcceptEncoding, Accept, ContentLength, ContentType, Host, Location, qitem, Quality, QualityItem}; +use hyper::client::{Request, Response}; +use hyper::header::{AcceptEncoding, Accept, ContentLength, ContentType, Host, Location, qitem, StrictTransportSecurity}; +use hyper::header::{Quality, QualityItem}; +use hyper::Error as HttpError; use hyper::method::Method; use hyper::mime::{Mime, TopLevel, SubLevel}; use hyper::net::{Fresh, HttpsConnector, Openssl, NetworkConnector, NetworkStream}; @@ -105,20 +109,9 @@ fn load_for_consumer(load_data: LoadData, devtools_chan: Option>, hsts_list: Arc>) { - let connector = { - // TODO: Is this still necessary? The type system is making it really hard to support both - // connectors. SSL is working, so it's not clear to me if the original rationale still - // stands? - // if opts::get().nossl { - // &HttpConnector - let mut context = SslContext::new(SslMethod::Sslv23).unwrap(); - context.set_verify(SSL_VERIFY_PEER, None); - context.set_CA_file(&resources_dir_path().join("certs")).unwrap(); + let requester = NetworkHttpRequester::new(); - &HttpsConnector::new(Openssl { context: Arc::new(context) }) - }; - - match load(load_data, resource_mgr_chan, devtools_chan, hsts_list, connector) { + match load(load_data, resource_mgr_chan, devtools_chan, hsts_list, &requester) { Err(LoadError::UnsupportedScheme(url)) => { let s = format!("{} request, but we don't support that scheme", &*url.scheme); send_error(url, s, start_chan) @@ -150,40 +143,67 @@ fn load_for_consumer(load_data: LoadData, } } -#[inline(always)] -fn connect(url: Url, - method: Method, - connector: &C) -> Result, LoadError> where - C: NetworkConnector, - S: Into> { - let connection = Request::with_connector(method, url.clone(), connector); +struct NetworkHttpRequester { + connector: HttpsConnector +} - let ssl_err_string = "Some(OpenSslErrors([UnknownError { library: \"SSL routines\", \ -function: \"SSL3_GET_SERVER_CERTIFICATE\", \ -reason: \"certificate verify failed\" }]))"; +impl NetworkHttpRequester { + fn new() -> NetworkHttpRequester { + // TODO: Is this still necessary? The type system is making it really hard to support both + // connectors. SSL is working, so it's not clear to me if the original rationale still + // stands? + // if opts::get().nossl { + // &HttpConnector + let mut context = SslContext::new(SslMethod::Sslv23).unwrap(); + context.set_verify(SSL_VERIFY_PEER, None); + context.set_CA_file(&resources_dir_path().join("certs")).unwrap(); - match connection { - Ok(req) => Ok(req), - - Err(HttpError::Io(ref io_error)) if ( - io_error.kind() == io::ErrorKind::Other && - io_error.description() == "Error in OpenSSL" && - // FIXME: This incredibly hacky. Make it more robust, and at least test it. - format!("{:?}", io_error.cause()) == ssl_err_string - ) => { - Err( - LoadError::Ssl( - url.clone(), - format!("ssl error {:?}: {:?} {:?}", io_error.kind(), io_error.description(), io_error.cause()) - ) - ) - }, - Err(e) => { - Err(LoadError::Connection(url, e.description().to_string())) + NetworkHttpRequester { + connector: HttpsConnector::new(Openssl { context: Arc::new(context) }) } } } +impl HttpRequester for NetworkHttpRequester { + fn send(&self, request: Request) -> Result { + unimplemented!() + } + + fn build(&self, url: Url, method: Method) -> Result, LoadError> { + let connection = Request::with_connector(method, url.clone(), &self.connector); + + let ssl_err_string = "Some(OpenSslErrors([UnknownError { library: \"SSL routines\", \ + function: \"SSL3_GET_SERVER_CERTIFICATE\", \ + reason: \"certificate verify failed\" }]))"; + + match connection { + Ok(req) => Ok(req), + + Err(HttpError::Io(ref io_error)) if ( + io_error.kind() == io::ErrorKind::Other && + io_error.description() == "Error in OpenSSL" && + // FIXME: This incredibly hacky. Make it more robust, and at least test it. + format!("{:?}", io_error.cause()) == ssl_err_string + ) => { + Err( + LoadError::Ssl( + url.clone(), + format!("ssl error {:?}: {:?} {:?}", io_error.kind(), io_error.description(), io_error.cause()) + ) + ) + }, + Err(e) => { + Err(LoadError::Connection(url, e.description().to_string())) + } + } + } +} + +pub trait HttpRequester { + fn build(&self, url: Url, method: Method) -> Result, LoadError>; + fn send(&self, request: Request) -> Result; +} + pub enum LoadError { UnsupportedScheme(Url), Connection(Url, String), @@ -194,14 +214,12 @@ pub enum LoadError { MaxRedirects(Url) } -fn load(mut load_data: LoadData, - resource_mgr_chan: IpcSender, - devtools_chan: Option>, - hsts_list: Arc>, - connector: &C) -> Result<(Box, Metadata), LoadError> where - C: NetworkConnector, - S: Into> { - +pub fn load(mut load_data: LoadData, + resource_mgr_chan: IpcSender, + devtools_chan: Option>, + hsts_list: Arc>, + requester: &HttpRequester) + -> Result<(Box, Metadata), LoadError> { // FIXME: At the time of writing this FIXME, servo didn't have any central // location for configuration. If you're reading this and such a // repository DOES exist, please update this constant to use it. @@ -243,7 +261,7 @@ fn load(mut load_data: LoadData, info!("requesting {}", url.serialize()); - let mut req = try!(connect(url.clone(), load_data.method.clone(), connector)); + let mut req = try!(requester.build(url.clone(), load_data.method.clone())); //Ensure that the host header is set from the original url let host = Host { diff --git a/tests/unit/net/http_loader.rs b/tests/unit/net/http_loader.rs index 9e27c47d73c..2c05aadb52d 100644 --- a/tests/unit/net/http_loader.rs +++ b/tests/unit/net/http_loader.rs @@ -2,153 +2,25 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -use net::http_loader::{load, LoadError}; -use hyper::net::{NetworkStream, NetworkConnector}; -use hyper; +use net::http_loader::{load, LoadError, HttpRequester}; use url::Url; -use std::io::{self, Read, Write, Cursor}; -use std::fmt; -use std::net::SocketAddr; use std::sync::{Arc, Mutex}; use ipc_channel::ipc; use net_traits::LoadData; use net::hsts::HSTSList; -use net::hsts::HSTSEntry; +use hyper::client::{Request, Response}; +use hyper::net::Fresh; +use hyper::method::Method; -#[derive(Clone)] -pub struct MockStream { - pub read: Cursor>, - pub write: Vec, - #[cfg(feature = "timeouts")] - pub read_timeout: Cell>, - #[cfg(feature = "timeouts")] - pub write_timeout: Cell> -} +struct MockHttpRequester; -impl fmt::Debug for MockStream { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "MockStream {{ read: {:?}, write: {:?} }}", self.read.get_ref(), self.write) - } -} - -impl PartialEq for MockStream { - fn eq(&self, other: &MockStream) -> bool { - self.read.get_ref() == other.read.get_ref() && self.write == other.write - } -} - -impl MockStream { - pub fn new() -> MockStream { - MockStream::with_input(b"") +impl HttpRequester for MockHttpRequester { + fn build(&self, url: Url, _: Method) -> Result, LoadError> { + Err(LoadError::Connection(url.clone(), "shouldn't connect".to_string())) } - #[cfg(not(feature = "timeouts"))] - pub fn with_input(input: &[u8]) -> MockStream { - MockStream { - read: Cursor::new(input.to_vec()), - write: vec![] - } - } - - #[cfg(feature = "timeouts")] - pub fn with_input(input: &[u8]) -> MockStream { - MockStream { - read: Cursor::new(input.to_vec()), - write: vec![], - read_timeout: Cell::new(None), - write_timeout: Cell::new(None), - } - } -} - -impl Read for MockStream { - fn read(&mut self, buf: &mut [u8]) -> io::Result { - self.read.read(buf) - } -} - -impl Write for MockStream { - fn write(&mut self, msg: &[u8]) -> io::Result { - Write::write(&mut self.write, msg) - } - - fn flush(&mut self) -> io::Result<()> { - Ok(()) - } -} - -impl NetworkStream for MockStream { - fn peer_addr(&mut self) -> io::Result { - Ok("127.0.0.1:1337".parse().unwrap()) - } - - #[cfg(feature = "timeouts")] - fn set_read_timeout(&self, dur: Option) -> io::Result<()> { - self.read_timeout.set(dur); - Ok(()) - } - - #[cfg(feature = "timeouts")] - fn set_write_timeout(&self, dur: Option) -> io::Result<()> { - self.write_timeout.set(dur); - Ok(()) - } -} - -/// A wrapper around a `MockStream` that allows one to clone it and keep an independent copy to the -/// same underlying stream. -#[derive(Clone)] -pub struct CloneableMockStream { - pub inner: Arc>, -} - -impl Write for CloneableMockStream { - fn write(&mut self, msg: &[u8]) -> io::Result { - self.inner.lock().unwrap().write(msg) - } - - fn flush(&mut self) -> io::Result<()> { - self.inner.lock().unwrap().flush() - } -} - -impl Read for CloneableMockStream { - fn read(&mut self, buf: &mut [u8]) -> io::Result { - self.inner.lock().unwrap().read(buf) - } -} - -impl NetworkStream for CloneableMockStream { - fn peer_addr(&mut self) -> io::Result { - self.inner.lock().unwrap().peer_addr() - } - - #[cfg(feature = "timeouts")] - fn set_read_timeout(&self, dur: Option) -> io::Result<()> { - self.inner.lock().unwrap().set_read_timeout(dur) - } - - #[cfg(feature = "timeouts")] - fn set_write_timeout(&self, dur: Option) -> io::Result<()> { - self.inner.lock().unwrap().set_write_timeout(dur) - } -} - -impl CloneableMockStream { - pub fn with_stream(stream: MockStream) -> CloneableMockStream { - CloneableMockStream { - inner: Arc::new(Mutex::new(stream)), - } - } -} - -pub struct MockConnector; - -impl NetworkConnector for MockConnector { - type Stream = MockStream; - - fn connect(&self, _host: &str, _port: u16, _scheme: &str) -> hyper::Result { - Ok(MockStream::new()) + fn send(&self, _: Request) -> Result { + Err(LoadError::Connection(Url::parse("http://example.com").unwrap(), "shouldn't connect".to_string())) } } @@ -156,10 +28,10 @@ impl NetworkConnector for MockConnector { fn test_load_errors_when_scheme_is_not_http_or_https() { let url = Url::parse("ftp://not-supported").unwrap(); let (cookies_chan, _) = ipc::channel().unwrap(); - let load_data = LoadData::new(url, None); + let load_data = LoadData::new(url.clone(), None); let hsts_list = Arc::new(Mutex::new(HSTSList { entries: Vec::new() })); - match load(load_data, cookies_chan, None, hsts_list, &MockConnector) { + match load(load_data, cookies_chan, None, hsts_list, &MockHttpRequester) { Err(LoadError::UnsupportedScheme(_)) => {} _ => panic!("expected ftp scheme to be unsupported") } @@ -169,10 +41,10 @@ fn test_load_errors_when_scheme_is_not_http_or_https() { fn test_load_errors_when_viewing_source_and_inner_url_scheme_is_not_http_or_https() { let url = Url::parse("view-source:ftp://not-supported").unwrap(); let (cookies_chan, _) = ipc::channel().unwrap(); - let load_data = LoadData::new(url, None); + let load_data = LoadData::new(url.clone(), None); let hsts_list = Arc::new(Mutex::new(HSTSList { entries: Vec::new() })); - match load(load_data, cookies_chan, None, hsts_list, &MockConnector) { + match load(load_data, cookies_chan, None, hsts_list, &MockHttpRequester) { Err(LoadError::UnsupportedScheme(_)) => {} _ => panic!("expected ftp scheme to be unsupported") }