Auto merge of #9525 - nikkisquared:test_filtered_responses, r=asajeffrey

Test filtered responses and implement Cors Check Fetch step

I've been writing tests for creating filtered responses. So far I have three of the four types being made (namely, Basic, CORS, and Opaque), and just need to figure out how to make an OpaqueRedirect filtered response, since it's handled separately from the others. I will also add more tests to ensure the content of the filtered responses matches the limitations placed by the specification.

Along the way I implemented Cors Check, since it's required for the CORS filtered response. @jdm suggested I handle it in here, since it's such a small step, compared to other parts of Fetch.

Since all the tests currently pass, and I've spent a while adding the Cors Check and other pieces, I figured now would be a good time to start having it reviewed.

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

components/net/fetch/methods.rs

@@ -9,6 +9,7 @@ use http_loader::{create_http_connector, obtain_response};
 use hyper::client::response::Response as HyperResponse;
 use hyper::header::{Accept, IfMatch, IfRange, IfUnmodifiedSince, Location};
 use hyper::header::{AcceptLanguage, ContentLength, ContentLanguage, HeaderView};
+use hyper::header::{AccessControlAllowCredentials, AccessControlAllowOrigin};
 use hyper::header::{Authorization, Basic, ContentEncoding, Encoding};
 use hyper::header::{ContentType, Header, Headers, IfModifiedSince, IfNoneMatch};
 use hyper::header::{QualityItem, q, qitem, Referer as RefererHeader, UserAgent};
@@ -27,7 +28,8 @@ use std::io::Read;
 use std::rc::Rc;
 use std::str::FromStr;
 use std::thread;
-use url::{Origin, Url, UrlParser};
+use url::idna::domain_to_ascii;
+use url::{Origin, Url, UrlParser, whatwg_scheme_type_mapper};
 use util::thread::spawn_named;
 
 pub fn fetch_async(request: Request, cors_flag: bool, listener: Box<AsyncFetchListener + Send>) {
@@ -729,12 +731,12 @@ fn http_network_or_cache_fetch(request: Rc<Request>,
 
             // Substep 1
             // TODO this substep
-            let cached_response: Option<Response> = None;
+            // let cached_response: Option<Response> = None;
 
             // Substep 2
-            if cached_response.is_none() {
+            // if cached_response.is_none() {
-                return Response::network_error();
+            //     return Response::network_error();
-            }
+            // }
 
             // Substep 3
 
@@ -860,10 +862,88 @@ fn preflight_fetch(request: Rc<Request>) -> Response {
 
 /// [CORS check](https://fetch.spec.whatwg.org#concept-cors-check)
 fn cors_check(request: Rc<Request>, response: &Response) -> Result<(), ()> {
-    // TODO: Implement CORS check spec
+
+    // Step 1
+    // let headers = request.headers.borrow();
+    let origin = response.headers.get::<AccessControlAllowOrigin>().cloned();
+
+    // Step 2
+    let origin = try!(origin.ok_or(()));
+
+    // Step 3
+    if request.credentials_mode != CredentialsMode::Include &&
+        origin == AccessControlAllowOrigin::Any {
+        return Ok(());
+    }
+
+    // Step 4
+    let origin = match origin {
+        AccessControlAllowOrigin::Value(origin) => origin,
+        // if it's Any or Null at this point, I see nothing to do but return Err(())
+        _ => return Err(())
+    };
+
+    // strings are already utf-8 encoded, so I don't need to re-encode origin for this step
+    match ascii_serialise_origin(&request.origin) {
+        Ok(request_origin) => {
+            if request_origin != origin {
+                return Err(());
+            }
+        },
+        _ => return Err(())
+    }
+
+    // Step 5
+    if request.credentials_mode != CredentialsMode::Include {
+        return Ok(());
+    }
+
+    // Step 6
+    let credentials = request.headers.borrow().get::<AccessControlAllowCredentials>().cloned();
+
+    // Step 7
+    if credentials.is_some() {
+        return Ok(());
+    }
+
+    // Step 8
     Err(())
 }
 
+/// [ASCII serialisation of an origin](https://html.spec.whatwg.org/multipage/#ascii-serialisation-of-an-origin)
+fn ascii_serialise_origin(origin: &Origin) -> Result<String, ()> {
+
+    let result = match *origin {
+
+        // Step 1
+        Origin::UID(_) => "null".to_owned(),
+
+        // Step 2
+        Origin::Tuple(ref scheme, ref host, ref port) => {
+
+            // Step 3
+            // this step is handled by the format!()s later in the function
+
+            // Step 4
+            // TODO throw a SecurityError in a meaningful way
+            // let host = host.as_str();
+            let host = try!(domain_to_ascii(host.serialize().as_str()).or(Err(())));
+
+            // Step 5
+            let default_port = whatwg_scheme_type_mapper(scheme).default_port();
+
+            if Some(*port) == default_port {
+                format!("{}://{}", scheme, host)
+            } else {
+                format!("{}://{}{}", scheme, host, port)
+            }
+        }
+    };
+
+    // Step 6
+    Ok(result)
+}
+
 fn global_user_agent() -> String {
     // TODO have a better useragent string
     const USER_AGENT_STRING: &'static str = "Servo";

components/net/fetch/response.rs

@@ -69,8 +69,8 @@ impl ResponseMethods for Response {
 
                 let headers = old_headers.iter().filter(|header| {
                     match &*header.name().to_ascii_lowercase() {
-                        "cache-control" | "content-language" |
+                        "cache-control" | "content-language" | "content-type" |
-                        "content-type" | "expires" | "last-modified" | "Pragma" => true,
+                        "expires" | "last-modified" | "pragma" => true,
                         "set-cookie" | "set-cookie2" => false,
                         header => {
                             let result =
@@ -88,12 +88,14 @@ impl ResponseMethods for Response {
                 response.headers = Headers::new();
                 response.status = None;
                 response.body = RefCell::new(ResponseBody::Empty);
+                response.cache_state = CacheState::None;
             },
 
             ResponseType::OpaqueRedirect => {
                 response.headers = Headers::new();
                 response.status = None;
                 response.body = RefCell::new(ResponseBody::Empty);
+                response.cache_state = CacheState::None;
             }
         }
 

components/net_traits/response.rs

@@ -31,7 +31,7 @@ pub enum TerminationReason {
 
 /// The response body can still be pushed to after fetch
 /// This provides a way to store unfinished response bodies
-#[derive(Clone)]
+#[derive(Clone, Debug)]
 pub enum ResponseBody {
     Empty, // XXXManishearth is this necessary, or is Done(vec![]) enough?
     Receiving(Vec<u8>),
@@ -39,7 +39,7 @@ pub enum ResponseBody {
 }
 
 /// [Cache state](https://fetch.spec.whatwg.org/#concept-response-cache-state)
-#[derive(Clone)]
+#[derive(Clone, Debug)]
 pub enum CacheState {
     None,
     Local,

components/servo/Cargo.lock

@@ -1247,6 +1247,7 @@ dependencies = [
  "net_traits 0.0.1",
  "plugins 0.0.1",
  "time 0.1.34 (registry+https://github.com/rust-lang/crates.io-index)",
+ "unicase 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "url 0.5.4 (registry+https://github.com/rust-lang/crates.io-index)",
  "util 0.0.1",
 ]

tests/unit/net/Cargo.toml

@@ -35,3 +35,4 @@ hyper = "0.7"
 url = {version = "0.5.4", features = ["heap_size"]}
 time = "0.1"
 flate2 = "0.2.0"
+unicase = "1.0"

tests/unit/net/fetch.rs

@@ -2,16 +2,21 @@
  * 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 hyper::header::{Location};
+use hyper::header::{AccessControlAllowHeaders, AccessControlAllowOrigin};
+use hyper::header::{CacheControl, ContentLanguage, ContentType, Expires, LastModified};
+use hyper::header::{Headers, HttpDate, Location, SetCookie, Pragma};
 use hyper::server::{Handler, Listening, Server};
 use hyper::server::{Request as HyperRequest, Response as HyperResponse};
 use hyper::status::StatusCode;
 use hyper::uri::RequestUri;
 use net::fetch::methods::fetch;
-use net_traits::request::{Context, Referer, Request};
+use net_traits::request::{Context, RedirectMode, Referer, Request, RequestMode};
-use net_traits::response::{Response, ResponseBody, ResponseType};
+use net_traits::response::{CacheState, Response, ResponseBody, ResponseType};
+use std::cell::Cell;
 use std::rc::Rc;
-use url::Url;
+use time::{self, Duration};
+use unicase::UniCase;
+use url::{Origin, OpaqueOrigin, Url};
 
 // TODO write a struct that impls Handler for storing test values
 
@@ -76,6 +81,180 @@ fn test_fetch_response_body_matches_const_message() {
     };
 }
 
+#[test]
+fn test_fetch_response_is_basic_filtered() {
+
+    static MESSAGE: &'static [u8] = b"";
+    let handler = move |_: HyperRequest, mut response: HyperResponse| {
+
+        response.headers_mut().set(SetCookie(vec![]));
+        // this header is obsoleted, so hyper doesn't implement it, but it's still covered by the spec
+        response.headers_mut().set_raw("Set-Cookie2", vec![]);
+
+        response.send(MESSAGE).unwrap();
+    };
+    let (mut server, url) = make_server(handler);
+
+    let origin = url.origin();
+    let mut request = Request::new(url, Context::Fetch, origin, false);
+    request.referer = Referer::NoReferer;
+    let wrapped_request = Rc::new(request);
+
+    let fetch_response = fetch(wrapped_request, false);
+    let _ = server.close();
+
+    assert!(!Response::is_network_error(&fetch_response));
+    assert_eq!(fetch_response.response_type, ResponseType::Basic);
+
+    let headers = fetch_response.headers;
+    assert!(!headers.has::<SetCookie>());
+    assert!(headers.get_raw("Set-Cookie2").is_none());
+}
+
+#[test]
+fn test_fetch_response_is_cors_filtered() {
+
+    static MESSAGE: &'static [u8] = b"";
+    let handler = move |_: HyperRequest, mut response: HyperResponse| {
+
+        // this is mandatory for the Cors Check to pass
+        // TODO test using different url encodings with this value ie. punycode
+        response.headers_mut().set(AccessControlAllowOrigin::Any);
+
+        // these are the headers that should be kept after filtering
+        response.headers_mut().set(CacheControl(vec![]));
+        response.headers_mut().set(ContentLanguage(vec![]));
+        response.headers_mut().set(ContentType::html());
+        response.headers_mut().set(Expires(HttpDate(time::now() + Duration::days(1))));
+        response.headers_mut().set(LastModified(HttpDate(time::now())));
+        response.headers_mut().set(Pragma::NoCache);
+
+        // these headers should not be kept after filtering, even though they are given a pass
+        response.headers_mut().set(SetCookie(vec![]));
+        response.headers_mut().set_raw("Set-Cookie2", vec![]);
+        response.headers_mut().set(
+            AccessControlAllowHeaders(vec![
+                UniCase("set-cookie".to_owned()),
+                UniCase("set-cookie2".to_owned())
+            ])
+        );
+
+        response.send(MESSAGE).unwrap();
+    };
+    let (mut server, url) = make_server(handler);
+
+    // an origin mis-match will stop it from defaulting to a basic filtered response
+    let origin = Origin::UID(OpaqueOrigin::new());
+    let mut request = Request::new(url, Context::Fetch, origin, false);
+    request.referer = Referer::NoReferer;
+    request.mode = RequestMode::CORSMode;
+    let wrapped_request = Rc::new(request);
+
+    let fetch_response = fetch(wrapped_request, false);
+    let _ = server.close();
+
+    assert!(!Response::is_network_error(&fetch_response));
+    assert_eq!(fetch_response.response_type, ResponseType::CORS);
+
+    let headers = fetch_response.headers;
+    assert!(headers.has::<CacheControl>());
+    assert!(headers.has::<ContentLanguage>());
+    assert!(headers.has::<ContentType>());
+    assert!(headers.has::<Expires>());
+    assert!(headers.has::<LastModified>());
+    assert!(headers.has::<Pragma>());
+
+    assert!(!headers.has::<AccessControlAllowOrigin>());
+    assert!(!headers.has::<SetCookie>());
+    assert!(headers.get_raw("Set-Cookie2").is_none());
+}
+
+#[test]
+fn test_fetch_response_is_opaque_filtered() {
+
+    static MESSAGE: &'static [u8] = b"";
+    let handler = move |_: HyperRequest, response: HyperResponse| {
+        response.send(MESSAGE).unwrap();
+    };
+    let (mut server, url) = make_server(handler);
+
+    // an origin mis-match will fall through to an Opaque filtered response
+    let origin = Origin::UID(OpaqueOrigin::new());
+    let mut request = Request::new(url, Context::Fetch, origin, false);
+    request.referer = Referer::NoReferer;
+    let wrapped_request = Rc::new(request);
+
+    let fetch_response = fetch(wrapped_request, false);
+    let _ = server.close();
+
+    assert!(!Response::is_network_error(&fetch_response));
+    assert_eq!(fetch_response.response_type, ResponseType::Opaque);
+
+    assert!(fetch_response.url_list.into_inner().len() == 0);
+    assert!(fetch_response.url.is_none());
+    // this also asserts that status message is "the empty byte sequence"
+    assert!(fetch_response.status.is_none());
+    assert_eq!(fetch_response.headers, Headers::new());
+    match fetch_response.body.into_inner() {
+        ResponseBody::Empty => { },
+        _ => panic!()
+    }
+    match fetch_response.cache_state {
+        CacheState::None => { },
+        _ => panic!()
+    }
+}
+
+#[test]
+fn test_fetch_response_is_opaque_redirect_filtered() {
+
+    static MESSAGE: &'static [u8] = b"";
+    let handler = move |request: HyperRequest, mut response: HyperResponse| {
+
+        let redirects = match request.uri {
+            RequestUri::AbsolutePath(url) =>
+                url.split("/").collect::<String>().parse::<u32>().unwrap_or(0),
+            RequestUri::AbsoluteUri(url) =>
+                url.path().unwrap().last().unwrap().split("/").collect::<String>().parse::<u32>().unwrap_or(0),
+            _ => panic!()
+        };
+
+        if redirects == 1 {
+            response.send(MESSAGE).unwrap();
+        } else {
+            *response.status_mut() = StatusCode::Found;
+            let url = format!("{}", 1);
+            response.headers_mut().set(Location(url.to_owned()));
+        }
+    };
+
+    let (mut server, url) = make_server(handler);
+
+    let origin = url.origin();
+    let mut request = Request::new(url, Context::Fetch, origin, false);
+    request.referer = Referer::NoReferer;
+    request.redirect_mode = Cell::new(RedirectMode::Manual);
+    let wrapped_request = Rc::new(request);
+
+    let fetch_response = fetch(wrapped_request, false);
+    let _ = server.close();
+
+    assert!(!Response::is_network_error(&fetch_response));
+    assert_eq!(fetch_response.response_type, ResponseType::OpaqueRedirect);
+
+    // this also asserts that status message is "the empty byte sequence"
+    assert!(fetch_response.status.is_none());
+    assert_eq!(fetch_response.headers, Headers::new());
+    match fetch_response.body.into_inner() {
+        ResponseBody::Empty => { },
+        _ => panic!()
+    }
+    match fetch_response.cache_state {
+        CacheState::None => { },
+        _ => panic!()
+    }
+}
+
 fn test_fetch_redirect_count(message: &'static [u8], redirect_cap: u32) -> Response {
 
     let handler = move |request: HyperRequest, mut response: HyperResponse| {

tests/unit/net/lib.rs

@@ -14,6 +14,7 @@ extern crate msg;
 extern crate net;
 extern crate net_traits;
 extern crate time;
+extern crate unicase;
 extern crate url;
 extern crate util;