script: Stop trusting pointers to DOM nodes that layout provides.

Pointers to DOM nodes from layout could go stale if incremental reflow
does not correctly destroy dead nodes. Therefore, we ask the JavaScript
garbage collector to verify that each DOM node is indeed a valid pointer
before calling event handlers on it, and fail otherwise.

3 files changed, 51 insertions, 29 deletions

diff --git a/src/components/script/dom/node.rs b/src/components/script/dom/node.rs
index a98f89b4817..dc5de2aac24 100644
--- a/src/components/script/dom/node.rs
+++ b/src/components/script/dom/node.rs
@@ -7,6 +7,7 @@
 use dom::bindings::utils::{Reflectable, Reflector, reflect_dom_object};
 use dom::bindings::utils::{DOMString, null_str_as_empty};
 use dom::bindings::utils::{ErrorResult, Fallible, NotFound, HierarchyRequest};
+use dom::bindings::utils;
 use dom::characterdata::CharacterData;
 use dom::document::{AbstractDocument, DocumentTypeId};
 use dom::documenttype::DocumentType;
@@ -17,15 +18,15 @@ use dom::htmliframeelement::HTMLIFrameElement;
 use dom::htmlimageelement::HTMLImageElement;
 use dom::nodelist::{NodeList};
 use dom::text::Text;
+use layout_interface::{LayoutChan, ReapLayoutDataMsg, UntrustedNodeAddress};
 
-use js::jsapi::{JSObject, JSContext};
-
-use layout_interface::{LayoutChan, ReapLayoutDataMsg};
-
-use std::cast;
+use js::jsapi::{JSContext, JSObject, JSRuntime};
+use js::jsfriendapi;
 use std::cast::transmute;
+use std::cast;
 use std::cell::{RefCell, Ref, RefMut};
 use std::iter::Filter;
+use std::libc::uintptr_t;
 use std::util;
 use std::unstable::raw::Box;
 
@@ -241,6 +242,22 @@ impl AbstractNode {
             _ => false
         }
     }
+
+    /// If the given untrusted node address represents a valid DOM node in the given runtime,
+    /// returns it.
+    pub fn from_untrusted_node_address(runtime: *JSRuntime, candidate: UntrustedNodeAddress)
+                                       -> AbstractNode {
+        unsafe {
+            let candidate: uintptr_t = cast::transmute(candidate);
+            let object: *JSObject = jsfriendapi::bindgen::JS_GetAddressableObject(runtime,
+                                                                                  candidate);
+            if object.is_null() {
+                fail!("Attempted to create an `AbstractNode` from an invalid pointer!")
+            }
+            let boxed_node: *mut Box<Node> = utils::unwrap(object);
+            AbstractNode::from_box(boxed_node)
+        }
+    }
 }
 
 impl<'a> AbstractNode {
diff --git a/src/components/script/layout_interface.rs b/src/components/script/layout_interface.rs
index 613e61a3429..b0b9ec479f4 100644
--- a/src/components/script/layout_interface.rs
+++ b/src/components/script/layout_interface.rs
@@ -14,8 +14,9 @@ use geom::rect::Rect;
 use geom::size::Size2D;
 use script_task::{ScriptChan};
 use servo_util::geometry::Au;
-use std::comm::{Chan, SharedChan};
 use std::cmp;
+use std::comm::{Chan, SharedChan};
+use std::libc::c_void;
 use style::Stylesheet;
 
 /// Asynchronous messages that script can send to layout.
@@ -58,9 +59,13 @@ pub enum LayoutQuery {
     HitTestQuery(AbstractNode, Point2D<f32>, Chan<Result<HitTestResponse, ()>>),
 }
 
+/// The address of a node. Layout sends these back. They must be validated via
+/// `from_untrusted_node_address` before they can be used, because we do not trust layout.
+pub type UntrustedNodeAddress = *c_void;
+
 pub struct ContentBoxResponse(Rect<Au>);
 pub struct ContentBoxesResponse(~[Rect<Au>]);
-pub struct HitTestResponse(AbstractNode);
+pub struct HitTestResponse(UntrustedNodeAddress);
 
 /// Determines which part of the 
 #[deriving(Eq, Ord)]
diff --git a/src/components/script/script_task.rs b/src/components/script/script_task.rs
index faec6d8031b..9ca9af2f8a6 100644
--- a/src/components/script/script_task.rs
+++ b/src/components/script/script_task.rs
@@ -850,31 +850,31 @@ impl ScriptTask {
                 }
                 let (port, chan) = Chan::new();
                 match page.query_layout(HitTestQuery(root.unwrap(), point, chan), port) {
-                    Ok(node) => match node {
-                        HitTestResponse(node) => {
-                            debug!("clicked on {:s}", node.debug_str());
-                            let mut node = node;
-                            // traverse node generations until a node that is an element is found
-                            while !node.is_element() {
-                                match node.parent_node() {
-                                    Some(parent) => {
-                                        node = parent;
-                                    }
-                                    None => break
-                                }
-                            }
-                            if node.is_element() {
-                                node.with_imm_element(|element| {
-                                    if "a" == element.tag_name {
-                                        self.load_url_from_element(page, element)
-                                    }
-                                })
+                    Ok(HitTestResponse(node_address)) => {
+                        debug!("node address is {:?}", node_address);
+                        let mut node = AbstractNode::from_untrusted_node_address(self.js_runtime
+                                                                                     .ptr,
+                                                                                 node_address);
+                        debug!("clicked on {:s}", node.debug_str());
+
+                        // Traverse node generations until a node that is an element is
+                        // found.
+                        while !node.is_element() {
+                            match node.parent_node() {
+                                Some(parent) => node = parent,
+                                None => break,
                             }
                         }
+
+                        if node.is_element() {
+                            node.with_imm_element(|element| {
+                                if "a" == element.tag_name {
+                                    self.load_url_from_element(page, element)
+                                }
+                            })
+                        }
                     },
-                    Err(()) => {
-                        debug!("layout query error");
-                    }
+                    Err(()) => debug!("layout query error"),
                 }
             }
             MouseDownEvent(..) => {}