mirror of
https://github.com/servo/servo.git
synced 2025-08-06 06:00:15 +01:00
Auto merge of #13467 - JanZerebecki:rm-same-origin-data-url, r=KiChjang
Remove same-origin-data-url flag from fetch implementation <!-- Please describe your changes on the following line: --> The spec removed it. Check the scheme instead, data is always same origin now, except for workers. Closes #13362 --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #13362 . <!-- Either: --> - [ ] There are tests for these changes OR - [x] These changes do not require tests because they only remove code. <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/13467) <!-- Reviewable:end -->
This commit is contained in:
commit
e494dedce5
7 changed files with 18 additions and 34 deletions
|
@ -194,8 +194,8 @@ fn main_fetch(request: Rc<Request>, cache: &mut CORSCache, cors_flag: bool,
|
||||||
};
|
};
|
||||||
|
|
||||||
if (same_origin && !cors_flag ) ||
|
if (same_origin && !cors_flag ) ||
|
||||||
(current_url.scheme() == "data" && request.same_origin_data.get()) ||
|
current_url.scheme() == "data" ||
|
||||||
(current_url.scheme() == "file" && request.same_origin_data.get()) ||
|
current_url.scheme() == "file" ||
|
||||||
current_url.scheme() == "about" ||
|
current_url.scheme() == "about" ||
|
||||||
request.mode == RequestMode::Navigate {
|
request.mode == RequestMode::Navigate {
|
||||||
basic_fetch(request.clone(), cache, target, done_chan, context)
|
basic_fetch(request.clone(), cache, target, done_chan, context)
|
||||||
|
@ -640,41 +640,35 @@ fn http_redirect_fetch(request: Rc<Request>,
|
||||||
// Step 1
|
// Step 1
|
||||||
assert_eq!(response.return_internal.get(), true);
|
assert_eq!(response.return_internal.get(), true);
|
||||||
|
|
||||||
// Step 3
|
// Step 2
|
||||||
// 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
|
|
||||||
if !response.actual_response().headers.has::<Location>() {
|
if !response.actual_response().headers.has::<Location>() {
|
||||||
return Rc::try_unwrap(response).ok().unwrap();
|
return Rc::try_unwrap(response).ok().unwrap();
|
||||||
}
|
}
|
||||||
|
|
||||||
// Step 2
|
// Step 3
|
||||||
let location = match response.actual_response().headers.get::<Location>() {
|
let location = match response.actual_response().headers.get::<Location>() {
|
||||||
Some(&Location(ref location)) => location.clone(),
|
Some(&Location(ref location)) => location.clone(),
|
||||||
// Step 4
|
|
||||||
_ => return Response::network_error()
|
_ => return Response::network_error()
|
||||||
};
|
};
|
||||||
|
|
||||||
// Step 5
|
|
||||||
let response_url = response.actual_response().url.as_ref().unwrap();
|
let response_url = response.actual_response().url.as_ref().unwrap();
|
||||||
let location_url = response_url.join(&*location);
|
let location_url = response_url.join(&*location);
|
||||||
|
|
||||||
// Step 6
|
|
||||||
let location_url = match location_url {
|
let location_url = match location_url {
|
||||||
Ok(url) => url,
|
Ok(url) => url,
|
||||||
_ => return Response::network_error()
|
_ => 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 {
|
if request.redirect_count.get() >= 20 {
|
||||||
return Response::network_error();
|
return Response::network_error();
|
||||||
}
|
}
|
||||||
|
|
||||||
// Step 8
|
// Step 6
|
||||||
request.redirect_count.set(request.redirect_count.get() + 1);
|
request.redirect_count.set(request.redirect_count.get() + 1);
|
||||||
|
|
||||||
// Step 9
|
// Step 7
|
||||||
request.same_origin_data.set(false);
|
|
||||||
|
|
||||||
let same_origin = if let Origin::Origin(ref origin) = *request.origin.borrow() {
|
let same_origin = if let Origin::Origin(ref origin) = *request.origin.borrow() {
|
||||||
*origin == request.current_url().origin()
|
*origin == request.current_url().origin()
|
||||||
} else {
|
} else {
|
||||||
|
@ -682,22 +676,21 @@ fn http_redirect_fetch(request: Rc<Request>,
|
||||||
};
|
};
|
||||||
let has_credentials = has_credentials(&location_url);
|
let has_credentials = has_credentials(&location_url);
|
||||||
|
|
||||||
// Step 10
|
|
||||||
if request.mode == RequestMode::CORSMode && !same_origin && has_credentials {
|
if request.mode == RequestMode::CORSMode && !same_origin && has_credentials {
|
||||||
return Response::network_error();
|
return Response::network_error();
|
||||||
}
|
}
|
||||||
|
|
||||||
// Step 11
|
// Step 8
|
||||||
if cors_flag && has_credentials {
|
if cors_flag && has_credentials {
|
||||||
return Response::network_error();
|
return Response::network_error();
|
||||||
}
|
}
|
||||||
|
|
||||||
// Step 12
|
// Step 9
|
||||||
if cors_flag && !same_origin {
|
if cors_flag && !same_origin {
|
||||||
*request.origin.borrow_mut() = Origin::Origin(UrlOrigin::new_opaque());
|
*request.origin.borrow_mut() = Origin::Origin(UrlOrigin::new_opaque());
|
||||||
}
|
}
|
||||||
|
|
||||||
// Step 13
|
// Step 10
|
||||||
let status_code = response.actual_response().status.unwrap();
|
let status_code = response.actual_response().status.unwrap();
|
||||||
if ((status_code == StatusCode::MovedPermanently || status_code == StatusCode::Found) &&
|
if ((status_code == StatusCode::MovedPermanently || status_code == StatusCode::Found) &&
|
||||||
*request.method.borrow() == Method::Post) ||
|
*request.method.borrow() == Method::Post) ||
|
||||||
|
@ -706,10 +699,13 @@ fn http_redirect_fetch(request: Rc<Request>,
|
||||||
*request.body.borrow_mut() = None;
|
*request.body.borrow_mut() = None;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Step 14
|
// Step 11
|
||||||
request.url_list.borrow_mut().push(location_url);
|
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)
|
main_fetch(request, cache, cors_flag, true, target, done_chan, context)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -120,7 +120,6 @@ pub struct RequestInit {
|
||||||
serialize_with = "::hyper_serde::serialize")]
|
serialize_with = "::hyper_serde::serialize")]
|
||||||
pub headers: Headers,
|
pub headers: Headers,
|
||||||
pub unsafe_request: bool,
|
pub unsafe_request: bool,
|
||||||
pub same_origin_data: bool,
|
|
||||||
pub body: Option<Vec<u8>>,
|
pub body: Option<Vec<u8>>,
|
||||||
// TODO: client object
|
// TODO: client object
|
||||||
pub type_: Type,
|
pub type_: Type,
|
||||||
|
@ -146,7 +145,6 @@ impl Default for RequestInit {
|
||||||
url: Url::parse("about:blank").unwrap(),
|
url: Url::parse("about:blank").unwrap(),
|
||||||
headers: Headers::new(),
|
headers: Headers::new(),
|
||||||
unsafe_request: false,
|
unsafe_request: false,
|
||||||
same_origin_data: false,
|
|
||||||
body: None,
|
body: None,
|
||||||
type_: Type::None,
|
type_: Type::None,
|
||||||
destination: Destination::None,
|
destination: Destination::None,
|
||||||
|
@ -188,7 +186,6 @@ pub struct Request {
|
||||||
// TODO: priority object
|
// TODO: priority object
|
||||||
pub origin: RefCell<Origin>,
|
pub origin: RefCell<Origin>,
|
||||||
pub omit_origin_header: Cell<bool>,
|
pub omit_origin_header: Cell<bool>,
|
||||||
pub same_origin_data: Cell<bool>,
|
|
||||||
/// https://fetch.spec.whatwg.org/#concept-request-referrer
|
/// https://fetch.spec.whatwg.org/#concept-request-referrer
|
||||||
pub referrer: RefCell<Referrer>,
|
pub referrer: RefCell<Referrer>,
|
||||||
pub referrer_policy: Cell<Option<ReferrerPolicy>>,
|
pub referrer_policy: Cell<Option<ReferrerPolicy>>,
|
||||||
|
@ -230,7 +227,6 @@ impl Request {
|
||||||
destination: Destination::None,
|
destination: Destination::None,
|
||||||
origin: RefCell::new(origin.unwrap_or(Origin::Client)),
|
origin: RefCell::new(origin.unwrap_or(Origin::Client)),
|
||||||
omit_origin_header: Cell::new(false),
|
omit_origin_header: Cell::new(false),
|
||||||
same_origin_data: Cell::new(false),
|
|
||||||
referrer: RefCell::new(Referrer::Client),
|
referrer: RefCell::new(Referrer::Client),
|
||||||
referrer_policy: Cell::new(None),
|
referrer_policy: Cell::new(None),
|
||||||
pipeline_id: Cell::new(pipeline_id),
|
pipeline_id: Cell::new(pipeline_id),
|
||||||
|
@ -256,7 +252,6 @@ impl Request {
|
||||||
*req.method.borrow_mut() = init.method;
|
*req.method.borrow_mut() = init.method;
|
||||||
*req.headers.borrow_mut() = init.headers;
|
*req.headers.borrow_mut() = init.headers;
|
||||||
req.unsafe_request = init.unsafe_request;
|
req.unsafe_request = init.unsafe_request;
|
||||||
req.same_origin_data.set(init.same_origin_data);
|
|
||||||
*req.body.borrow_mut() = init.body;
|
*req.body.borrow_mut() = init.body;
|
||||||
req.type_ = init.type_;
|
req.type_ = init.type_;
|
||||||
req.destination = init.destination;
|
req.destination = init.destination;
|
||||||
|
|
|
@ -244,8 +244,6 @@ fn fetch_a_classic_script(script: &HTMLScriptElement,
|
||||||
},
|
},
|
||||||
origin: doc.url().clone(),
|
origin: doc.url().clone(),
|
||||||
pipeline_id: Some(script.global().r().pipeline_id()),
|
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_url: Some(doc.url().clone()),
|
||||||
referrer_policy: doc.get_referrer_policy(),
|
referrer_policy: doc.get_referrer_policy(),
|
||||||
.. RequestInit::default()
|
.. RequestInit::default()
|
||||||
|
|
|
@ -159,7 +159,6 @@ impl Request {
|
||||||
// TODO: `entry settings object` is not implemented in Servo yet.
|
// TODO: `entry settings object` is not implemented in Servo yet.
|
||||||
*request.origin.borrow_mut() = Origin::Client;
|
*request.origin.borrow_mut() = Origin::Client;
|
||||||
request.omit_origin_header = temporary_request.omit_origin_header;
|
request.omit_origin_header = temporary_request.omit_origin_header;
|
||||||
request.same_origin_data.set(true);
|
|
||||||
request.referrer = temporary_request.referrer;
|
request.referrer = temporary_request.referrer;
|
||||||
request.referrer_policy = temporary_request.referrer_policy;
|
request.referrer_policy = temporary_request.referrer_policy;
|
||||||
request.mode = temporary_request.mode;
|
request.mode = temporary_request.mode;
|
||||||
|
|
|
@ -587,7 +587,6 @@ impl XMLHttpRequestMethods for XMLHttpRequest {
|
||||||
url: self.request_url.borrow().clone().unwrap(),
|
url: self.request_url.borrow().clone().unwrap(),
|
||||||
headers: (*self.request_headers.borrow()).clone(),
|
headers: (*self.request_headers.borrow()).clone(),
|
||||||
unsafe_request: true,
|
unsafe_request: true,
|
||||||
same_origin_data: true,
|
|
||||||
// XXXManishearth figure out how to avoid this clone
|
// XXXManishearth figure out how to avoid this clone
|
||||||
body: extracted.as_ref().map(|e| e.0.clone()),
|
body: extracted.as_ref().map(|e| e.0.clone()),
|
||||||
// XXXManishearth actually "subresource", but it doesn't exist
|
// XXXManishearth actually "subresource", but it doesn't exist
|
||||||
|
|
|
@ -44,7 +44,6 @@ fn request_init_from_request(request: NetTraitsRequest) -> NetTraitsRequestInit
|
||||||
url: request.url(),
|
url: request.url(),
|
||||||
headers: request.headers.borrow().clone(),
|
headers: request.headers.borrow().clone(),
|
||||||
unsafe_request: request.unsafe_request,
|
unsafe_request: request.unsafe_request,
|
||||||
same_origin_data: request.same_origin_data.get(),
|
|
||||||
body: request.body.borrow().clone(),
|
body: request.body.borrow().clone(),
|
||||||
type_: request.type_,
|
type_: request.type_,
|
||||||
destination: request.destination,
|
destination: request.destination,
|
||||||
|
|
|
@ -144,7 +144,6 @@ fn test_fetch_data() {
|
||||||
let url = Url::parse("data:text/html,<p>Servo</p>").unwrap();
|
let url = Url::parse("data:text/html,<p>Servo</p>").unwrap();
|
||||||
let origin = Origin::Origin(url.origin());
|
let origin = Origin::Origin(url.origin());
|
||||||
let request = Request::new(url, Some(origin), false, None);
|
let request = Request::new(url, Some(origin), false, None);
|
||||||
request.same_origin_data.set(true);
|
|
||||||
let expected_resp_body = "<p>Servo</p>".to_owned();
|
let expected_resp_body = "<p>Servo</p>".to_owned();
|
||||||
let fetch_response = fetch_sync(request, None);
|
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 url = Url::from_file_path(path.clone()).unwrap();
|
||||||
let origin = Origin::Origin(url.origin());
|
let origin = Origin::Origin(url.origin());
|
||||||
let request = Request::new(url, Some(origin), false, None);
|
let request = Request::new(url, Some(origin), false, None);
|
||||||
request.same_origin_data.set(true);
|
|
||||||
|
|
||||||
let fetch_response = fetch_sync(request, None);
|
let fetch_response = fetch_sync(request, None);
|
||||||
assert!(!fetch_response.is_network_error());
|
assert!(!fetch_response.is_network_error());
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue