Added some same-origin-domain checks.

components/script/devtools.rs

@@ -10,7 +10,6 @@ use dom::bindings::codegen::Bindings::CSSStyleDeclarationBinding::CSSStyleDeclar
 use dom::bindings::codegen::Bindings::DOMRectBinding::DOMRectMethods;
 use dom::bindings::codegen::Bindings::DocumentBinding::DocumentMethods;
 use dom::bindings::codegen::Bindings::ElementBinding::ElementMethods;
-use dom::bindings::codegen::Bindings::LocationBinding::LocationMethods;
 use dom::bindings::codegen::Bindings::WindowBinding::WindowMethods;
 use dom::bindings::conversions::{ConversionResult, FromJSValConvertible, jsstring_to_str};
 use dom::bindings::inheritance::Castable;
@@ -261,6 +260,6 @@ pub fn handle_request_animation_frame(documents: &Documents,
 pub fn handle_reload(documents: &Documents,
                      id: PipelineId) {
     if let Some(win) = documents.find_window(id) {
-        win.Location().Reload();
+        win.Location().reload_without_origin_check();
     }
 }

components/script/dom/document.rs

@@ -3396,7 +3396,10 @@ impl DocumentMethods for Document {
 
         let entry_responsible_document = GlobalScope::entry().as_window().Document();
 
-        if !self.origin.same_origin(&entry_responsible_document.origin) {
+        // This check should probably be same-origin-domain
+        // https://github.com/whatwg/html/issues/2282
+        // https://github.com/whatwg/html/pull/2288
+        if !self.origin.same_origin_domain(&entry_responsible_document.origin) {
             // Step 4.
             return Err(Error::Security);
         }

components/script/dom/history.rs

@@ -4,7 +4,6 @@
 
 use dom::bindings::codegen::Bindings::HistoryBinding;
 use dom::bindings::codegen::Bindings::HistoryBinding::HistoryMethods;
-use dom::bindings::codegen::Bindings::LocationBinding::LocationMethods;
 use dom::bindings::codegen::Bindings::WindowBinding::WindowMethods;
 use dom::bindings::inheritance::Castable;
 use dom::bindings::js::{JS, Root};
@@ -65,7 +64,7 @@ impl HistoryMethods for History {
         } else if delta < 0 {
             TraversalDirection::Back(-delta as usize)
         } else {
-            self.window.Location().Reload();
+            self.window.Location().reload_without_origin_check();
             return;
         };
 

components/script/dom/htmliframeelement.rs

@@ -327,20 +327,6 @@ impl HTMLIFrameElement {
             false
         }
     }
-
-    pub fn get_content_window(&self) -> Option<Root<Window>> {
-        self.pipeline_id.get()
-            .and_then(|pipeline_id| ScriptThread::find_document(pipeline_id))
-            .and_then(|document| {
-                let current_global = GlobalScope::current();
-                let current_document = current_global.as_window().Document();
-                if document.origin().same_origin(current_document.origin()) {
-                    Some(Root::from_ref(document.window()))
-                } else {
-                    None
-                }
-            })
-    }
 }
 
 pub trait HTMLIFrameElementLayoutMethods {
@@ -512,15 +498,33 @@ impl HTMLIFrameElementMethods for HTMLIFrameElement {
 
     // https://html.spec.whatwg.org/multipage/#dom-iframe-contentwindow
     fn GetContentWindow(&self) -> Option<Root<BrowsingContext>> {
-        match self.get_content_window() {
-            Some(ref window) => Some(window.browsing_context()),
-            None => None
+        if self.pipeline_id.get().is_some() {
+            ScriptThread::find_browsing_context(self.frame_id)
+        } else {
+            None
         }
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-iframe-contentdocument
+    // https://html.spec.whatwg.org/multipage/#concept-bcc-content-document
     fn GetContentDocument(&self) -> Option<Root<Document>> {
-        self.get_content_window().map(|window| window.Document())
+        // Step 1.
+        let pipeline_id = match self.pipeline_id.get() {
+            None => return None,
+            Some(pipeline_id) => pipeline_id,
+        };
+        // Step 2-3.
+        let document = match ScriptThread::find_document(pipeline_id) {
+            None => return None,
+            Some(document) => document,
+        };
+        // Step 4.
+        let current = GlobalScope::current().as_window().Document();
+        if !current.origin().same_origin_domain(document.origin()) {
+            return None;
+        }
+        // Step 5.
+        Some(document)
     }
 
     // Experimental mozbrowser implementation is based on the webidl

components/script/dom/location.rs

@@ -4,10 +4,12 @@
 
 use dom::bindings::codegen::Bindings::LocationBinding;
 use dom::bindings::codegen::Bindings::LocationBinding::LocationMethods;
-use dom::bindings::error::{Error, ErrorResult};
+use dom::bindings::codegen::Bindings::WindowBinding::WindowBinding::WindowMethods;
+use dom::bindings::error::{Error, ErrorResult, Fallible};
 use dom::bindings::js::{JS, Root};
 use dom::bindings::reflector::{Reflector, reflect_dom_object};
 use dom::bindings::str::{DOMString, USVString};
+use dom::globalscope::GlobalScope;
 use dom::urlhelper::UrlHelper;
 use dom::window::Window;
 use dom_struct::dom_struct;
@@ -43,11 +45,27 @@ impl Location {
         setter(&mut url, value);
         self.window.load_url(url, false, false, None);
     }
+
+    fn check_same_origin_domain(&self) -> ErrorResult {
+        let entry_document = GlobalScope::entry().as_window().Document();
+        let this_document = self.window.Document();
+        if entry_document.origin().same_origin_domain(this_document.origin()) {
+            Ok(())
+        } else {
+            Err(Error::Security)
+        }
+    }
+
+    // https://html.spec.whatwg.org/multipage/#dom-location-reload
+    pub fn reload_without_origin_check(&self) {
+        self.window.load_url(self.get_url(), true, true, None);
+    }
 }
 
 impl LocationMethods for Location {
     // https://html.spec.whatwg.org/multipage/#dom-location-assign
     fn Assign(&self, url: USVString) -> ErrorResult {
+        // Note: no call to self.check_same_origin_domain()
         // TODO: per spec, we should use the _API base URL_ specified by the
         //       _entry settings object_.
         let base_url = self.window.get_url();
@@ -60,12 +78,15 @@ impl LocationMethods for Location {
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-location-reload
-    fn Reload(&self) {
+    fn Reload(&self) -> ErrorResult {
+        try!(self.check_same_origin_domain());
         self.window.load_url(self.get_url(), true, true, None);
+        Ok(())
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-location-replace
     fn Replace(&self, url: USVString) -> ErrorResult {
+        // Note: no call to self.check_same_origin_domain()
         // TODO: per spec, we should use the _API base URL_ specified by the
         //       _entry settings object_.
         let base_url = self.window.get_url();
@@ -78,97 +99,124 @@ impl LocationMethods for Location {
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-location-hash
-    fn Hash(&self) -> USVString {
-        UrlHelper::Hash(&self.get_url())
+    fn GetHash(&self) -> Fallible<USVString> {
+        try!(self.check_same_origin_domain());
+        Ok(UrlHelper::Hash(&self.get_url()))
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-location-hash
-    fn SetHash(&self, mut value: USVString) {
+    fn SetHash(&self, mut value: USVString) -> ErrorResult {
         if value.0.is_empty() {
             value = USVString("#".to_owned());
         }
+        try!(self.check_same_origin_domain());
         self.set_url_component(value, UrlHelper::SetHash);
+        Ok(())
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-location-host
-    fn Host(&self) -> USVString {
-        UrlHelper::Host(&self.get_url())
+    fn GetHost(&self) -> Fallible<USVString> {
+        try!(self.check_same_origin_domain());
+        Ok(UrlHelper::Host(&self.get_url()))
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-location-host
-    fn SetHost(&self, value: USVString) {
+    fn SetHost(&self, value: USVString) -> ErrorResult {
+        try!(self.check_same_origin_domain());
         self.set_url_component(value, UrlHelper::SetHost);
+        Ok(())
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-location-origin
-    fn Origin(&self) -> USVString {
-        UrlHelper::Origin(&self.get_url())
+    fn GetOrigin(&self) -> Fallible<USVString> {
+        try!(self.check_same_origin_domain());
+        Ok(UrlHelper::Origin(&self.get_url()))
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-location-hostname
-    fn Hostname(&self) -> USVString {
-        UrlHelper::Hostname(&self.get_url())
+    fn GetHostname(&self) -> Fallible<USVString> {
+        try!(self.check_same_origin_domain());
+        Ok(UrlHelper::Hostname(&self.get_url()))
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-location-hostname
-    fn SetHostname(&self, value: USVString) {
+    fn SetHostname(&self, value: USVString) -> ErrorResult {
+        try!(self.check_same_origin_domain());
         self.set_url_component(value, UrlHelper::SetHostname);
+        Ok(())
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-location-href
-    fn Href(&self) -> USVString {
-        UrlHelper::Href(&self.get_url())
+    fn GetHref(&self) -> Fallible<USVString> {
+        try!(self.check_same_origin_domain());
+        Ok(UrlHelper::Href(&self.get_url()))
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-location-href
-    fn SetHref(&self, value: USVString) {
-        if let Ok(url) = self.window.get_url().join(&value.0) {
-            self.window.load_url(url, false, false, None);
-        }
+    fn SetHref(&self, value: USVString) -> ErrorResult {
+        // Note: no call to self.check_same_origin_domain()
+        let url = match self.window.get_url().join(&value.0) {
+            Ok(url) => url,
+            Err(e) => return Err(Error::Type(format!("Couldn't parse URL: {}", e))),
+        };
+        self.window.load_url(url, false, false, None);
+        Ok(())
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-location-pathname
-    fn Pathname(&self) -> USVString {
-        UrlHelper::Pathname(&self.get_url())
+    fn GetPathname(&self) -> Fallible<USVString> {
+        try!(self.check_same_origin_domain());
+        Ok(UrlHelper::Pathname(&self.get_url()))
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-location-pathname
-    fn SetPathname(&self, value: USVString) {
+    fn SetPathname(&self, value: USVString) -> ErrorResult {
+        try!(self.check_same_origin_domain());
         self.set_url_component(value, UrlHelper::SetPathname);
+        Ok(())
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-location-port
-    fn Port(&self) -> USVString {
-        UrlHelper::Port(&self.get_url())
+    fn GetPort(&self) -> Fallible<USVString> {
+        try!(self.check_same_origin_domain());
+        Ok(UrlHelper::Port(&self.get_url()))
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-location-port
-    fn SetPort(&self, value: USVString) {
+    fn SetPort(&self, value: USVString) -> ErrorResult {
+        try!(self.check_same_origin_domain());
         self.set_url_component(value, UrlHelper::SetPort);
+        Ok(())
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-location-protocol
-    fn Protocol(&self) -> USVString {
-        UrlHelper::Protocol(&self.get_url())
+    fn GetProtocol(&self) -> Fallible<USVString> {
+        try!(self.check_same_origin_domain());
+        Ok(UrlHelper::Protocol(&self.get_url()))
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-location-protocol
-    fn SetProtocol(&self, value: USVString) {
+    fn SetProtocol(&self, value: USVString) -> ErrorResult {
+        try!(self.check_same_origin_domain());
         self.set_url_component(value, UrlHelper::SetProtocol);
+        Ok(())
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-location-href
-    fn Stringifier(&self) -> DOMString {
-        DOMString::from(self.Href().0)
+    fn Stringifier(&self) -> Fallible<DOMString> {
+        Ok(DOMString::from(try!(self.GetHref()).0))
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-location-search
-    fn Search(&self) -> USVString {
-        UrlHelper::Search(&self.get_url())
+    fn GetSearch(&self) -> Fallible<USVString> {
+        try!(self.check_same_origin_domain());
+        Ok(UrlHelper::Search(&self.get_url()))
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-location-search
-    fn SetSearch(&self, value: USVString) {
+    fn SetSearch(&self, value: USVString) -> ErrorResult {
+        try!(self.check_same_origin_domain());
         self.set_url_component(value, UrlHelper::SetSearch);
+        Ok(())
     }
 }

components/script/dom/webidls/Location.webidl

@@ -4,26 +4,24 @@
 
 // https://html.spec.whatwg.org/multipage/#location
 [Exposed=Window, Unforgeable] interface Location {
-  /*stringifier*/ attribute USVString href;
-  readonly attribute USVString origin;
-           attribute USVString protocol;
-           attribute USVString host;
-           attribute USVString hostname;
-           attribute USVString port;
-           attribute USVString pathname;
-           attribute USVString search;
-           attribute USVString hash;
+  /*stringifier*/ [Throws] attribute USVString href;
+  [Throws] readonly attribute USVString origin;
+  [Throws]          attribute USVString protocol;
+  [Throws]          attribute USVString host;
+  [Throws]          attribute USVString hostname;
+  [Throws]          attribute USVString port;
+  [Throws]          attribute USVString pathname;
+  [Throws]          attribute USVString search;
+  [Throws]          attribute USVString hash;
 
-  [Throws]
-  void assign(USVString url);
-  [Throws]
-  void replace(USVString url);
-  void reload();
+  [Throws] void assign(USVString url);
+  [Throws] void replace(USVString url);
+  [Throws] void reload();
 
   //[SameObject] readonly attribute USVString[] ancestorOrigins;
 
   // This is only doing as well as gecko right now.
   // https://github.com/servo/servo/issues/7590 is on file for
   // adding attribute stringifier support.
-  stringifier;
+  [Throws] stringifier;
 };

components/script/dom/window.rs

@@ -42,7 +42,7 @@ use dom::location::Location;
 use dom::mediaquerylist::{MediaQueryList, WeakMediaQueryListVec};
 use dom::messageevent::MessageEvent;
 use dom::navigator::Navigator;
-use dom::node::{Node, from_untrusted_node_address, window_from_node, NodeDamage};
+use dom::node::{Node, from_untrusted_node_address, document_from_node, window_from_node, NodeDamage};
 use dom::performance::Performance;
 use dom::promise::Promise;
 use dom::screen::Screen;
@@ -528,7 +528,20 @@ impl WindowMethods for Window {
 
     // https://html.spec.whatwg.org/multipage/#dom-frameelement
     fn GetFrameElement(&self) -> Option<Root<Element>> {
-        self.browsing_context().frame_element().map(Root::from_ref)
+        // Steps 1-3.
+        if let Some(context) = self.browsing_context.get() {
+            // Step 4-5.
+            if let Some(container) = context.frame_element() {
+                // Step 6.
+                let container_doc = document_from_node(container);
+                let current_doc = GlobalScope::current().as_window().Document();
+                if current_doc.origin().same_origin_domain(container_doc.origin()) {
+                    // Step 7.
+                    return Some(Root::from_ref(container));
+                }
+            }
+        }
+        None
     }
 
     // https://html.spec.whatwg.org/multipage/#dom-navigator

components/script/script_thread.rs

@@ -27,7 +27,6 @@ use dom::bindings::cell::DOMRefCell;
 use dom::bindings::codegen::Bindings::CSSStyleDeclarationBinding::CSSStyleDeclarationMethods;
 use dom::bindings::codegen::Bindings::DocumentBinding::{DocumentMethods, DocumentReadyState};
 use dom::bindings::codegen::Bindings::EventBinding::EventInit;
-use dom::bindings::codegen::Bindings::LocationBinding::LocationMethods;
 use dom::bindings::codegen::Bindings::TransitionEventBinding::TransitionEventInit;
 use dom::bindings::codegen::Bindings::WindowBinding::WindowMethods;
 use dom::bindings::conversions::{ConversionResult, FromJSValConvertible, StringificationBehavior};
@@ -644,6 +643,14 @@ impl ScriptThread {
         }))
     }
 
+    pub fn find_browsing_context(id: FrameId) -> Option<Root<BrowsingContext>> {
+        SCRIPT_THREAD_ROOT.with(|root| root.get().and_then(|script_thread| {
+            let script_thread = unsafe { &*script_thread };
+            script_thread.browsing_contexts.borrow().get(&id)
+                .map(|context| Root::from_ref(&**context))
+        }))
+    }
+
     /// Creates a new script thread.
     pub fn new(state: InitialScriptState,
                port: Receiver<MainThreadScriptMsg>,
@@ -2101,7 +2108,7 @@ impl ScriptThread {
     fn handle_reload(&self, pipeline_id: PipelineId) {
         let window = self.documents.borrow().find_window(pipeline_id);
         if let Some(window) = window {
-            window.Location().Reload();
+            window.Location().reload_without_origin_check();
         }
     }
 

components/script/webdriver_handlers.rs

@@ -113,29 +113,24 @@ pub fn handle_get_frame_id(documents: &Documents,
                            pipeline: PipelineId,
                            webdriver_frame_id: WebDriverFrameId,
                            reply: IpcSender<Result<Option<PipelineId>, ()>>) {
-    let window = match webdriver_frame_id {
+    let result = match webdriver_frame_id {
         WebDriverFrameId::Short(_) => {
             // This isn't supported yet
             Ok(None)
         },
         WebDriverFrameId::Element(x) => {
-            match find_node_by_unique_id(documents, pipeline, x) {
-                Some(ref node) => {
-                    match node.downcast::<HTMLIFrameElement>() {
-                        Some(ref elem) => Ok(elem.get_content_window()),
-                        None => Err(())
-                    }
-                },
-                None => Err(())
-            }
+            find_node_by_unique_id(documents, pipeline, x)
+                .and_then(|node| node.downcast::<HTMLIFrameElement>().map(|elem| elem.pipeline_id()))
+                .ok_or(())
         },
         WebDriverFrameId::Parent => {
-            documents.find_window(pipeline).map(|window| window.parent()).ok_or(())
+            documents.find_window(pipeline)
+                .map(|window| window.parent_info().map(|(parent_id, _)| parent_id))
+                .ok_or(())
         }
     };
 
-    let frame_id = window.map(|x| x.map(|x| x.upcast::<GlobalScope>().pipeline_id()));
-    reply.send(frame_id).unwrap()
+    reply.send(result).unwrap()
 }
 
 pub fn handle_find_element_css(documents: &Documents, pipeline: PipelineId, selector: String,