From 95a7482d26d354e7e02a4309e72d9cd6722e490f Mon Sep 17 00:00:00 2001 From: Jan Zerebecki Date: Tue, 27 Sep 2016 17:17:33 +0200 Subject: [PATCH] Remove same-origin-data-url flag from fetch implementation The spec removed it. Check the scheme instead, data is always same origin now, except for workers. This also updates the comments to make step numbers match the spec. Closes #13362 --- components/net/fetch/methods.rs | 40 ++++++++++------------ components/net_traits/request.rs | 5 --- components/script/dom/htmlscriptelement.rs | 2 -- components/script/dom/request.rs | 1 - components/script/dom/xmlhttprequest.rs | 1 - components/script/fetch.rs | 1 - tests/unit/net/fetch.rs | 2 -- 7 files changed, 18 insertions(+), 34 deletions(-) diff --git a/components/net/fetch/methods.rs b/components/net/fetch/methods.rs index a97ee2e9985..52536472514 100644 --- a/components/net/fetch/methods.rs +++ b/components/net/fetch/methods.rs @@ -194,8 +194,8 @@ fn main_fetch(request: Rc, cache: &mut CORSCache, cors_flag: bool, }; if (same_origin && !cors_flag ) || - (current_url.scheme() == "data" && request.same_origin_data.get()) || - (current_url.scheme() == "file" && request.same_origin_data.get()) || + current_url.scheme() == "data" || + current_url.scheme() == "file" || current_url.scheme() == "about" || request.mode == RequestMode::Navigate { basic_fetch(request.clone(), cache, target, done_chan, context) @@ -640,41 +640,35 @@ fn http_redirect_fetch(request: Rc, // Step 1 assert_eq!(response.return_internal.get(), true); - // Step 3 - // this step is done early, because querying if Location exists says - // if it is None or Some, making it easy to seperate from the retrieval failure case + // Step 2 if !response.actual_response().headers.has::() { return Rc::try_unwrap(response).ok().unwrap(); } - // Step 2 + // Step 3 let location = match response.actual_response().headers.get::() { Some(&Location(ref location)) => location.clone(), - // Step 4 _ => return Response::network_error() }; - - // Step 5 let response_url = response.actual_response().url.as_ref().unwrap(); let location_url = response_url.join(&*location); - - // Step 6 let location_url = match location_url { Ok(url) => url, _ => return Response::network_error() }; - // Step 7 + // Step 4 + // TODO implement return network_error if not HTTP(S) + + // Step 5 if request.redirect_count.get() >= 20 { return Response::network_error(); } - // Step 8 + // Step 6 request.redirect_count.set(request.redirect_count.get() + 1); - // Step 9 - request.same_origin_data.set(false); - + // Step 7 let same_origin = if let Origin::Origin(ref origin) = *request.origin.borrow() { *origin == request.current_url().origin() } else { @@ -682,22 +676,21 @@ fn http_redirect_fetch(request: Rc, }; let has_credentials = has_credentials(&location_url); - // Step 10 if request.mode == RequestMode::CORSMode && !same_origin && has_credentials { return Response::network_error(); } - // Step 11 + // Step 8 if cors_flag && has_credentials { return Response::network_error(); } - // Step 12 + // Step 9 if cors_flag && !same_origin { *request.origin.borrow_mut() = Origin::Origin(UrlOrigin::new_opaque()); } - // Step 13 + // Step 10 let status_code = response.actual_response().status.unwrap(); if ((status_code == StatusCode::MovedPermanently || status_code == StatusCode::Found) && *request.method.borrow() == Method::Post) || @@ -706,10 +699,13 @@ fn http_redirect_fetch(request: Rc, *request.body.borrow_mut() = None; } - // Step 14 + // Step 11 request.url_list.borrow_mut().push(location_url); - // Step 15 + // Step 12 + // TODO implement referrer policy + + // Step 13 main_fetch(request, cache, cors_flag, true, target, done_chan, context) } diff --git a/components/net_traits/request.rs b/components/net_traits/request.rs index c31f821680d..2a84c91c759 100644 --- a/components/net_traits/request.rs +++ b/components/net_traits/request.rs @@ -120,7 +120,6 @@ pub struct RequestInit { serialize_with = "::hyper_serde::serialize")] pub headers: Headers, pub unsafe_request: bool, - pub same_origin_data: bool, pub body: Option>, // TODO: client object pub type_: Type, @@ -146,7 +145,6 @@ impl Default for RequestInit { url: Url::parse("about:blank").unwrap(), headers: Headers::new(), unsafe_request: false, - same_origin_data: false, body: None, type_: Type::None, destination: Destination::None, @@ -188,7 +186,6 @@ pub struct Request { // TODO: priority object pub origin: RefCell, pub omit_origin_header: Cell, - pub same_origin_data: Cell, /// https://fetch.spec.whatwg.org/#concept-request-referrer pub referrer: RefCell, pub referrer_policy: Cell>, @@ -230,7 +227,6 @@ impl Request { destination: Destination::None, origin: RefCell::new(origin.unwrap_or(Origin::Client)), omit_origin_header: Cell::new(false), - same_origin_data: Cell::new(false), referrer: RefCell::new(Referrer::Client), referrer_policy: Cell::new(None), pipeline_id: Cell::new(pipeline_id), @@ -256,7 +252,6 @@ impl Request { *req.method.borrow_mut() = init.method; *req.headers.borrow_mut() = init.headers; req.unsafe_request = init.unsafe_request; - req.same_origin_data.set(init.same_origin_data); *req.body.borrow_mut() = init.body; req.type_ = init.type_; req.destination = init.destination; diff --git a/components/script/dom/htmlscriptelement.rs b/components/script/dom/htmlscriptelement.rs index 4b16908968f..15df6ee4e33 100644 --- a/components/script/dom/htmlscriptelement.rs +++ b/components/script/dom/htmlscriptelement.rs @@ -244,8 +244,6 @@ fn fetch_a_classic_script(script: &HTMLScriptElement, }, origin: doc.url().clone(), pipeline_id: Some(script.global().r().pipeline_id()), - // FIXME: Set to true for now, discussion in https://github.com/whatwg/fetch/issues/381 - same_origin_data: true, referrer_url: Some(doc.url().clone()), referrer_policy: doc.get_referrer_policy(), .. RequestInit::default() diff --git a/components/script/dom/request.rs b/components/script/dom/request.rs index 4ffbeb2dcd1..fc977978949 100644 --- a/components/script/dom/request.rs +++ b/components/script/dom/request.rs @@ -159,7 +159,6 @@ impl Request { // TODO: `entry settings object` is not implemented in Servo yet. *request.origin.borrow_mut() = Origin::Client; request.omit_origin_header = temporary_request.omit_origin_header; - request.same_origin_data.set(true); request.referrer = temporary_request.referrer; request.referrer_policy = temporary_request.referrer_policy; request.mode = temporary_request.mode; diff --git a/components/script/dom/xmlhttprequest.rs b/components/script/dom/xmlhttprequest.rs index 2a193cea77a..4643921bc2d 100644 --- a/components/script/dom/xmlhttprequest.rs +++ b/components/script/dom/xmlhttprequest.rs @@ -587,7 +587,6 @@ impl XMLHttpRequestMethods for XMLHttpRequest { url: self.request_url.borrow().clone().unwrap(), headers: (*self.request_headers.borrow()).clone(), unsafe_request: true, - same_origin_data: true, // XXXManishearth figure out how to avoid this clone body: extracted.as_ref().map(|e| e.0.clone()), // XXXManishearth actually "subresource", but it doesn't exist diff --git a/components/script/fetch.rs b/components/script/fetch.rs index 1117d050319..0fc0fef1f89 100644 --- a/components/script/fetch.rs +++ b/components/script/fetch.rs @@ -44,7 +44,6 @@ fn request_init_from_request(request: NetTraitsRequest) -> NetTraitsRequestInit url: request.url(), headers: request.headers.borrow().clone(), unsafe_request: request.unsafe_request, - same_origin_data: request.same_origin_data.get(), body: request.body.borrow().clone(), type_: request.type_, destination: request.destination, diff --git a/tests/unit/net/fetch.rs b/tests/unit/net/fetch.rs index 9f9607b1743..217cc8ccee5 100644 --- a/tests/unit/net/fetch.rs +++ b/tests/unit/net/fetch.rs @@ -144,7 +144,6 @@ fn test_fetch_data() { let url = Url::parse("data:text/html,

Servo

").unwrap(); let origin = Origin::Origin(url.origin()); let request = Request::new(url, Some(origin), false, None); - request.same_origin_data.set(true); let expected_resp_body = "

Servo

".to_owned(); let fetch_response = fetch_sync(request, None); @@ -173,7 +172,6 @@ fn test_fetch_file() { let url = Url::from_file_path(path.clone()).unwrap(); let origin = Origin::Origin(url.origin()); let request = Request::new(url, Some(origin), false, None); - request.same_origin_data.set(true); let fetch_response = fetch_sync(request, None); assert!(!fetch_response.is_network_error());