Skip to content

Commit

Permalink
Auto merge of #9525 - nikkisquared:test_filtered_responses, r=asajeffrey
Browse files Browse the repository at this point in the history
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 -->
  • Loading branch information
bors-servo committed Feb 9, 2016
2 parents cb8be14 + e8665d4 commit f1018b8
Show file tree
Hide file tree
Showing 7 changed files with 278 additions and 14 deletions.
92 changes: 86 additions & 6 deletions components/net/fetch/methods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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>) {
Expand Down Expand Up @@ -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() {
return Response::network_error();
}
// if cached_response.is_none() {
// return Response::network_error();
// }

// Substep 3

Expand Down Expand Up @@ -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";
Expand Down
6 changes: 4 additions & 2 deletions components/net/fetch/response.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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" |
"content-type" | "expires" | "last-modified" | "Pragma" => true,
"cache-control" | "content-language" | "content-type" |
"expires" | "last-modified" | "pragma" => true,
"set-cookie" | "set-cookie2" => false,
header => {
let result =
Expand All @@ -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;
}
}

Expand Down
4 changes: 2 additions & 2 deletions components/net_traits/response.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,15 @@ 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>),
Done(Vec<u8>),
}

/// [Cache state](https://fetch.spec.whatwg.org/#concept-response-cache-state)
#[derive(Clone)]
#[derive(Clone, Debug)]
pub enum CacheState {
None,
Local,
Expand Down
1 change: 1 addition & 0 deletions components/servo/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions tests/unit/net/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
187 changes: 183 additions & 4 deletions tests/unit/net/fetch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::response::{Response, ResponseBody, ResponseType};
use net_traits::request::{Context, RedirectMode, Referer, Request, RequestMode};
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

Expand Down Expand Up @@ -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| {
Expand Down
1 change: 1 addition & 0 deletions tests/unit/net/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down

0 comments on commit f1018b8

Please sign in to comment.