diff --git a/components/net/http_loader.rs b/components/net/http_loader.rs index e6c4a482cd8..9cf6f15c4d2 100644 --- a/components/net/http_loader.rs +++ b/components/net/http_loader.rs @@ -28,7 +28,7 @@ use hyper::Error as HttpError; use hyper::method::Method; use hyper::http::RawStatus; use hyper::mime::{Mime, TopLevel, SubLevel}; -use hyper::net::{Fresh, Streaming, HttpsConnector, Openssl, NetworkConnector, NetworkStream}; +use hyper::net::{Fresh, HttpsConnector, Openssl}; use hyper::status::{StatusCode, StatusClass}; use ipc_channel::ipc::{self, IpcSender}; use log; @@ -110,9 +110,7 @@ fn load_for_consumer(load_data: LoadData, devtools_chan: Option>, hsts_list: Arc>) { - let requester = NetworkHttpRequester::new(); - - match load(load_data, resource_mgr_chan, devtools_chan, hsts_list, &requester) { + match load::(load_data, resource_mgr_chan, devtools_chan, hsts_list, &NetworkHttpRequestFactory) { Err(LoadError::UnsupportedScheme(url)) => { let s = format!("{} request, but we don't support that scheme", &*url.scheme); send_error(url, s, start_chan) @@ -144,27 +142,6 @@ fn load_for_consumer(load_data: LoadData, } } -struct NetworkHttpRequester { - connector: HttpsConnector -} - -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(); - - NetworkHttpRequester { - connector: HttpsConnector::new(Openssl { context: Arc::new(context) }) - } - } -} - pub trait HttpResponse: Read { fn headers(&self) -> &Headers; fn status(&self) -> StatusCode; @@ -196,16 +173,31 @@ impl HttpResponse for WrappedHttpResponse { } } -impl NetworkHttpRequester { - fn start_connection(&self, url: &Url, method: &Method) -> Result, LoadError> { - let connection = Request::with_connector(method.clone(), url.clone(), &self.connector); +pub trait HttpRequestFactory { + type R: HttpRequest; + + fn create(&self, url: Url, method: Method) -> Result; +} + +struct NetworkHttpRequestFactory; + +impl HttpRequestFactory for NetworkHttpRequestFactory { + type R = WrappedHttpRequest; + + fn create(&self, url: Url, method: Method) -> Result { + 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 connector = HttpsConnector::new(Openssl { context: Arc::new(context) }); + let connection = Request::with_connector(method.clone(), url.clone(), &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), + let request = match connection { + Ok(req) => req, Err(HttpError::Io(ref io_error)) if ( io_error.kind() == io::ErrorKind::Other && @@ -223,28 +215,38 @@ impl NetworkHttpRequester { Err(e) => { return Err(LoadError::Connection(url.clone(), e.description().to_string())) } - } + }; + + Ok(WrappedHttpRequest { request: request }) } } -impl HttpRequester for NetworkHttpRequester { - fn send(&self, url: &Url, method: &Method, headers: &Headers, body: &Option>) -> Result, LoadError> { - let mut request = try!(self.start_connection(url, method)); +pub trait HttpRequest { + type R: HttpResponse + 'static; - *request.headers_mut() = headers.clone(); + fn headers_mut(&mut self) -> &mut Headers; + fn send(self, body: &Option>) -> Result; +} - // TODO: fix HEAD method (don't write the body, and force content-length to 0) - if let Some(ref data) = *body { - request.headers_mut().set(ContentLength(data.len() as u64)); - } +struct WrappedHttpRequest { + request: Request +} - let mut request_writer = match request.start() { +impl HttpRequest for WrappedHttpRequest { + type R = WrappedHttpResponse; + + fn headers_mut(&mut self) -> &mut Headers { + self.request.headers_mut() + } + + fn send(self, body: &Option>) -> Result { + let mut request_writer = match self.request.start() { Ok(streaming) => streaming, Err(e) => return Err(LoadError::Connection(Url::parse("http://example.com").unwrap(), e.description().to_string())) }; if let Some(ref data) = *body { - match request_writer.write_all(&*data) { + match request_writer.write_all(&data) { Err(e) => { return Err(LoadError::Connection(Url::parse("http://example.com").unwrap(), e.description().to_string())) } @@ -257,13 +259,8 @@ impl HttpRequester for NetworkHttpRequester { Err(e) => return Err(LoadError::Connection(Url::parse("http://example.com").unwrap(), e.description().to_string())) }; - Ok(Box::new(WrappedHttpResponse { response: response })) + Ok(WrappedHttpResponse { response: response }) } - -} - -pub trait HttpRequester { - fn send(&self, url: &Url, method: &Method, headers: &Headers, body: &Option>) -> Result, LoadError>; } #[derive(Debug)] @@ -277,12 +274,12 @@ pub enum LoadError { MaxRedirects(Url) } -pub fn load(mut load_data: LoadData, +pub fn load(mut load_data: LoadData, resource_mgr_chan: IpcSender, devtools_chan: Option>, hsts_list: Arc>, - requester: &HttpRequester) - -> Result<(Box, Metadata), LoadError> { + request_factory: &HttpRequestFactory) + -> Result<(Box, Metadata), LoadError> where A: HttpRequest + 'static { // 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. @@ -374,17 +371,24 @@ pub fn load(mut load_data: LoadData, request_headers.set_raw("Accept-Encoding".to_owned(), vec![b"gzip, deflate".to_vec()]); } + // --- Start sending the request + // TODO: Avoid automatically sending request body if a redirect has occurred. + let mut req = try!(request_factory.create(url.clone(), load_data.method.clone())); + *req.headers_mut() = request_headers; + if log_enabled!(log::LogLevel::Info) { info!("{}", load_data.method); - for header in request_headers.iter() { + for header in req.headers_mut().iter() { info!(" - {}", header); } info!("{:?}", load_data.data); } - // --- Start sending the request - // TODO: Avoid automatically sending request body if a redirect has occurred. - let response = try!(requester.send(&url, &load_data.method, &request_headers, &load_data.data)); + if let Some(ref data) = load_data.data { + req.headers_mut().set(ContentLength(data.len() as u64)); + } + + let response = try!(req.send(&load_data.data)); // --- Tell devtools we've made a request // Send an HttpRequest message to devtools with a unique request_id @@ -401,7 +405,7 @@ pub fn load(mut load_data: LoadData, } // Dump headers, but only do the iteration if info!() is enabled. - info!("got HTTP response {}, headers:", (*response).status()); + info!("got HTTP response {}, headers:", response.status()); if log_enabled!(log::LogLevel::Info) { for header in response.headers().iter() { info!(" - {}", header); diff --git a/tests/unit/net/http_loader.rs b/tests/unit/net/http_loader.rs index 067ed89f83d..876aed61d81 100644 --- a/tests/unit/net/http_loader.rs +++ b/tests/unit/net/http_loader.rs @@ -2,85 +2,30 @@ * 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, HttpRequester, HttpResponse}; +use net::http_loader::{load, LoadError, HttpRequestFactory, HttpRequest, HttpResponse}; use net::resource_task::new_resource_task; use url::Url; use std::sync::{Arc, Mutex}; use ipc_channel::ipc; use net_traits::LoadData; use net::hsts::HSTSList; -use hyper::client::Response; use hyper::method::Method; use hyper::http::RawStatus; use hyper::status::StatusCode; use hyper::header::{Headers, Location, ContentLength}; use std::io::{self, Read}; use std::cmp::{self}; -use std::mem::{self}; use std::borrow::Cow; -fn redirect_to(host: &str) -> Box { - let mut headers = Headers::new(); - headers.set(Location(host.to_string())); - - struct Redirect { - h: Headers, - sr: RawStatus - } - - impl Read for Redirect { - fn read(&mut self, buf: &mut [u8]) -> io::Result { - Ok(0) - } - } - - impl HttpResponse for Redirect { - fn headers(&self) -> &Headers { &self.h } - fn status(&self) -> StatusCode { StatusCode::MovedPermanently } - fn status_raw(&self) -> &RawStatus { &self.sr } - } - - Box::new( - Redirect { - h: headers, - sr: RawStatus(301, Cow::Borrowed("Moved Permanently")) - } - ) -} - -fn respond_with(body: &str) -> Box { +fn respond_with(body: Vec) -> MockResponse { let mut headers = Headers::new(); headers.set(ContentLength(body.len() as u64)); - struct TextResponse { - h: Headers, - body: Vec, - sr: RawStatus - } - - impl Read for TextResponse { - fn read(&mut self, buf: &mut [u8]) -> io::Result { - let buf_len = buf.len(); - for (a, b) in buf.iter_mut().zip(&self.body[0 .. cmp::min(buf_len, self.body.len())]) { - *a = *b - } - - Ok(cmp::min(buf.len(), self.body.len())) - } - } - - impl HttpResponse for TextResponse { - fn headers(&self) -> &Headers { &self.h } - fn status(&self) -> StatusCode { StatusCode::Ok } - fn status_raw(&self) -> &RawStatus { &self.sr } - } - - Box::new( - TextResponse { - h: headers, - body: <[_]>::to_vec(body.as_bytes()), - sr: RawStatus(200, Cow::Borrowed("Ok")) - } + MockResponse::new( + headers, + StatusCode::Ok, + RawStatus(200, Cow::Borrowed("Ok")), + body ) } @@ -96,16 +41,94 @@ fn read_response(reader: &mut Read) -> String { } } +struct MockResponse { + h: Headers, + sc: StatusCode, + sr: RawStatus, + msg: Vec +} + +impl MockResponse { + fn new(h: Headers, sc: StatusCode, sr: RawStatus, msg: Vec) -> MockResponse { + MockResponse { h: h, sc: sc, sr: sr, msg: msg } + } +} + +impl Read for MockResponse { + fn read(&mut self, buf: &mut [u8]) -> io::Result { + let buf_len = buf.len(); + for (a, b) in buf.iter_mut().zip(&self.msg[0 .. cmp::min(buf_len, self.msg.len())]) { + *a = *b + } + + Ok(cmp::min(buf.len(), self.msg.len())) + } +} + +impl HttpResponse for MockResponse { + fn headers(&self) -> &Headers { &self.h } + fn status(&self) -> StatusCode { self.sc } + fn status_raw(&self) -> &RawStatus { &self.sr } +} + +fn redirect_to(host: String) -> MockResponse { + let mut headers = Headers::new(); + headers.set(Location(host.to_string())); + + MockResponse::new( + headers, + StatusCode::MovedPermanently, + RawStatus(301, Cow::Borrowed("Moved Permanently")), + <[_]>::to_vec("".as_bytes()) + ) +} + + +enum RequestType { + Redirect(String), + Text(Vec) +} + +struct MockRequest { + headers: Headers, + t: RequestType +} + +impl MockRequest { + fn new(t: RequestType) -> MockRequest { + MockRequest { headers: Headers::new(), t: t } + } +} + +impl HttpRequest for MockRequest { + type R=MockResponse; + + fn headers_mut(&mut self) -> &mut Headers { &mut self.headers } + + fn send(self, _: &Option>) -> Result { + match self.t { + RequestType::Redirect(location) => { + Ok(redirect_to(location)) + }, + RequestType::Text(b) => { + Ok(respond_with(b)) + } + } + } +} + #[test] fn test_load_errors_when_there_a_redirect_loop() { - struct Redirector; + struct Factory; - impl HttpRequester for Redirector { - fn send(&self, url: &Url, _: &Method, _: &Headers, _: &Option>) -> Result, LoadError> { + impl HttpRequestFactory for Factory { + type R=MockRequest; + + fn create(&self, url: Url, _: Method) -> Result { if url.domain().unwrap() == "mozilla.com" { - Ok(redirect_to("http://mozilla.org")) + Ok(MockRequest::new(RequestType::Redirect("http://mozilla.org".to_string()))) } else if url.domain().unwrap() == "mozilla.org" { - Ok(redirect_to("http://mozilla.com")) + Ok(MockRequest::new(RequestType::Redirect("http://mozilla.com".to_string()))) } else { panic!("unexpected host {:?}", url) } @@ -117,7 +140,7 @@ fn test_load_errors_when_there_a_redirect_loop() { let load_data = LoadData::new(url.clone(), None); let hsts_list = Arc::new(Mutex::new(HSTSList { entries: Vec::new() })); - match load(load_data, resource_mgr, None, hsts_list, &Redirector) { + match load::(load_data, resource_mgr, None, hsts_list, &Factory) { Err(LoadError::InvalidRedirect(_, msg)) => { assert_eq!(msg, "redirect loop"); }, @@ -127,12 +150,14 @@ fn test_load_errors_when_there_a_redirect_loop() { #[test] fn test_load_errors_when_there_is_too_many_redirects() { - struct Redirector; + struct Factory; - impl HttpRequester for Redirector { - fn send(&self, url: &Url, _: &Method, _: &Headers, _: &Option>) -> Result, LoadError> { + impl HttpRequestFactory for Factory { + type R=MockRequest; + + fn create(&self, url: Url, _: Method) -> Result { if url.domain().unwrap() == "mozilla.com" { - Ok(redirect_to(&*format!("{}/1", url.serialize()))) + Ok(MockRequest::new(RequestType::Redirect(format!("{}/1", url.serialize())))) } else { panic!("unexpected host {:?}", url) } @@ -144,7 +169,7 @@ fn test_load_errors_when_there_is_too_many_redirects() { let load_data = LoadData::new(url.clone(), None); let hsts_list = Arc::new(Mutex::new(HSTSList { entries: Vec::new() })); - match load(load_data, resource_mgr, None, hsts_list, &Redirector) { + match load::(load_data, resource_mgr, None, hsts_list, &Factory) { Err(LoadError::MaxRedirects(url)) => { assert_eq!(url.domain().unwrap(), "mozilla.com") }, @@ -154,16 +179,24 @@ fn test_load_errors_when_there_is_too_many_redirects() { #[test] fn test_load_follows_a_redirect() { - struct Redirector; + struct Factory; - impl HttpRequester for Redirector { - fn send(&self, url: &Url, _: &Method, _: &Headers, _: &Option>) -> Result, LoadError> { + impl HttpRequestFactory for Factory { + type R=MockRequest; + + fn create(&self, url: Url, _: Method) -> Result { if url.domain().unwrap() == "mozilla.com" { - Ok(redirect_to("http://mozilla.org")) + Ok(MockRequest::new(RequestType::Redirect("http://mozilla.org".to_string()))) } else if url.domain().unwrap() == "mozilla.org" { - Ok(respond_with("Yay!")) + Ok( + MockRequest::new( + RequestType::Text( + <[_]>::to_vec("Yay!".as_bytes()) + ) + ) + ) } else { - panic!("unexpected host") + panic!("unexpected host {:?}", url) } } } @@ -173,20 +206,22 @@ fn test_load_follows_a_redirect() { let load_data = LoadData::new(url.clone(), None); let hsts_list = Arc::new(Mutex::new(HSTSList { entries: Vec::new() })); - match load(load_data, resource_mgr, None, hsts_list, &Redirector) { + match load::(load_data, resource_mgr, None, hsts_list, &Factory) { Err(_) => panic!("expected to follow a redirect"), - Ok((mut r, m)) => { + Ok((mut r, _)) => { let response = read_response(&mut *r); assert_eq!(response, "Yay!".to_string()); } } } -struct DontConnectHttpRequester; +struct DontConnectFactory; -impl HttpRequester for DontConnectHttpRequester { - fn send(&self, _: &Url, _: &Method, _: &Headers, _: &Option>) -> Result, LoadError> { - Err(LoadError::Connection(Url::parse("http://example.com").unwrap(), "shouldn't connect".to_string())) +impl HttpRequestFactory for DontConnectFactory { + type R=MockRequest; + + fn create(&self, url: Url, _: Method) -> Result { + Err(LoadError::Connection(url, "should not have connected".to_string())) } } @@ -197,7 +232,7 @@ fn test_load_errors_when_scheme_is_not_http_or_https() { 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, &DontConnectHttpRequester) { + match load::(load_data, cookies_chan, None, hsts_list, &DontConnectFactory) { Err(LoadError::UnsupportedScheme(_)) => {} _ => panic!("expected ftp scheme to be unsupported") } @@ -210,7 +245,7 @@ fn test_load_errors_when_viewing_source_and_inner_url_scheme_is_not_http_or_http 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, &DontConnectHttpRequester) { + match load::(load_data, cookies_chan, None, hsts_list, &DontConnectFactory) { Err(LoadError::UnsupportedScheme(_)) => {} _ => panic!("expected ftp scheme to be unsupported") }