Auto merge of #9321 - nikkisquared:response_body, r=jdm

Test setting response.body and fetching a message on a server

I've updated http_fetch to now set response.body, as well as written a test to ensure that fetch can retrieve a message on a server. I've also looked into partially implementing some more of http_fetch while trying to figure out where response.body gets written to.

As always I'd like feedback on my logic, I'm confident there are more steps for handling response.body I need but I find the specification difficult to parse on this.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9321)
<!-- Reviewable:end -->

components/net/fetch/methods.rs

@@ -17,10 +17,12 @@ use hyper::mime::{Attr, Mime, SubLevel, TopLevel, Value};
 use hyper::status::StatusCode;
 use net_traits::request::{CacheMode, Context, ContextFrameType, CredentialsMode};
 use net_traits::request::{RedirectMode, Referer, Request, RequestMode, ResponseTainting};
-use net_traits::response::{CacheState, HttpsState, Response, ResponseType, TerminationReason};
+use net_traits::response::{CacheState, HttpsState, TerminationReason};
+use net_traits::response::{Response, ResponseBody, ResponseType};
 use net_traits::{AsyncFetchListener, Metadata};
 use resource_thread::CancellationListener;
 use std::ascii::AsciiExt;
+use std::io::Read;
 use std::rc::Rc;
 use std::str::FromStr;
 use std::thread;
@@ -163,7 +165,8 @@ fn http_fetch(request: Rc<Request>,
     // Step 3
     if !request.skip_service_worker.get() && !request.is_service_worker_global_scope {
 
-        // TODO: Substep 1 (handle fetch unimplemented)
+        // Substep 1
+        // TODO (handle fetch unimplemented)
 
         if let Some(ref res) = response {
 
@@ -617,11 +620,15 @@ fn http_network_fetch(request: Rc<Request>,
 
     let mut response = Response::new();
     match wrapped_response {
-        Ok(res) => {
+        Ok(mut res) => {
             // is it okay for res.version to be unused?
             response.url = Some(res.response.url.clone());
             response.status = Some(res.response.status);
             response.headers = res.response.headers.clone();
+
+            let mut body = vec![];
+            res.response.read_to_end(&mut body);
+            response.body = ResponseBody::Done(body);
         },
         Err(e) =>
             response.termination_reason = Some(TerminationReason::Fatal)
@@ -633,7 +640,7 @@ fn http_network_fetch(request: Rc<Request>,
         // Substep 2
 
     // TODO how can I tell if response was retrieved over HTTPS?
-    // TODO: Servo needs to decide what ciphers are to be treated as "deprecated"
+    // TODO Servo needs to decide what ciphers are to be treated as "deprecated"
     response.https_state = HttpsState::None;
 
     // TODO how do I read request?
@@ -656,9 +663,14 @@ fn http_network_fetch(request: Rc<Request>,
     *response.url_list.borrow_mut() = request.url_list.borrow().clone();
 
     // Step 7
+    // TODO this step isn't possible yet
 
     // Step 8
+    if Response::is_network_error(&response) && request.cache_mode.get() == CacheMode::NoStore {
+        // TODO update response in the HTTP cache for request
+    }
 
+    // TODO these steps aren't possible yet
     // Step 9
         // Substep 1
         // Substep 2

components/net/fetch/response.rs

@@ -75,8 +75,7 @@ impl ResponseMethods for Response {
                 response.response_type = filter_type;
             },
 
-            ResponseType::Opaque |
-            ResponseType::OpaqueRedirect => {
+            ResponseType::Opaque | ResponseType::OpaqueRedirect => {
                 response.headers = Headers::new();
                 response.status = None;
                 response.body = ResponseBody::Empty;

tests/unit/net/fetch.rs

@@ -6,7 +6,7 @@ use hyper::server::{Listening, Server};
 use hyper::server::{Request as HyperRequest, Response as HyperResponse};
 use net::fetch::methods::fetch;
 use net_traits::request::{Context, Referer, Request};
-use net_traits::response::Response;
+use net_traits::response::{Response, ResponseBody};
 use std::rc::Rc;
 use url::Url;
 
@@ -42,3 +42,24 @@ fn test_fetch_response_is_not_network_error() {
         panic!("fetch response shouldn't be a network error");
     }
 }
+
+#[test]
+fn test_fetch_response_body_matches_const_message() {
+
+    static MESSAGE: &'static [u8] = b"Hello World!";
+    let (mut server, url) = make_server(MESSAGE);
+
+    let mut request = Request::new(url, Context::Fetch, false);
+    request.referer = Referer::NoReferer;
+    let wrapped_request = Rc::new(request);
+
+    let fetch_response = fetch(wrapped_request, false);
+    let _ = server.close();
+
+    match fetch_response.body {
+        ResponseBody::Done(body) => {
+            assert_eq!(body, MESSAGE);
+        },
+        _ => { panic!() }
+    };
+}