Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Properly set origin of fetch requests #16508

Merged
merged 1 commit into from Jul 17, 2017
Merged
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

@@ -216,7 +216,8 @@ impl FontCache {
url: url.clone(),
type_: RequestType::Font,
destination: Destination::Font,
origin: url.clone(),
// TODO: Add a proper origin - Can't import GlobalScope from gfx

This comment has been minimized.

@fnune

fnune Apr 25, 2017

Author Contributor

Help needed for the following:

I can't use script::dom::globalscope::GlobalScope; from here, because dom is private. Also I can't build the origin from anywhere else (I think?).

Similarly, in the tests for http_loader.rs I can't import GlobalScope, and I see no way I can mock an origin without access to dom.

This comment has been minimized.

@KiChjang

KiChjang Apr 25, 2017

Member

Instead of using GlobalScope, figure out a way to directly thread through an Origin?

This comment has been minimized.

@fnune

fnune Apr 25, 2017

Author Contributor

That would be fine for mocking in the tests, maybe. But in font_cache_thread.rs we need the current origin so I can't just build it... maybe if I could access the current URL and then build an origin from it? I haven't figured out how to find the URL though.

This comment has been minimized.

@KiChjang

KiChjang Apr 25, 2017

Member

I meant something like adding a new origin field to the AddWebFont enum tuple variant, and then look at all the places where AddWebFont is created/called, and add the appropriate Origin for it. Does that make sense?

This comment has been minimized.

@fnune

fnune Apr 29, 2017

Author Contributor

I think I can just create an opaque origin out of nothing because it's a font loader anyway, and it's going to be fetching from different places which will never be the same origin.

This comment has been minimized.

@jdm

jdm Jun 4, 2017

Member

I recommend adding an ImmutableOrigin value to the Url variant of the Source enum, and using that origin here.

// We can leave origin to be set by default
.. RequestInit::default()
};

@@ -1371,7 +1371,7 @@ fn cors_check(request: &Request, response: &Response) -> Result<(), ()> {
};

match request.origin {
Origin::Origin(ref o) if o.ascii_serialization() == origin => {},
Origin::Origin(ref o) if o.ascii_serialization() == origin.trim() => {},
_ => return Err(())
}

@@ -158,9 +158,7 @@ pub struct RequestInit {
pub use_cors_preflight: bool,
pub credentials_mode: CredentialsMode,
pub use_url_credentials: bool,
// this should actually be set by fetch, but fetch
// doesn't have info about the client right now
pub origin: ServoUrl,
pub origin: ImmutableOrigin,

This comment has been minimized.

@asajeffrey

asajeffrey Jun 3, 2017

Member

Yay! This has been bugging me for a while!

// XXXManishearth these should be part of the client object
pub referrer_url: Option<ServoUrl>,
pub referrer_policy: Option<ReferrerPolicy>,
@@ -188,7 +186,7 @@ impl Default for RequestInit {
use_cors_preflight: false,
credentials_mode: CredentialsMode::Omit,
use_url_credentials: false,
origin: ServoUrl::parse("about:blank").unwrap(),
origin: ImmutableOrigin::new_opaque(),
referrer_url: None,
referrer_policy: None,
pipeline_id: None,
@@ -302,7 +300,7 @@ impl Request {

pub fn from_init(init: RequestInit) -> Request {
let mut req = Request::new(init.url.clone(),
Some(Origin::Origin(init.origin.origin())),
Some(Origin::Origin(init.origin)),
init.pipeline_id);
req.method = init.method;
req.headers = init.headers;
@@ -160,6 +160,7 @@ impl DedicatedWorkerGlobalScope {
let serialized_worker_url = worker_url.to_string();
let name = format!("WebWorker for {}", serialized_worker_url);
let top_level_browsing_context_id = TopLevelBrowsingContextId::installed();
let origin = GlobalScope::current().expect("No current global object").origin().immutable().clone();

thread::Builder::new().name(name).spawn(move || {
thread_state::initialize(thread_state::SCRIPT | thread_state::IN_WORKER);
@@ -179,10 +180,10 @@ impl DedicatedWorkerGlobalScope {
destination: Destination::Worker,
credentials_mode: CredentialsMode::Include,
use_url_credentials: true,
origin: worker_url,
pipeline_id: pipeline_id,
referrer_url: referrer_url,
referrer_policy: referrer_policy,
origin,
.. RequestInit::default()
};

@@ -387,7 +387,7 @@ impl EventSource {
// TODO: Step 9 set request's client settings
let mut request = RequestInit {
url: url_record,
origin: global.get_url(),
origin: global.origin().immutable().clone(),
pipeline_id: Some(global.pipeline_id()),
// https://html.spec.whatwg.org/multipage/#create-a-potential-cors-request
use_url_credentials: true,
@@ -259,7 +259,7 @@ impl HTMLImageElement {

let request = RequestInit {
url: img_url.clone(),
origin: document.url().clone(),
origin: document.origin().immutable().clone(),
type_: RequestType::Image,
pipeline_id: Some(document.global().pipeline_id()),
.. RequestInit::default()
@@ -550,7 +550,7 @@ impl HTMLMediaElement {
destination: Destination::Media,
credentials_mode: CredentialsMode::Include,
use_url_credentials: true,
origin: document.url(),
origin: document.origin().immutable().clone(),
pipeline_id: Some(self.global().pipeline_id()),
referrer_url: Some(document.url()),
referrer_policy: document.get_referrer_policy(),
@@ -253,7 +253,7 @@ fn fetch_a_classic_script(script: &HTMLScriptElement,
Some(CorsSettings::Anonymous) => CredentialsMode::CredentialsSameOrigin,
_ => CredentialsMode::Include,
},
origin: doc.url(),
origin: doc.origin().immutable().clone(),
pipeline_id: Some(script.global().pipeline_id()),
referrer_url: Some(doc.url()),
referrer_policy: doc.get_referrer_policy(),
@@ -151,6 +151,7 @@ impl ServiceWorkerGlobalScope {
.. } = scope_things;

let serialized_worker_url = script_url.to_string();
let origin = GlobalScope::current().expect("No current global object").origin().immutable().clone();
thread::Builder::new().name(format!("ServiceWorker for {}", serialized_worker_url)).spawn(move || {
thread_state::initialize(SCRIPT | IN_WORKER);
let roots = RootCollection::new();
@@ -164,10 +165,10 @@ impl ServiceWorkerGlobalScope {
destination: Destination::ServiceWorker,
credentials_mode: CredentialsMode::Include,
use_url_credentials: true,
origin: script_url,
pipeline_id: pipeline_id,
referrer_url: referrer_url,
referrer_policy: referrer_policy,
origin,
.. RequestInit::default()
};

@@ -211,7 +211,7 @@ impl WorkerGlobalScopeMethods for WorkerGlobalScope {
destination: Destination::Script,
credentials_mode: CredentialsMode::Include,
use_url_credentials: true,
origin: self.worker_url.clone(),
origin: global_scope.origin().immutable().clone(),
pipeline_id: Some(self.upcast::<GlobalScope>().pipeline_id()),
referrer_url: None,
referrer_policy: None,
@@ -588,17 +588,14 @@ impl WorkletThread {
// TODO: Fetch a module graph, not just a single script.
// TODO: Fetch the script asynchronously?
// TODO: Caching.
// TODO: Avoid re-parsing the origin as a URL.
let resource_fetcher = self.global_init.resource_threads.sender();
let origin_url = ServoUrl::parse(&*origin.unicode_serialization())
.unwrap_or_else(|_| ServoUrl::parse("about:blank").unwrap());
let request = RequestInit {
url: script_url,
type_: RequestType::Script,
destination: Destination::Script,
mode: RequestMode::CorsMode,
origin: origin_url,
credentials_mode: credentials.into(),
origin,
.. RequestInit::default()
};
let script = load_whole_resource(request, &resource_fetcher).ok()
@@ -594,7 +594,7 @@ impl XMLHttpRequestMethods for XMLHttpRequest {
use_cors_preflight: has_handlers,
credentials_mode: credentials_mode,
use_url_credentials: use_url_credentials,
origin: self.global().get_url(),
origin: self.global().origin().immutable().clone(),
referrer_url: self.referrer_url.clone(),
referrer_policy: self.referrer_policy.clone(),
pipeline_id: Some(self.global().pipeline_id()),
@@ -56,10 +56,7 @@ fn request_init_from_request(request: NetTraitsRequest) -> NetTraitsRequestInit
use_cors_preflight: request.use_cors_preflight,
credentials_mode: request.credentials_mode,
use_url_credentials: request.use_url_credentials,
// TODO: NetTraitsRequestInit and NetTraitsRequest have different "origin"
// ... NetTraitsRequestInit.origin: Url
// ... NetTraitsRequest.origin: RefCell<Origin>
origin: request.url(),
origin: GlobalScope::current().expect("No current global object").origin().immutable().clone(),
referrer_url: from_referrer_to_referrer_url(&request),
referrer_policy: request.referrer_policy,
pipeline_id: request.pipeline_id,
@@ -70,7 +70,7 @@ pub fn fetch_image_for_layout(url: ServoUrl,

let request = FetchRequestInit {
url: url,
origin: document.url().clone(),
origin: document.origin().immutable().clone(),
type_: RequestType::Image,
pipeline_id: Some(document.global().pipeline_id()),
.. FetchRequestInit::default()
@@ -1919,6 +1919,12 @@ impl ScriptThread {
ROUTER.route_ipc_receiver_to_mpsc_sender(ipc_timer_event_port,
self.timer_event_chan.clone());

let origin = if final_url.as_str() == "about:blank" {
incomplete.origin.clone()
} else {
MutableOrigin::new(final_url.origin())
};

// Create the window and document objects.
let window = Window::new(self.js_runtime.clone(),
MainThreadScriptChan(sender.clone()),
@@ -1942,7 +1948,7 @@ impl ScriptThread {
incomplete.pipeline_id,
incomplete.parent_info,
incomplete.window_size,
incomplete.origin.clone(),
origin,
self.webvr_thread.clone());

// Initialize the browsing context for the window.
@@ -2274,13 +2280,13 @@ impl ScriptThread {
destination: Destination::Document,
credentials_mode: CredentialsMode::Include,
use_url_credentials: true,
origin: load_data.url.clone(),
pipeline_id: Some(id),
referrer_url: load_data.referrer_url,
referrer_policy: load_data.referrer_policy,
headers: load_data.headers,
body: load_data.data,
redirect_mode: RedirectMode::Manual,
origin: incomplete.origin.immutable().clone(),
.. RequestInit::default()
};

@@ -261,7 +261,7 @@ impl<'a> StylesheetLoader<'a> {
Some(CorsSettings::Anonymous) => CredentialsMode::CredentialsSameOrigin,
_ => CredentialsMode::Include,
},
origin: document.url(),
origin: document.origin().immutable().clone(),
pipeline_id: Some(self.elem.global().pipeline_id()),
referrer_url: Some(document.url()),
referrer_policy: referrer_policy,
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.