From 7633cd54c2c5d75109043a305c1ce325fdb74a16 Mon Sep 17 00:00:00 2001 From: Sam Gibson Date: Sun, 9 Aug 2015 14:42:31 +1000 Subject: [PATCH] Abstract everything but the response from hyper Because we're using unsized types not for requesting, there's not a satisfactory way of doing this without boxing the request... Once unsized stuff lands in rust 1.2/1.3(???) then this should be implemented with Rc's instead of Box's. For the time being I'm not sure what else to do. servo/servo#6727 --- components/net/http_loader.rs | 92 ++++++++++++++++++++--------------- tests/unit/net/http_loader.rs | 9 ++-- 2 files changed, 57 insertions(+), 44 deletions(-) diff --git a/components/net/http_loader.rs b/components/net/http_loader.rs index 5f3a1ebe330..190ae0f98a7 100644 --- a/components/net/http_loader.rs +++ b/components/net/http_loader.rs @@ -23,11 +23,11 @@ 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::header::{Quality, QualityItem, Headers}; use hyper::Error as HttpError; use hyper::method::Method; use hyper::mime::{Mime, TopLevel, SubLevel}; -use hyper::net::{Fresh, HttpsConnector, Openssl, NetworkConnector, NetworkStream}; +use hyper::net::{Fresh, Streaming, HttpsConnector, Openssl, NetworkConnector, NetworkStream}; use hyper::status::{StatusCode, StatusClass}; use ipc_channel::ipc::{self, IpcSender}; use log; @@ -164,20 +164,52 @@ impl NetworkHttpRequester { } } -impl HttpRequester for NetworkHttpRequester { - fn send(&self, request: Request) -> Result { - unimplemented!() +pub trait HttpRequest { + fn headers(&self) -> &Headers; + fn headers_mut(&mut self) -> &mut Headers; + fn send(self: Box) -> Result; +} + +struct NetworkHttpRequest { + fresh: Request +} + +impl HttpRequest for NetworkHttpRequest { + fn headers(&self) -> &Headers { + self.fresh.headers() } - fn build(&self, url: Url, method: Method) -> Result, LoadError> { + fn headers_mut(&mut self) -> &mut Headers { + self.fresh.headers_mut() + } + + fn send(self: Box) -> Result { + let connected = match self.fresh.start() { + Ok(streaming) => streaming, + Err(e) => return Err(LoadError::Connection(Url::parse("http://example.com").unwrap(), e.description().to_string())) + }; + + match connected.send() { + Ok(w) => Ok(w), + Err(e) => return Err(LoadError::Connection(Url::parse("http://example.com").unwrap(), e.description().to_string())) + } + } +} + +impl HttpRequester for NetworkHttpRequester { + fn send(&self, request: Box) -> Result { + request.send() + } + + 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), + let request = match connection { + Ok(req) => req, Err(HttpError::Io(ref io_error)) if ( io_error.kind() == io::ErrorKind::Other && @@ -185,7 +217,7 @@ impl HttpRequester for NetworkHttpRequester { // FIXME: This incredibly hacky. Make it more robust, and at least test it. format!("{:?}", io_error.cause()) == ssl_err_string ) => { - Err( + return Err( LoadError::Ssl( url.clone(), format!("ssl error {:?}: {:?} {:?}", io_error.kind(), io_error.description(), io_error.cause()) @@ -193,15 +225,17 @@ impl HttpRequester for NetworkHttpRequester { ) }, Err(e) => { - Err(LoadError::Connection(url, e.description().to_string())) + return Err(LoadError::Connection(url, e.description().to_string())) } - } + }; + + Ok(Box::new(NetworkHttpRequest { fresh: request })) } } pub trait HttpRequester { - fn build(&self, url: Url, method: Method) -> Result, LoadError>; - fn send(&self, request: Request) -> Result; + fn build(&self, url: Url, method: Method) -> Result, LoadError>; + fn send(&self, request: Box) -> Result; } pub enum LoadError { @@ -321,35 +355,23 @@ pub fn load(mut load_data: LoadData, // --- Start sending the request // Avoid automatically sending request body if a redirect has occurred. - let writer = match load_data.data { + let response = match load_data.data { Some(ref data) if iters == 1 => { req.headers_mut().set(ContentLength(data.len() as u64)); - let mut writer = match req.start() { + match requester.send(req) { Ok(w) => w, - Err(e) => { - return Err(LoadError::Connection(url, e.description().to_string())); - } - }; - - match writer.write_all(&*data) { - Err(e) => { - return Err(LoadError::Connection(url, e.description().to_string())); - } - _ => {} - }; - writer + Err(e) => return Err(e) + } }, _ => { match load_data.method { Method::Get | Method::Head => (), _ => req.headers_mut().set(ContentLength(0)) } - match req.start() { + match requester.send(req) { Ok(w) => w, - Err(e) => { - return Err(LoadError::Connection(url, e.description().to_string())); - } + Err(e) => return Err(e) } } }; @@ -368,14 +390,6 @@ pub fn load(mut load_data: LoadData, net_event))).unwrap(); } - // --- Finish writing the request and read the response - let response = match writer.send() { - Ok(r) => r, - Err(e) => { - return Err(LoadError::Connection(url, e.description().to_string())); - } - }; - // Dump headers, but only do the iteration if info!() is enabled. info!("got HTTP response {}, headers:", response.status); if log_enabled!(log::LogLevel::Info) { diff --git a/tests/unit/net/http_loader.rs b/tests/unit/net/http_loader.rs index 2c05aadb52d..863700eef53 100644 --- a/tests/unit/net/http_loader.rs +++ b/tests/unit/net/http_loader.rs @@ -2,24 +2,23 @@ * 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}; +use net::http_loader::{load, LoadError, HttpRequester, HttpRequest}; use url::Url; use std::sync::{Arc, Mutex}; use ipc_channel::ipc; use net_traits::LoadData; use net::hsts::HSTSList; -use hyper::client::{Request, Response}; -use hyper::net::Fresh; +use hyper::client::Response; use hyper::method::Method; struct MockHttpRequester; impl HttpRequester for MockHttpRequester { - fn build(&self, url: Url, _: Method) -> Result, LoadError> { + fn build(&self, url: Url, _: Method) -> Result, LoadError> { Err(LoadError::Connection(url.clone(), "shouldn't connect".to_string())) } - fn send(&self, _: Request) -> Result { + fn send(&self, _: Box) -> Result { Err(LoadError::Connection(Url::parse("http://example.com").unwrap(), "shouldn't connect".to_string())) } }