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

"javascript:" urls: execute in correct global scope #17083

Merged
merged 8 commits into from Sep 9, 2017
Next

"javascript:" urls: run in correct global

Make some changes to javascript scheme url evaluation to bring it
closer to
https://html.spec.whatwg.org/multipage/browsing-the-web.html#navigating-across-documents

- Evaluate the js before the page load so that it happens in the
  correct `window` global.
- If the result is not a string, the response should be a 204.

This required saving some data in load_data, since it's not
possible to modify the response at the point where we're evaluating
the js.
  • Loading branch information
danielj41 committed Aug 23, 2017
commit fa3e9ab2444daab2c67b7e25fd6a5f88264b4ddb
@@ -87,8 +87,8 @@ use script_runtime::{CommonScriptMsg, ScriptChan, ScriptThreadEventCategory};
use script_runtime::{ScriptPort, StackRootTLS, get_reports, new_rt_and_cx};
use script_traits::{CompositorEvent, ConstellationControlMsg};
use script_traits::{DocumentActivity, DiscardBrowsingContext, EventResult};
use script_traits::{InitialScriptState, LayoutMsg, LoadData, MouseButton, MouseEventType, MozBrowserEvent};
use script_traits::{NewLayoutInfo, ScriptToConstellationChan, ScriptMsg, UpdatePipelineIdReason};
use script_traits::{InitialScriptState, JsEvalResult, LayoutMsg, LoadData, MouseButton, MouseEventType};
use script_traits::{MozBrowserEvent, NewLayoutInfo, ScriptToConstellationChan, ScriptMsg, UpdatePipelineIdReason};
use script_traits::{ScriptThreadFactory, TimerEvent, TimerSchedulerMsg, TimerSource};
use script_traits::{TouchEventType, TouchId, UntrustedNodeAddress, WindowSizeData, WindowSizeType};
use script_traits::CompositorEvent::{KeyEvent, MouseButtonEvent, MouseMoveEvent, ResizeEvent};
@@ -1506,7 +1506,7 @@ impl ScriptThread {
load_data.url.clone(),
origin);
if load_data.url.as_str() == "about:blank" {
self.start_page_load_about_blank(new_load);
self.start_page_load_about_blank(new_load, load_data.js_eval_result);
} else {
self.pre_page_load(new_load, load_data);
}
@@ -1677,6 +1677,18 @@ impl ScriptThread {
match idx {
Some(idx) => {
let load = self.incomplete_loads.borrow_mut().remove(idx);

// https://html.spec.whatwg.org/multipage/#process-a-navigate-response
// 2. If response's status is 204 or 205, then abort these steps.
match metadata {
Some(Metadata { status: Some((204 ... 205, _)), .. }) => {
// TODO: This leaves the page in a broken state where you can't follow
// other links. Fix this.
return None;
},
_ => ()
};

metadata.map(|meta| self.load(meta, load))
}
None => {
@@ -2116,39 +2128,7 @@ impl ScriptThread {
// Notify devtools that a new script global exists.
self.notify_devtools(document.Title(), final_url.clone(), (incomplete.pipeline_id, None));

let is_javascript = incomplete.url.scheme() == "javascript";
let parse_input = if is_javascript {
use url::percent_encoding::percent_decode;

// Turn javascript: URL into JS code to eval, according to the steps in
// https://html.spec.whatwg.org/multipage/#javascript-protocol

// This slice of the URL’s serialization is equivalent to (5.) to (7.):
// Start with the scheme data of the parsed URL;
// append question mark and query component, if any;
// append number sign and fragment component if any.
let encoded = &incomplete.url[Position::BeforePath..];

// Percent-decode (8.) and UTF-8 decode (9.)
let script_source = percent_decode(encoded.as_bytes()).decode_utf8_lossy();

// Script source is ready to be evaluated (11.)
unsafe {
let _ac = JSAutoCompartment::new(self.get_cx(), window.reflector().get_jsobject().get());
rooted!(in(self.get_cx()) let mut jsval = UndefinedValue());
window.upcast::<GlobalScope>().evaluate_js_on_global_with_result(
&script_source, jsval.handle_mut());
let strval = DOMString::from_jsval(self.get_cx(),
jsval.handle(),
StringificationBehavior::Empty);
match strval {
Ok(ConversionResult::Success(s)) => s,
_ => DOMString::new(),
}
}
} else {
DOMString::new()
};
let parse_input = DOMString::new();

document.set_https_state(metadata.https_state);

@@ -2327,16 +2307,64 @@ impl ScriptThread {
/// for the given pipeline (specifically the "navigate" algorithm).
fn handle_navigate(&self, parent_pipeline_id: PipelineId,
browsing_context_id: Option<BrowsingContextId>,
load_data: LoadData,
mut load_data: LoadData,
replace: bool) {
match browsing_context_id {
Some(browsing_context_id) => {
let iframe = self.documents.borrow().find_iframe(parent_pipeline_id, browsing_context_id);
if let Some(iframe) = iframe {
iframe.navigate_or_reload_child_browsing_context(Some(load_data), NavigationType::Regular, replace);
}

// TODO: Test javascript: urls in iframes

This comment has been minimized.

Copy link
@jdm

jdm Aug 24, 2017

Member

This looks more like a "note to self"; want to file an issue for this and remove the comment instead?

This comment has been minimized.

Copy link
@danielj41

danielj41 Aug 25, 2017

Author Contributor

I was planning on looking into it (and this TODO) a bit more in this pull request. I'll open new issues if they aren't easy to fix!

This comment has been minimized.

Copy link
@danielj41

danielj41 Aug 29, 2017

Author Contributor

I added support for <iframe src="javascript:'something'"></iframe> in 6ae6031. This was necessary to prevent a regression from master.

I fixed the other TODO (can't follow links after executing a javascript url) in ff786a0.

}
None => {
let is_javascript = load_data.url.scheme() == "javascript";
if is_javascript {
use url::percent_encoding::percent_decode;

This comment has been minimized.

Copy link
@jdm

jdm Aug 24, 2017

Member

Move this to with the other use statements at the top of the file.

This comment has been minimized.

Copy link
@danielj41

danielj41 Aug 29, 2017

Author Contributor

Fixed in 5d28dd6


// Turn javascript: URL into JS code to eval, according to the steps in
// https://html.spec.whatwg.org/multipage/#javascript-protocol

// This slice of the URL’s serialization is equivalent to (5.) to (7.):
// Start with the scheme data of the parsed URL;
// append question mark and query component, if any;
// append number sign and fragment component if any.
let encoded = &load_data.url[Position::BeforePath..];

// Percent-decode (8.) and UTF-8 decode (9.)
let script_source = percent_decode(encoded.as_bytes()).decode_utf8_lossy();

// Script source is ready to be evaluated (11.)
let window = self.documents.borrow().find_window(parent_pipeline_id);
if let Some(window) = window {
let _ac = JSAutoCompartment::new(self.get_cx(), window.reflector().get_jsobject().get());
rooted!(in(self.get_cx()) let mut jsval = UndefinedValue());
window.upcast::<GlobalScope>().evaluate_js_on_global_with_result(
&script_source, jsval.handle_mut());

load_data.js_eval_result = if jsval.get().is_string() {
unsafe {
let strval = DOMString::from_jsval(self.get_cx(),
jsval.handle(),
StringificationBehavior::Empty);
match strval {
Ok(ConversionResult::Success(s)) => {
Some(JsEvalResult::Ok(String::from(s).as_bytes().to_vec()))
},
_ => None,
}
}
} else {
Some(JsEvalResult::NoContent)
};
}
}

if is_javascript {
load_data.url = ServoUrl::parse("about:blank").unwrap();

This comment has been minimized.

Copy link
@jdm

jdm Aug 24, 2017

Member

This block can be merged with the previous one.

This comment has been minimized.

Copy link
@danielj41

danielj41 Aug 29, 2017

Author Contributor

I tried to make this look cleaner in 5d28dd6. I couldn't merge it with the previous block because there was an immutable reference to url in that block (though I suppose I could change that).

}

self.script_sender
.send((parent_pipeline_id, ScriptMsg::LoadUrl(load_data, replace)))
.unwrap();
@@ -2375,7 +2403,7 @@ impl ScriptThread {
/// argument until a notification is received that the fetch is complete.
fn pre_page_load(&self, incomplete: InProgressLoad, load_data: LoadData) {
let id = incomplete.pipeline_id.clone();
let mut req_init = RequestInit {
let req_init = RequestInit {
url: load_data.url.clone(),
method: load_data.method,
destination: Destination::Document,
@@ -2391,10 +2419,6 @@ impl ScriptThread {
.. RequestInit::default()
};

if req_init.url.scheme() == "javascript" {
req_init.url = ServoUrl::parse("about:blank").unwrap();
}

let context = ParserContext::new(id, load_data.url);
self.incomplete_parser_contexts.borrow_mut().push((id, context));

@@ -2434,7 +2458,7 @@ impl ScriptThread {

/// Synchronously fetch `about:blank`. Stores the `InProgressLoad`
/// argument until a notification is received that the fetch is complete.
fn start_page_load_about_blank(&self, incomplete: InProgressLoad) {
fn start_page_load_about_blank(&self, incomplete: InProgressLoad, js_eval_result: Option<JsEvalResult>) {
let id = incomplete.pipeline_id;

self.incomplete_loads.borrow_mut().push(incomplete);
@@ -2444,8 +2468,20 @@ impl ScriptThread {

let mut meta = Metadata::default(url);
meta.set_content_type(Some(&mime!(Text / Html)));

// If this page load is the result of a javascript scheme url, map
// the evaluation result into a response.
let chunk = match js_eval_result {
Some(JsEvalResult::Ok(content)) => content,
Some(JsEvalResult::NoContent) => {
meta.status = Some((204, b"No Content".to_vec()));
vec![]
},
None => vec![]
};

context.process_response(Ok(FetchMetadata::Unfiltered(meta)));
context.process_response_chunk(vec![]);
context.process_response_chunk(chunk);
context.process_response_eof(Ok(()));
}

@@ -146,12 +146,24 @@ pub struct LoadData {
pub headers: Headers,
/// The data.
pub data: Option<Vec<u8>>,
/// The result of evaluating a javascript scheme url.
pub js_eval_result: Option<JsEvalResult>,
/// The referrer policy.
pub referrer_policy: Option<ReferrerPolicy>,
/// The referrer URL.
pub referrer_url: Option<ServoUrl>,
}

/// The result of evaluating a javascript scheme url.
#[derive(Clone, Debug, Deserialize, Serialize)]
pub enum JsEvalResult {
/// The js evaluation had a non-string result, 204 status code.
/// https://html.spec.whatwg.org/multipage/#navigate 12.11
NoContent,
/// The js evaluation had a string result.
Ok(Vec<u8>)
}

impl LoadData {
/// Create a new `LoadData` object.
pub fn new(url: ServoUrl,
@@ -165,6 +177,7 @@ impl LoadData {
method: Method::Get,
headers: Headers::new(),
data: None,
js_eval_result: None,
referrer_policy: referrer_policy,
referrer_url: referrer_url,
}
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.