Bug 780370 - Remap objects from standard prototypes even if they're explicitly exposed. r=mrbkap
authorBobby Holley <bobbyholley@gmail.com>
Thu, 23 Aug 2012 19:07:14 -0700
changeset 106387 c766aa050f0f95d3645da1a3152a9a9152506b4d
parent 106386 20a3b0eeb776ba57f2219fc49470c5716f07b849
child 106388 281b36542acd08890101493a1bac38af711e42db
push id1989
push userakeybl@mozilla.com
push dateTue, 28 Aug 2012 00:20:43 +0000
treeherdermozilla-aurora@a8e95ae10ea7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmrbkap
bugs780370
milestone17.0a1
Bug 780370 - Remap objects from standard prototypes even if they're explicitly exposed. r=mrbkap
js/xpconnect/tests/unit/test_bug780370.js
js/xpconnect/tests/unit/xpcshell.ini
js/xpconnect/wrappers/AccessCheck.cpp
js/xpconnect/wrappers/ChromeObjectWrapper.cpp
new file mode 100644
--- /dev/null
+++ b/js/xpconnect/tests/unit/test_bug780370.js
@@ -0,0 +1,20 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * 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/. */
+
+/* See https://bugzilla.mozilla.org/show_bug.cgi?id=780370 */
+
+const Cu = Components.utils;
+
+// Use a COW to expose a function from a standard prototype, and make sure it's
+// still remapped.
+
+function run_test()
+{
+  var sb = Cu.Sandbox("http://www.example.com");
+  sb.obj = { foo: 42, __exposedProps__: { hasOwnProperty: 'r' } };
+  do_check_eq(Cu.evalInSandbox('typeof obj.foo', sb), 'undefined', "COW works as expected");
+  do_check_true(Cu.evalInSandbox('obj.hasOwnProperty === Object.prototype.hasOwnProperty', sb),
+                "Remapping happens even when the property is explicitly exposed");
+  do_check_eq(Cu.evalInSandbox('Object.prototype.bar = 10; obj.bar', sb), 10);
+}
--- a/js/xpconnect/tests/unit/xpcshell.ini
+++ b/js/xpconnect/tests/unit/xpcshell.ini
@@ -7,16 +7,17 @@ tail =
 [test_bug451678.js]
 [test_bug596580.js]
 [test_bug604362.js]
 [test_bug608142.js]
 [test_bug641378.js]
 [test_bug677864.js]
 [test_bug711404.js]
 [test_bug778409.js]
+[test_bug780370.js]
 [test_bug_442086.js]
 [test_file.js]
 [test_import.js]
 [test_import_fail.js]
 [test_js_weak_references.js]
 [test_reflect_parse.js]
 [test_localeCompare.js]
 # Bug 676965: test fails consistently on Android
--- a/js/xpconnect/wrappers/AccessCheck.cpp
+++ b/js/xpconnect/wrappers/AccessCheck.cpp
@@ -473,16 +473,17 @@ ExposedPropertiesOnly::check(JSContext *
         return false;
     }
 
     JSObject *hallpass = &exposedProps.toObject();
 
     Access access = NO_ACCESS;
 
     JSPropertyDescriptor desc;
+    memset(&desc, 0, sizeof(desc));
     if (!JS_GetPropertyDescriptorById(cx, hallpass, id, JSRESOLVE_QUALIFIED, &desc)) {
         return false; // Error
     }
     if (desc.obj == NULL || !(desc.attrs & JSPROP_ENUMERATE)) {
         JSAutoCompartment wrapperAC(cx, wrapper);
         return PermitIfUniversalXPConnect(cx, id, act, perm); // Deny
     }
 
--- a/js/xpconnect/wrappers/ChromeObjectWrapper.cpp
+++ b/js/xpconnect/wrappers/ChromeObjectWrapper.cpp
@@ -9,30 +9,45 @@ namespace xpc {
 // from the wrappee's scope.
 //
 // One of the reasons for doing this is to allow standard operations like
 // chromeArray.forEach(..) to Just Work without explicitly listing them in
 // __exposedProps__. Since proxies don't automatically inherit behavior from
 // their prototype, we have to instrument the traps to do this manually.
 ChromeObjectWrapper ChromeObjectWrapper::singleton;
 
+static bool
+PropIsFromStandardPrototype(JSContext *cx, JSPropertyDescriptor *desc)
+{
+    MOZ_ASSERT(desc->obj);
+    JSObject *unwrapped = js::UnwrapObject(desc->obj);
+    JSAutoCompartment ac(cx, unwrapped);
+    return JS_IdentifyClassPrototype(cx, unwrapped) != JSProto_Null;
+}
+
 bool
 ChromeObjectWrapper::getPropertyDescriptor(JSContext *cx, JSObject *wrapper,
                                            jsid id, bool set,
                                            js::PropertyDescriptor *desc)
 {
     // First, try the lookup on the base wrapper. This can throw for various
     // reasons, including sets (gets fail silently). There's nothing we can really
     // do for sets, so we can conveniently propagate any exception we hit here.
     desc->obj = NULL;
     if (!ChromeObjectWrapperBase::getPropertyDescriptor(cx, wrapper, id,
                                                         set, desc)) {
         return false;
     }
 
+    // If the property is something that can be found on a standard prototype,
+    // prefer the one we'll get via the prototype chain in the content
+    // compartment.
+    if (desc->obj && PropIsFromStandardPrototype(cx, desc))
+        desc->obj = NULL;
+
     // If we found something, were doing a set, or have no proto, we're done.
     JSObject *wrapperProto = JS_GetPrototype(wrapper);
     if (desc->obj || set || !wrapperProto)
         return true;
 
     // If not, try doing the lookup on the prototype.
     JS_ASSERT(js::IsObjectInContextCompartment(wrapper, cx));
     return JS_GetPropertyDescriptorById(cx, wrapperProto, id, 0, desc);
@@ -48,33 +63,52 @@ ChromeObjectWrapper::has(JSContext *cx, 
     // If we found something or have no prototype, we're done.
     JSObject *wrapperProto = JS_GetPrototype(wrapper);
     if (*bp || !wrapperProto)
         return true;
 
     // Try the prototype if that failed.
     JS_ASSERT(js::IsObjectInContextCompartment(wrapper, cx));
     JSPropertyDescriptor desc;
+    memset(&desc, 0, sizeof(desc));
     if (!JS_GetPropertyDescriptorById(cx, wrapperProto, id, 0, &desc))
         return false;
     *bp = !!desc.obj;
     return true;
 }
 
 bool
 ChromeObjectWrapper::get(JSContext *cx, JSObject *wrapper, JSObject *receiver,
                          jsid id, js::Value *vp)
 {
-    // Try the lookup on the base wrapper.
-    if (!ChromeObjectWrapperBase::get(cx, wrapper, receiver, id, vp))
+    // Start with a call to getPropertyDescriptor. We unfortunately need to do
+    // this because the call signature of ::get doesn't give us any way to
+    // determine the object upon which the property was found.
+    JSPropertyDescriptor desc;
+    memset(&desc, 0, sizeof(desc));
+    if (!ChromeObjectWrapperBase::getPropertyDescriptor(cx, wrapper, id, false,
+                                                        &desc)) {
         return false;
+    }
 
-    // If we found something or have no proto, we're done.
+    // Only call through to the get trap on the underlying object if we'll find
+    // something, and if what we'll find is not on a standard prototype.
+    vp->setUndefined();
+    if (desc.obj && !PropIsFromStandardPrototype(cx, &desc)) {
+        // Call the get trap.
+        if (!ChromeObjectWrapperBase::get(cx, wrapper, receiver, id, vp))
+            return false;
+        // If we found something, we're done.
+        if (!vp->isUndefined())
+            return true;
+    }
+
+    // If we have no proto, we're done.
     JSObject *wrapperProto = JS_GetPrototype(wrapper);
-    if (!vp->isUndefined() || !wrapperProto)
+    if (!wrapperProto)
         return true;
 
     // Try the prototype.
     JS_ASSERT(js::IsObjectInContextCompartment(wrapper, cx));
     return js::GetGeneric(cx, wrapperProto, receiver, id, vp);
 }
 
 }