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

@@ -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 {
+pub trait HttpRequest {
-    fn send(&self, request: Request<Fresh>) -> Result<Response, LoadError> {
+    fn headers(&self) -> &Headers;
-        unimplemented!()
+    fn headers_mut(&mut self) -> &mut Headers;
+    fn send(self: Box<Self>) -> Result<Response, LoadError>;
+}
+
+struct NetworkHttpRequest {
+    fresh: Request<Fresh>
+}
+
+impl HttpRequest for NetworkHttpRequest {
+    fn headers(&self) -> &Headers {
+        self.fresh.headers()
     }
 
-    fn build(&self, url: Url, method: Method) -> Result<Request<Fresh>, LoadError> {
+    fn headers_mut(&mut self) -> &mut Headers {
+        self.fresh.headers_mut()
+    }
+
+    fn send(self: Box<Self>) -> Result<Response, LoadError> {
+        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<HttpRequest>) -> Result<Response, LoadError> {
+        request.send()
+    }
+
+    fn build(&self, url: Url, method: Method) -> Result<Box<HttpRequest>, 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 {
+        let request = match connection {
-            Ok(req) => Ok(req),
+            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<Request<Fresh>, LoadError>;
+    fn build(&self, url: Url, method: Method) -> Result<Box<HttpRequest>, LoadError>;
-    fn send(&self, request: Request<Fresh>) -> Result<Response, LoadError>;
+    fn send(&self, request: Box<HttpRequest>) -> Result<Response, LoadError>;
 }
 
 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) => {
+                    Err(e) => return 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
             },
             _ => {
                 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) => {
+                    Err(e) => return Err(e)
-                        return Err(LoadError::Connection(url, e.description().to_string()));
-                    }
                 }
             }
         };
@@ -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) {

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::client::Response;
-use hyper::net::Fresh;
 use hyper::method::Method;
 
 struct MockHttpRequester;
 
 impl HttpRequester for MockHttpRequester {
-    fn build(&self, url: Url, _: Method) -> Result<Request<Fresh>, LoadError> {
+    fn build(&self, url: Url, _: Method) -> Result<Box<HttpRequest>, LoadError> {
         Err(LoadError::Connection(url.clone(), "shouldn't connect".to_string()))
     }
 
-    fn send(&self, _: Request<Fresh>) -> Result<Response, LoadError> {
+    fn send(&self, _: Box<HttpRequest>) -> Result<Response, LoadError> {
         Err(LoadError::Connection(Url::parse("http://example.com").unwrap(), "shouldn't connect".to_string()))
     }
 }