From fbc8938bee86cdc8c9427092b8b2c5dd8286a47b Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Wed, 24 Aug 2016 14:49:52 +0200 Subject: [PATCH 1/3] Mark get_property_on_prototype and has_property_on_prototype as unsafe --- components/script/dom/bindings/utils.rs | 59 ++++++++++++------------- 1 file changed, 29 insertions(+), 30 deletions(-) diff --git a/components/script/dom/bindings/utils.rs b/components/script/dom/bindings/utils.rs index ac902a5d68c..53b69fc0b40 100644 --- a/components/script/dom/bindings/utils.rs +++ b/components/script/dom/bindings/utils.rs @@ -127,32 +127,29 @@ pub type ProtoOrIfaceArray = [*mut JSObject; PROTO_OR_IFACE_LENGTH]; /// set to true and `*vp` to the value, otherwise `*found` is set to false. /// /// Returns false on JSAPI failure. -pub fn get_property_on_prototype(cx: *mut JSContext, - proxy: HandleObject, - id: HandleId, - found: *mut bool, - vp: MutableHandleValue) - -> bool { - unsafe { - // let proto = GetObjectProto(proxy); - rooted!(in(cx) let mut proto = ptr::null_mut()); - if !JS_GetPrototype(cx, proxy, proto.handle_mut()) || proto.is_null() { - *found = false; - return true; - } - let mut has_property = false; - if !JS_HasPropertyById(cx, proto.handle(), id, &mut has_property) { - return false; - } - *found = has_property; - let no_output = vp.ptr.is_null(); - if !has_property || no_output { - return true; - } - - rooted!(in(cx) let receiver = ObjectValue(&*proxy.get())); - JS_ForwardGetPropertyTo(cx, proto.handle(), id, receiver.handle(), vp) +pub unsafe fn get_property_on_prototype(cx: *mut JSContext, + proxy: HandleObject, + id: HandleId, + found: *mut bool, + vp: MutableHandleValue) + -> bool { + rooted!(in(cx) let mut proto = ptr::null_mut()); + if !JS_GetPrototype(cx, proxy, proto.handle_mut()) || proto.is_null() { + *found = false; + return true; } + let mut has_property = false; + if !JS_HasPropertyById(cx, proto.handle(), id, &mut has_property) { + return false; + } + *found = has_property; + let no_output = vp.ptr.is_null(); + if !has_property || no_output { + return true; + } + + rooted!(in(cx) let receiver = ObjectValue(&*proxy.get())); + JS_ForwardGetPropertyTo(cx, proto.handle(), id, receiver.handle(), vp) } /// Get an array index from the given `jsid`. Returns `None` if the given @@ -285,12 +282,14 @@ pub fn set_dictionary_property(cx: *mut JSContext, } /// Returns whether `proxy` has a property `id` on its prototype. -pub fn has_property_on_prototype(cx: *mut JSContext, proxy: HandleObject, id: HandleId) -> bool { - // MOZ_ASSERT(js::IsProxy(proxy) && js::GetProxyHandler(proxy) == handler); +pub unsafe fn has_property_on_prototype(cx: *mut JSContext, + proxy: HandleObject, + id: HandleId) + -> bool { let mut found = false; - !get_property_on_prototype(cx, proxy, id, &mut found, unsafe { - MutableHandleValue::from_marked_location(ptr::null_mut()) - }) || found + !get_property_on_prototype( + cx, proxy, id, &mut found, + MutableHandleValue::from_marked_location(ptr::null_mut())) || found } /// Drop the resources held by reserved slots of a global object From f70fa989547a256255ae74264ac6e906709b72f4 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Wed, 24 Aug 2016 15:29:12 +0200 Subject: [PATCH 2/3] Make has_property_on_prototype fallible --- .../dom/bindings/codegen/CodegenRust.py | 35 +++++++++++++------ components/script/dom/bindings/utils.rs | 13 ++++--- 2 files changed, 33 insertions(+), 15 deletions(-) diff --git a/components/script/dom/bindings/codegen/CodegenRust.py b/components/script/dom/bindings/codegen/CodegenRust.py index 8e04fc76a10..4eb02f796e0 100644 --- a/components/script/dom/bindings/codegen/CodegenRust.py +++ b/components/script/dom/bindings/codegen/CodegenRust.py @@ -4724,10 +4724,17 @@ class CGDOMJSProxyHandler_getOwnPropertyDescriptor(CGAbstractExternMethod): # Once we start supporting OverrideBuiltins we need to make # ResolveOwnProperty or EnumerateOwnProperties filter out named # properties that shadow prototype properties. - namedGet = ("\n" + - "if RUST_JSID_IS_STRING(id) && !has_property_on_prototype(cx, proxy, id) {\n" + - CGIndenter(CGProxyNamedGetter(self.descriptor, templateValues)).define() + "\n" + - "}\n") + namedGet = """ +if RUST_JSID_IS_STRING(id) { + let mut has_on_proto = false; + if !has_property_on_prototype(cx, proxy, id, &mut has_on_proto) { + return false; + } + if !has_on_proto { + %s + } +} +""" % CGIndenter(CGProxyNamedGetter(self.descriptor, templateValues), 8).define() else: namedGet = "" @@ -4952,12 +4959,20 @@ class CGDOMJSProxyHandler_hasOwn(CGAbstractExternMethod): namedGetter = self.descriptor.operations['NamedGetter'] if namedGetter: - named = ("if RUST_JSID_IS_STRING(id) && !has_property_on_prototype(cx, proxy, id) {\n" + - CGIndenter(CGProxyNamedGetter(self.descriptor)).define() + "\n" + - " *bp = found;\n" - " return true;\n" - "}\n" + - "\n") + named = """\ +if RUST_JSID_IS_STRING(id) { + let mut has_on_proto = false; + if !has_property_on_prototype(cx, proxy, id, &mut has_on_proto) { + return false; + } + if !has_on_proto { + %s + *bp = found; + return true; + } +} + +""" % CGIndenter(CGProxyNamedGetter(self.descriptor), 8).define() else: named = "" diff --git a/components/script/dom/bindings/utils.rs b/components/script/dom/bindings/utils.rs index 53b69fc0b40..666dba519a4 100644 --- a/components/script/dom/bindings/utils.rs +++ b/components/script/dom/bindings/utils.rs @@ -284,12 +284,15 @@ pub fn set_dictionary_property(cx: *mut JSContext, /// Returns whether `proxy` has a property `id` on its prototype. pub unsafe fn has_property_on_prototype(cx: *mut JSContext, proxy: HandleObject, - id: HandleId) + id: HandleId, + found: &mut bool) -> bool { - let mut found = false; - !get_property_on_prototype( - cx, proxy, id, &mut found, - MutableHandleValue::from_marked_location(ptr::null_mut())) || found + rooted!(in(cx) let mut proto = ptr::null_mut()); + if !JS_GetPrototype(cx, proxy, proto.handle_mut()) { + return false; + } + assert!(!proto.is_null()); + JS_HasPropertyById(cx, proto.handle(), id, found) } /// Drop the resources held by reserved slots of a global object From 6c1167b1e2ec6939f88878aa21eebaab7dbe547e Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Wed, 24 Aug 2016 15:34:35 +0200 Subject: [PATCH 3/3] Pass the receiver to get_property_on_prototype (fixes #11600) --- components/script/dom/bindings/codegen/CodegenRust.py | 2 +- components/script/dom/bindings/utils.rs | 6 +++--- .../HTMLCollection-as-proto-length-get-throws.html.ini | 5 ----- 3 files changed, 4 insertions(+), 9 deletions(-) delete mode 100644 tests/wpt/metadata/dom/collections/HTMLCollection-as-proto-length-get-throws.html.ini diff --git a/components/script/dom/bindings/codegen/CodegenRust.py b/components/script/dom/bindings/codegen/CodegenRust.py index 4eb02f796e0..643262c0db2 100644 --- a/components/script/dom/bindings/codegen/CodegenRust.py +++ b/components/script/dom/bindings/codegen/CodegenRust.py @@ -5050,7 +5050,7 @@ if !expando.is_null() { %s let mut found = false; -if !get_property_on_prototype(cx, proxy, id, &mut found, vp) { +if !get_property_on_prototype(cx, proxy, receiver, id, &mut found, vp) { return false; } diff --git a/components/script/dom/bindings/utils.rs b/components/script/dom/bindings/utils.rs index 666dba519a4..030584c2755 100644 --- a/components/script/dom/bindings/utils.rs +++ b/components/script/dom/bindings/utils.rs @@ -29,7 +29,7 @@ use js::jsapi::{JS_GetProperty, JS_GetPrototype, JS_GetReservedSlot, JS_HasPrope use js::jsapi::{JS_HasPropertyById, JS_IsExceptionPending, JS_IsGlobalObject}; use js::jsapi::{JS_ResolveStandardClass, JS_SetProperty, ToWindowProxyIfWindow}; use js::jsapi::{JS_StringHasLatin1Chars, MutableHandleValue, ObjectOpResult}; -use js::jsval::{JSVal, ObjectValue, UndefinedValue}; +use js::jsval::{JSVal, UndefinedValue}; use js::rust::{GCMethods, ToString}; use libc; use std::ffi::CString; @@ -129,6 +129,7 @@ pub type ProtoOrIfaceArray = [*mut JSObject; PROTO_OR_IFACE_LENGTH]; /// Returns false on JSAPI failure. pub unsafe fn get_property_on_prototype(cx: *mut JSContext, proxy: HandleObject, + receiver: HandleValue, id: HandleId, found: *mut bool, vp: MutableHandleValue) @@ -148,8 +149,7 @@ pub unsafe fn get_property_on_prototype(cx: *mut JSContext, return true; } - rooted!(in(cx) let receiver = ObjectValue(&*proxy.get())); - JS_ForwardGetPropertyTo(cx, proto.handle(), id, receiver.handle(), vp) + JS_ForwardGetPropertyTo(cx, proto.handle(), id, receiver, vp) } /// Get an array index from the given `jsid`. Returns `None` if the given diff --git a/tests/wpt/metadata/dom/collections/HTMLCollection-as-proto-length-get-throws.html.ini b/tests/wpt/metadata/dom/collections/HTMLCollection-as-proto-length-get-throws.html.ini deleted file mode 100644 index 0e56cb9f2f3..00000000000 --- a/tests/wpt/metadata/dom/collections/HTMLCollection-as-proto-length-get-throws.html.ini +++ /dev/null @@ -1,5 +0,0 @@ -[HTMLCollection-as-proto-length-get-throws.html] - type: testharness - [HTMLcollection as a prototype should not allow getting .length on the base object] - expected: FAIL -