From 96403247218ce628a39b954c361d0707d0d0f97d Mon Sep 17 00:00:00 2001 From: Josh Matthews Date: Tue, 10 Sep 2013 15:43:28 -0700 Subject: [PATCH 1/2] Make sandboxed iframes run with different script tasks. --- src/components/main/constellation.rs | 16 +++--- src/components/msg/constellation_msg.rs | 8 ++- src/components/script/dom/element.rs | 16 ++++-- .../script/dom/htmliframeelement.rs | 49 +++++++++++++++++-- .../script/html/hubbub_html_parser.rs | 8 +-- src/components/script/script_task.rs | 12 +++-- src/support/spidermonkey/rust-mozjs | 2 +- src/test/html/test_sandboxed.html | 14 ++++++ src/test/html/test_sandboxed_iframe.html | 24 +++++++++ 9 files changed, 125 insertions(+), 24 deletions(-) create mode 100644 src/test/html/test_sandboxed.html create mode 100644 src/test/html/test_sandboxed_iframe.html diff --git a/src/components/main/constellation.rs b/src/components/main/constellation.rs index 7ae714783068..271b72526bb4 100644 --- a/src/components/main/constellation.rs +++ b/src/components/main/constellation.rs @@ -13,9 +13,9 @@ use geom::size::Size2D; use geom::rect::Rect; use gfx::opts::Opts; use pipeline::Pipeline; -use servo_msg::constellation_msg::{ConstellationChan, ExitMsg, FrameRectMsg}; +use servo_msg::constellation_msg::{ConstellationChan, ExitMsg, FrameRectMsg, IFrameSandboxState}; use servo_msg::constellation_msg::{InitLoadUrlMsg, LoadIframeUrlMsg, LoadUrlMsg}; -use servo_msg::constellation_msg::{Msg, NavigateMsg, NavigationType}; +use servo_msg::constellation_msg::{Msg, NavigateMsg, NavigationType, IFrameUnsandboxed}; use servo_msg::constellation_msg::{PipelineId, RendererReadyMsg, ResizedWindowMsg, SubpageId}; use servo_msg::constellation_msg; use script::script_task::{SendEventMsg, ResizeInactiveMsg, ExecuteMsg}; @@ -328,8 +328,8 @@ impl Constellation { FrameRectMsg(pipeline_id, subpage_id, rect) => { self.handle_frame_rect_msg(pipeline_id, subpage_id, rect); } - LoadIframeUrlMsg(url, source_pipeline_id, subpage_id, size_future) => { - self.handle_load_iframe_url_msg(url, source_pipeline_id, subpage_id, size_future); + LoadIframeUrlMsg(url, source_pipeline_id, subpage_id, size_future, sandbox) => { + self.handle_load_iframe_url_msg(url, source_pipeline_id, subpage_id, size_future, sandbox); } // Load a new page, usually -- but not always -- from a mouse click or typed url // If there is already a pending page (self.pending_frames), it will not be overridden; @@ -455,7 +455,8 @@ impl Constellation { url: Url, source_pipeline_id: PipelineId, subpage_id: SubpageId, - size_future: Future>) { + size_future: Future>, + sandbox: IFrameSandboxState) { // A message from the script associated with pipeline_id that it has // parsed an iframe during html parsing. This iframe will result in a // new pipeline being spawned and a frame tree being added to pipeline_id's @@ -489,9 +490,10 @@ impl Constellation { source's Url is None. There should never be a LoadUrlIframeMsg from a pipeline that was never given a url to load."); + let same_script = (source_url.host == url.host && + source_url.port == url.port) && sandbox == IFrameUnsandboxed; // FIXME(tkuehn): Need to follow the standardized spec for checking same-origin - let pipeline = @mut if (source_url.host == url.host && - source_url.port == url.port) { + let pipeline = @mut if same_script { debug!("Constellation: loading same-origin iframe at %?", url); // Reuse the script task if same-origin url's Pipeline::with_script(next_pipeline_id, diff --git a/src/components/msg/constellation_msg.rs b/src/components/msg/constellation_msg.rs index 51bcbb08c467..c5b4dbb48d07 100644 --- a/src/components/msg/constellation_msg.rs +++ b/src/components/msg/constellation_msg.rs @@ -27,12 +27,18 @@ impl ConstellationChan { } } +#[deriving(Eq)] +pub enum IFrameSandboxState { + IFrameSandboxed, + IFrameUnsandboxed +} + pub enum Msg { ExitMsg(Chan<()>), InitLoadUrlMsg(Url), FrameRectMsg(PipelineId, SubpageId, Rect), LoadUrlMsg(PipelineId, Url, Future>), - LoadIframeUrlMsg(Url, PipelineId, SubpageId, Future>), + LoadIframeUrlMsg(Url, PipelineId, SubpageId, Future>, IFrameSandboxState), NavigateMsg(NavigationDirection), RendererReadyMsg(PipelineId), ResizedWindowMsg(Size2D), diff --git a/src/components/script/dom/element.rs b/src/components/script/dom/element.rs index be475bd1d857..0bb776628c94 100644 --- a/src/components/script/dom/element.rs +++ b/src/components/script/dom/element.rs @@ -139,9 +139,9 @@ impl<'self> Element { return None; } - pub fn set_attr(&mut self, name: &DOMString, value: &DOMString) { - let name = name.to_str(); - let value_cell = Cell::new(value.to_str()); + pub fn set_attr(&mut self, raw_name: &DOMString, raw_value: &DOMString) { + let name = raw_name.to_str(); + let value_cell = Cell::new(raw_value.to_str()); let mut found = false; for attr in self.attrs.mut_iter() { if eq_slice(attr.name, name) { @@ -158,7 +158,15 @@ impl<'self> Element { self.style_attribute = Some( Stylesheet::from_attribute( FromStr::from_str("http://www.example.com/").unwrap(), - value.get_ref())); + raw_value.get_ref())); + } + + //XXXjdm We really need something like a vtable so we can call AfterSetAttr. + // This hardcoding is awful. + if self.parent.abstract.unwrap().is_iframe_element() { + do self.parent.abstract.unwrap().with_mut_iframe_element |iframe| { + iframe.AfterSetAttr(raw_name, raw_value); + } } match self.parent.owner_doc { diff --git a/src/components/script/dom/htmliframeelement.rs b/src/components/script/dom/htmliframeelement.rs index 288a988928aa..4c45e570b541 100644 --- a/src/components/script/dom/htmliframeelement.rs +++ b/src/components/script/dom/htmliframeelement.rs @@ -2,7 +2,7 @@ * 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 dom::bindings::utils::{DOMString, null_string, ErrorResult}; +use dom::bindings::utils::{DOMString, null_string, ErrorResult, str}; use dom::document::AbstractDocument; use dom::htmlelement::HTMLElement; use dom::windowproxy::WindowProxy; @@ -11,14 +11,26 @@ use geom::rect::Rect; use servo_msg::constellation_msg::{ConstellationChan, FrameRectMsg, PipelineId, SubpageId}; +use std::ascii::StrAsciiExt; use std::comm::ChanOne; use extra::url::Url; use std::util::replace; +enum SandboxAllowance { + AllowNothing = 0x00, + AllowSameOrigin = 0x01, + AllowTopNavigation = 0x02, + AllowForms = 0x04, + AllowScripts = 0x08, + AllowPointerLock = 0x10, + AllowPopups = 0x20 +} + pub struct HTMLIFrameElement { parent: HTMLElement, frame: Option, size: Option, + sandbox: Option } struct IFrameSize { @@ -39,6 +51,11 @@ impl IFrameSize { } } +impl HTMLIFrameElement { + pub fn is_sandboxed(&self) -> bool { + self.sandbox.is_some() + } +} impl HTMLIFrameElement { pub fn Src(&self) -> DOMString { @@ -63,10 +80,32 @@ impl HTMLIFrameElement { } pub fn Sandbox(&self) -> DOMString { - null_string - } - - pub fn SetSandbox(&self, _sandbox: &DOMString) { + self.parent.parent.GetAttribute(&str(~"sandbox")) + } + + pub fn SetSandbox(&mut self, sandbox: &DOMString) { + let mut rv = Ok(()); + self.parent.parent.SetAttribute(&str(~"sandbox"), sandbox, &mut rv); + } + + pub fn AfterSetAttr(&mut self, name: &DOMString, value: &DOMString) { + let name = name.to_str(); + if "sandbox" == name { + let mut modes = AllowNothing as u8; + let words = value.to_str(); + for word in words.split_iter(' ') { + modes |= match word.to_ascii_lower().as_slice() { + "allow-same-origin" => AllowSameOrigin, + "allow-forms" => AllowForms, + "allow-pointer-lock" => AllowPointerLock, + "allow-popups" => AllowPopups, + "allow-scripts" => AllowScripts, + "allow-top-navigation" => AllowTopNavigation, + _ => AllowNothing + } as u8; + } + self.sandbox = Some(modes); + } } pub fn AllowFullscreen(&self) -> bool { diff --git a/src/components/script/html/hubbub_html_parser.rs b/src/components/script/html/hubbub_html_parser.rs index 64340a28b77a..42c9e43cf482 100644 --- a/src/components/script/html/hubbub_html_parser.rs +++ b/src/components/script/html/hubbub_html_parser.rs @@ -93,7 +93,7 @@ enum JSMessage { /// Messages generated by the HTML parser upon discovery of additional resources pub enum HtmlDiscoveryMessage { HtmlDiscoveredStyle(Stylesheet), - HtmlDiscoveredIFrame((Url, SubpageId, Future>)), + HtmlDiscoveredIFrame((Url, SubpageId, Future>, bool)), HtmlDiscoveredScript(JSResult) } @@ -272,7 +272,7 @@ pub fn build_element_from_tag(cx: *JSContext, tag: &str) -> AbstractNode { page.layout_chan.send(AddStylesheetMsg(sheet)); } - Some(HtmlDiscoveredIFrame((iframe_url, subpage_id, size_future))) => { + Some(HtmlDiscoveredIFrame((iframe_url, subpage_id, size_future, sandboxed))) => { page.next_subpage_id = SubpageId(*subpage_id + 1); + let sandboxed = if sandboxed { + IFrameSandboxed + } else { + IFrameUnsandboxed + }; self.constellation_chan.send(LoadIframeUrlMsg(iframe_url, pipeline_id, subpage_id, - size_future)); + size_future, + sandboxed)); } None => break } diff --git a/src/support/spidermonkey/rust-mozjs b/src/support/spidermonkey/rust-mozjs index 353ef00db32e..a9bc3b5cf968 160000 --- a/src/support/spidermonkey/rust-mozjs +++ b/src/support/spidermonkey/rust-mozjs @@ -1 +1 @@ -Subproject commit 353ef00db32e526f054f1f7899cd93425cfb1c59 +Subproject commit a9bc3b5cf968e279cfb082e58207c60e4f4a1a9b diff --git a/src/test/html/test_sandboxed.html b/src/test/html/test_sandboxed.html new file mode 100644 index 000000000000..f0c485b57946 --- /dev/null +++ b/src/test/html/test_sandboxed.html @@ -0,0 +1,14 @@ + + + +hiiiiiiiii + +byeeeeeeeeeeee + + diff --git a/src/test/html/test_sandboxed_iframe.html b/src/test/html/test_sandboxed_iframe.html new file mode 100644 index 000000000000..210a6134a1bb --- /dev/null +++ b/src/test/html/test_sandboxed_iframe.html @@ -0,0 +1,24 @@ + + + +hi there + + From 636c30affeec0578a9ecb36adf9b6ec702d56bb9 Mon Sep 17 00:00:00 2001 From: Josh Matthews Date: Wed, 11 Sep 2013 12:18:23 -0700 Subject: [PATCH 2/2] Add trace hooks for Window and Document, and remove explicit rooting for the root DOM node. Fixes #901. --- .../script/dom/bindings/codegen/Bindings.conf | 5 ++- .../dom/bindings/codegen/CodegenRust.py | 28 ++++++++-------- src/components/script/dom/bindings/node.rs | 2 +- src/components/script/dom/document.rs | 33 ++++++++++--------- src/components/script/dom/window.rs | 32 ++++++++++++++++-- src/components/script/script_task.rs | 3 -- src/support/spidermonkey/rust-mozjs | 2 +- 7 files changed, 66 insertions(+), 39 deletions(-) diff --git a/src/components/script/dom/bindings/codegen/Bindings.conf b/src/components/script/dom/bindings/codegen/Bindings.conf index 0ef1d9d502a7..709509792e19 100644 --- a/src/components/script/dom/bindings/codegen/Bindings.conf +++ b/src/components/script/dom/bindings/codegen/Bindings.conf @@ -143,6 +143,7 @@ DOMInterfaces = { 'Document': { 'nativeType': 'AbstractDocument', 'pointerType': '', + 'customTrace': 'trace' }, 'DOMParser': { @@ -228,6 +229,7 @@ DOMInterfaces = { 'HTMLDocument': { 'nativeType': 'AbstractDocument', 'pointerType': '', + 'customTrace': 'trace' }, 'HTMLFormElement': { @@ -433,7 +435,8 @@ DOMInterfaces = { }], 'Window': { - 'createGlobal': True + 'createGlobal': True, + 'customTrace': 'trace' }, 'WindowProxy': { diff --git a/src/components/script/dom/bindings/codegen/CodegenRust.py b/src/components/script/dom/bindings/codegen/CodegenRust.py index 0e01fcef059b..44946cef9910 100644 --- a/src/components/script/dom/bindings/codegen/CodegenRust.py +++ b/src/components/script/dom/bindings/codegen/CodegenRust.py @@ -2221,13 +2221,13 @@ def define(self): static Class: DOMJSClass = DOMJSClass { base: JSClass { name: &Class_name as *u8 as *libc::c_char, flags: JSCLASS_IS_DOMJSCLASS | %s | (((%s) & JSCLASS_RESERVED_SLOTS_MASK) << JSCLASS_RESERVED_SLOTS_SHIFT), //JSCLASS_HAS_RESERVED_SLOTS(%s), - addProperty: %s, /* addProperty */ - delProperty: crust::JS_PropertyStub, /* delProperty */ - getProperty: crust::JS_PropertyStub, /* getProperty */ - setProperty: crust::JS_StrictPropertyStub, /* setProperty */ - enumerate: crust::JS_EnumerateStub, - resolve: crust::JS_ResolveStub, - convert: crust::JS_ConvertStub, + addProperty: Some(%s), /* addProperty */ + delProperty: Some(crust::JS_PropertyStub), /* delProperty */ + getProperty: Some(crust::JS_PropertyStub), /* getProperty */ + setProperty: Some(crust::JS_StrictPropertyStub), /* setProperty */ + enumerate: Some(crust::JS_EnumerateStub), + resolve: Some(crust::JS_ResolveStub), + convert: Some(crust::JS_ConvertStub), finalize: Some(%s), /* finalize */ checkAccess: None, /* checkAccess */ call: None, /* call */ @@ -2269,13 +2269,13 @@ def define(self): static PrototypeClass: JSClass = JSClass { name: &PrototypeClassName__ as *u8 as *libc::c_char, flags: (1 & JSCLASS_RESERVED_SLOTS_MASK) << JSCLASS_RESERVED_SLOTS_SHIFT, //JSCLASS_HAS_RESERVED_SLOTS(1) - addProperty: crust::JS_PropertyStub, /* addProperty */ - delProperty: crust::JS_PropertyStub, /* delProperty */ - getProperty: crust::JS_PropertyStub, /* getProperty */ - setProperty: crust::JS_StrictPropertyStub, /* setProperty */ - enumerate: crust::JS_EnumerateStub, - resolve: crust::JS_ResolveStub, - convert: crust::JS_ConvertStub, + addProperty: Some(crust::JS_PropertyStub), /* addProperty */ + delProperty: Some(crust::JS_PropertyStub), /* delProperty */ + getProperty: Some(crust::JS_PropertyStub), /* getProperty */ + setProperty: Some(crust::JS_StrictPropertyStub), /* setProperty */ + enumerate: Some(crust::JS_EnumerateStub), + resolve: Some(crust::JS_ResolveStub), + convert: Some(crust::JS_ConvertStub), finalize: None, /* finalize */ checkAccess: None, /* checkAccess */ call: None, /* call */ diff --git a/src/components/script/dom/bindings/node.rs b/src/components/script/dom/bindings/node.rs index 05aff3793caa..a453d5d66c82 100644 --- a/src/components/script/dom/bindings/node.rs +++ b/src/components/script/dom/bindings/node.rs @@ -130,7 +130,7 @@ impl Traceable for Node { } } } - error!("tracing %p?:", self.wrapper.get_wrapper()); + debug!("tracing %p?:", self.wrapper.get_wrapper()); trace_node(tracer, self.parent_node, "parent"); trace_node(tracer, self.first_child, "first child"); trace_node(tracer, self.last_child, "last child"); diff --git a/src/components/script/dom/document.rs b/src/components/script/dom/document.rs index 51352b2faebf..4333ea5b9342 100644 --- a/src/components/script/dom/document.rs +++ b/src/components/script/dom/document.rs @@ -5,6 +5,7 @@ use dom::bindings::codegen::DocumentBinding; use dom::bindings::utils::{DOMString, WrapperCache, ErrorResult, null_string, str}; use dom::bindings::utils::{BindingObject, CacheableWrapper, rust_box, DerivedWrapper}; +use dom::bindings::utils::Traceable; use dom::element::{Element}; use dom::element::{HTMLHtmlElementTypeId, HTMLHeadElementTypeId, HTMLTitleElementTypeId}; use dom::event::Event; @@ -18,13 +19,15 @@ use dom::window::Window; use dom::windowproxy::WindowProxy; use dom::htmltitleelement::HTMLTitleElement; use html::hubbub_html_parser::build_element_from_tag; -use js::jsapi::{JS_AddObjectRoot, JS_RemoveObjectRoot, JSObject, JSContext, JSVal}; +use js::jsapi::{JSObject, JSContext, JSVal}; +use js::jsapi::{JSTRACE_OBJECT, JSTracer, JS_CallTracer}; use js::glue::RUST_OBJECT_TO_JSVAL; use servo_util::tree::TreeNodeRef; use std::cast; use std::ptr; use std::str::eq_slice; +use std::libc; pub trait WrappableDocument { fn init_wrapper(@mut self, cx: *JSContext); @@ -89,14 +92,6 @@ pub struct Document { impl Document { #[fixed_stack_segment] pub fn new(root: AbstractNode, window: Option<@mut Window>, doctype: DocumentType) -> Document { - let compartment = unsafe {(*window.get_ref().page).js_info.get_ref().js_compartment }; - do root.with_base |base| { - assert!(base.wrapper.get_wrapper().is_not_null()); - let rootable = base.wrapper.get_rootable(); - unsafe { - JS_AddObjectRoot(compartment.cx.ptr, rootable); - } - } Document { root: root, wrapper: WrapperCache::new(), @@ -460,17 +455,23 @@ impl Document { window.content_changed() } } +} +impl Traceable for Document { #[fixed_stack_segment] - pub fn teardown(&self) { + fn trace(&self, tracer: *mut JSTracer) { unsafe { - let compartment = (*self.window.get_ref().page).js_info.get_ref().js_compartment; - do self.root.with_base |node| { - assert!(node.wrapper.get_wrapper().is_not_null()); - let rootable = node.wrapper.get_rootable(); - JS_RemoveObjectRoot(compartment.cx.ptr, rootable); + (*tracer).debugPrinter = ptr::null(); + (*tracer).debugPrintIndex = -1; + do "root".to_c_str().with_ref |name| { + (*tracer).debugPrintArg = name as *libc::c_void; + debug!("tracing root node"); + do self.root.with_base |node| { + JS_CallTracer(tracer as *JSTracer, + node.wrapper.wrapper, + JSTRACE_OBJECT as u32); + } } } } } - diff --git a/src/components/script/dom/window.rs b/src/components/script/dom/window.rs index 00c4b63f539a..e6164a96cb43 100644 --- a/src/components/script/dom/window.rs +++ b/src/components/script/dom/window.rs @@ -3,7 +3,7 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ use dom::bindings::codegen::WindowBinding; -use dom::bindings::utils::{WrapperCache, DOMString, null_string}; +use dom::bindings::utils::{WrapperCache, DOMString, null_string, Traceable}; use dom::bindings::utils::{CacheableWrapper, BindingObject}; use dom::document::AbstractDocument; use dom::node::{AbstractNode, ScriptView}; @@ -14,8 +14,8 @@ use script_task::{ExitMsg, FireTimerMsg, Page, ScriptChan}; use servo_msg::compositor_msg::ScriptListener; use js::glue::*; -use js::jsapi::{JSObject, JSContext, JS_DefineProperty}; -use js::jsapi::{JSPropertyOp, JSStrictPropertyOp}; +use js::jsapi::{JSObject, JSContext, JS_DefineProperty, JS_CallTracer}; +use js::jsapi::{JSPropertyOp, JSStrictPropertyOp, JSTracer, JSTRACE_OBJECT}; use js::{JSVAL_NULL, JSPROP_ENUMERATE}; use std::cast; @@ -25,6 +25,7 @@ use std::comm::SharedChan; use std::io; use std::ptr; use std::int; +use std::libc; use std::rt::rtio::RtioTimer; use std::rt::io::timer::Timer; use js::jsapi::JSVal; @@ -214,3 +215,28 @@ impl Window { } } +impl Traceable for Window { + #[fixed_stack_segment] + fn trace(&self, tracer: *mut JSTracer) { + debug!("tracing window"); + unsafe { + match (*self.page).frame { + Some(frame) => { + do frame.document.with_base |doc| { + (*tracer).debugPrinter = ptr::null(); + (*tracer).debugPrintIndex = -1; + do "document".to_c_str().with_ref |name| { + (*tracer).debugPrintArg = name as *libc::c_void; + debug!("tracing document"); + JS_CallTracer(tracer as *JSTracer, + doc.wrapper.wrapper, + JSTRACE_OBJECT as u32); + } + } + } + None => () + } + } + } +} + diff --git a/src/components/script/script_task.rs b/src/components/script/script_task.rs index 9d45dc7d8dcb..4025de24bb49 100644 --- a/src/components/script/script_task.rs +++ b/src/components/script/script_task.rs @@ -555,9 +555,6 @@ impl ScriptTask { fn handle_exit_msg(&mut self) { for page in self.page_tree.iter() { page.join_layout(); - do page.frame.unwrap().document.with_mut_base |doc| { - doc.teardown(); - } page.layout_chan.send(layout_interface::ExitMsg); } self.compositor.close(); diff --git a/src/support/spidermonkey/rust-mozjs b/src/support/spidermonkey/rust-mozjs index a9bc3b5cf968..5b83f5f77a62 160000 --- a/src/support/spidermonkey/rust-mozjs +++ b/src/support/spidermonkey/rust-mozjs @@ -1 +1 @@ -Subproject commit a9bc3b5cf968e279cfb082e58207c60e4f4a1a9b +Subproject commit 5b83f5f77a6215864738a5636a9bc9db2a097070