Bug 726949. Instead of using the given proto for the sandbox directly, use a proxy that forwards to the given proto but rebinds all getters/setters/methods to use the given proto, not the sandbox global, as this. r=bholley, a=tracking-firefox
☠☠ backed out by d9cb12986a3a ☠ ☠
authorBoris Zbarsky <bzbarsky@mit.edu>
Thu, 19 Apr 2012 14:19:41 -0400
changeset 95295 3e24a0ebd104a5d11712ab9a7879003db8601773
parent 95294 6294c13ee51e040d96b0b0fcc1d6335f28261758
child 95296 d585de9a3103dbdcca84b08e07d88049f381d182
push id886
push userlsblakk@mozilla.com
push dateMon, 04 Jun 2012 19:57:52 +0000
treeherdermozilla-beta@bbd8d5efd6d1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbholley, tracking-firefox
bugs726949
milestone14.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 726949. Instead of using the given proto for the sandbox directly, use a proxy that forwards to the given proto but rebinds all getters/setters/methods to use the given proto, not the sandbox global, as this. r=bholley, a=tracking-firefox The code in XPCQuickStubs.h just moved from XPCQuickStubs.cpp.
js/src/jswrapper.h
js/xpconnect/src/XPCComponents.cpp
js/xpconnect/src/XPCQuickStubs.cpp
js/xpconnect/src/XPCQuickStubs.h
js/xpconnect/tests/chrome/Makefile.in
js/xpconnect/tests/chrome/test_bug726949.xul
--- a/js/src/jswrapper.h
+++ b/js/src/jswrapper.h
@@ -46,17 +46,17 @@
 
 #include "jsapi.h"
 #include "jsproxy.h"
 
 namespace js {
 
 class DummyFrameGuard;
 
-/* Base class that just implements no-op forwarding methods for funamental
+/* Base class that just implements no-op forwarding methods for fundamental
  * traps. This is meant to be used as a base class for ProxyHandlers that
  * want transparent forwarding behavior but don't want to use the derived
  * traps and other baggage of js::Wrapper.
  */
 class JS_FRIEND_API(AbstractWrapper) : public ProxyHandler
 {
   public:
     explicit AbstractWrapper();
--- a/js/xpconnect/src/XPCComponents.cpp
+++ b/js/xpconnect/src/XPCComponents.cpp
@@ -3061,16 +3061,108 @@ WrapForSandbox(JSContext *cx, bool wantX
 // other. The only thing we need out of identity objects are unique addresses.
 class Identity : public nsISupports
 {
     NS_DECL_ISUPPORTS
 };
 
 NS_IMPL_ISUPPORTS0(Identity)
 
+class SandboxProxyHandler : public js::AbstractWrapper {
+public:
+    SandboxProxyHandler() : js::AbstractWrapper()
+    {
+    }
+
+    virtual bool getPropertyDescriptor(JSContext *cx, JSObject *proxy, jsid id,
+                                       bool set, PropertyDescriptor *desc);
+    virtual bool getOwnPropertyDescriptor(JSContext *cx, JSObject *proxy,
+                                          jsid id, bool set,
+                                          PropertyDescriptor *desc);
+};
+
+SandboxProxyHandler sandboxProxyHandler;
+
+template<typename Op>
+bool BindPropertyOp(JSContext *cx, JSObject *targetObj, Op& op,
+                    PropertyDescriptor *desc, jsid id, unsigned attrFlag)
+{
+    if (!op) {
+        return true;
+    }
+
+    JSObject *func;
+    if (desc->attrs & attrFlag) {
+        // Already an object
+        func = JS_FUNC_TO_DATA_PTR(JSObject *, op);
+    } else {
+        // We have an actual property op.  For getters, we use 0
+        // args, for setters we use 1 arg.
+        uint32_t args = (attrFlag == JSPROP_GETTER) ? 0 : 1;
+        func = GeneratePropertyOp(cx, desc->obj, id, args, op);
+        if (!func)
+            return false;
+    }
+    func = JS_BindCallable(cx, func, targetObj);
+    if (!func)
+        return false;
+    op = JS_DATA_TO_FUNC_PTR(Op, func);
+    desc->attrs |= attrFlag;
+    return true;
+}
+
+bool
+SandboxProxyHandler::getPropertyDescriptor(JSContext *cx, JSObject *proxy,
+                                           jsid id, bool set,
+                                           PropertyDescriptor *desc)
+{
+    JSObject *obj = wrappedObject(proxy);
+    JS_ASSERT(js::GetObjectCompartment(obj) == js::GetObjectCompartment(proxy));
+    // XXXbz Not sure about the JSRESOLVE_QUALIFIED here, but we have
+    // no way to tell for sure whether to use it.
+    if (!JS_GetPropertyDescriptorById(cx, obj, id,
+                                      (set ? JSRESOLVE_ASSIGNING : 0) | JSRESOLVE_QUALIFIED,
+                                      desc))
+        return false;
+
+    if (!desc->obj)
+        return true; // No property, nothing to do
+
+    // Now fix up the getter/setter/value as needed to be bound to desc->obj
+    if (!BindPropertyOp(cx, obj, desc->getter, desc, id, JSPROP_GETTER))
+        return false;
+    if (!BindPropertyOp(cx, obj, desc->setter, desc, id, JSPROP_SETTER))
+        return false;
+    if (desc->value.isObject()) {
+        JSObject* val = &desc->value.toObject();
+        if (JS_ObjectIsCallable(cx, val)) {
+            val = JS_BindCallable(cx, val, obj);
+            if (!val)
+                return false;
+            desc->value = ObjectValue(*val);
+        }
+    }
+
+    return true;
+}
+
+bool
+SandboxProxyHandler::getOwnPropertyDescriptor(JSContext *cx, JSObject *proxy,
+                                              jsid id, bool set,
+                                              PropertyDescriptor *desc)
+{
+    if (!getPropertyDescriptor(cx, proxy, id, set, desc))
+        return false;
+
+    if (desc->obj != wrappedObject(proxy))
+        desc->obj = nsnull;
+
+    return true;
+}
+
 nsresult
 xpc_CreateSandboxObject(JSContext * cx, jsval * vp, nsISupports *prinOrSop, JSObject *proto,
                         bool wantXrays, const nsACString &sandboxName, nsISupports *identityPtr)
 {
     // Create the sandbox global object
     nsresult rv;
     nsCOMPtr<nsIXPConnect> xpc(do_GetService(nsIXPConnect::GetCID(), &rv));
     if (NS_FAILED(rv))
@@ -3129,16 +3221,30 @@ xpc_CreateSandboxObject(JSContext * cx, 
 
             if (xpc::WrapperFactory::IsXrayWrapper(proto) && !wantXrays) {
                 jsval v = OBJECT_TO_JSVAL(proto);
                 if (!xpc::WrapperFactory::WaiveXrayAndWrap(cx, &v))
                     return NS_ERROR_FAILURE;
                 proto = JSVAL_TO_OBJECT(v);
             }
 
+            // Now check what sort of thing we've got in |proto|
+            JSObject *unwrappedProto = js::UnwrapObject(proto, false);
+            js::Class *unwrappedClass = js::GetObjectClass(unwrappedProto);
+            if (IS_WRAPPER_CLASS(unwrappedClass) ||
+                mozilla::dom::bindings::IsDOMClass(Jsvalify(unwrappedClass))) {
+                // Wrap it up in a proxy that will do the right thing in terms
+                // of this-binding for methods.
+                proto = js::NewProxyObject(cx, &sandboxProxyHandler,
+                                           ObjectValue(*proto), nsnull,
+                                           sandbox);
+                if (!proto)
+                    return NS_ERROR_OUT_OF_MEMORY;
+            }
+
             ok = JS_SetPrototype(cx, sandbox, proto);
             if (!ok)
                 return NS_ERROR_XPC_UNEXPECTED;
         }
 
         // Pass on ownership of sop to |sandbox|.
         JS_SetPrivate(sandbox, sop.forget().get());
 
--- a/js/xpconnect/src/XPCQuickStubs.cpp
+++ b/js/xpconnect/src/XPCQuickStubs.cpp
@@ -111,109 +111,30 @@ LookupInterfaceOrAncestor(PRUint32 table
             if (entry)
                 break;
             info.swap(parent);
         }
     }
     return entry;
 }
 
-// Apply |op| to |obj|, |id|, and |vp|. If |op| is a setter, treat the assignment as lenient.
-template<typename Op>
-static inline JSBool ApplyPropertyOp(JSContext *cx, Op op, JSObject *obj, jsid id, jsval *vp);
-
-template<>
-inline JSBool
-ApplyPropertyOp<JSPropertyOp>(JSContext *cx, JSPropertyOp op, JSObject *obj, jsid id, jsval *vp)
-{
-    return op(cx, obj, id, vp);
-}
-
-template<>
-inline JSBool
-ApplyPropertyOp<JSStrictPropertyOp>(JSContext *cx, JSStrictPropertyOp op, JSObject *obj,
-                                    jsid id, jsval *vp)
-{
-    return op(cx, obj, id, true, vp);
-}
-
-template<typename Op>
-static JSBool
-PropertyOpForwarder(JSContext *cx, unsigned argc, jsval *vp)
-{
-    // Layout:
-    //   this = our this
-    //   property op to call = callee reserved slot 0
-    //   name of the property = callee reserved slot 1
-
-    JSObject *callee = JSVAL_TO_OBJECT(JS_CALLEE(cx, vp));
-    JSObject *obj = JS_THIS_OBJECT(cx, vp);
-    if (!obj)
-        return false;
-
-    jsval v = js::GetFunctionNativeReserved(callee, 0);
-
-    JSObject *ptrobj = JSVAL_TO_OBJECT(v);
-    Op *popp = static_cast<Op *>(JS_GetPrivate(ptrobj));
-
-    v = js::GetFunctionNativeReserved(callee, 1);
-
-    jsval argval = (argc > 0) ? JS_ARGV(cx, vp)[0] : JSVAL_VOID;
-    jsid id;
-    if (!JS_ValueToId(cx, argval, &id))
-        return false;
-    JS_SET_RVAL(cx, vp, argval);
-    return ApplyPropertyOp<Op>(cx, *popp, obj, id, vp);
-}
-
 static void
 PointerFinalize(JSFreeOp *fop, JSObject *obj)
 {
     JSPropertyOp *popp = static_cast<JSPropertyOp *>(JS_GetPrivate(obj));
     delete popp;
 }
 
-static JSClass
+JSClass
 PointerHolderClass = {
     "Pointer", JSCLASS_HAS_PRIVATE,
     JS_PropertyStub, JS_PropertyStub, JS_PropertyStub, JS_StrictPropertyStub,
     JS_EnumerateStub, JS_ResolveStub, JS_ConvertStub, PointerFinalize
 };
 
-template<typename Op>
-static JSObject *
-GeneratePropertyOp(JSContext *cx, JSObject *obj, jsid id, unsigned argc, Op pop)
-{
-    // The JS engine provides two reserved slots on function objects for
-    // XPConnect to use. Use them to stick the necessary info here.
-    JSFunction *fun =
-        js::NewFunctionByIdWithReserved(cx, PropertyOpForwarder<Op>, argc, 0, obj, id);
-    if (!fun)
-        return nsnull;
-
-    JSObject *funobj = JS_GetFunctionObject(fun);
-
-    JS::AutoObjectRooter tvr(cx, funobj);
-
-    // Unfortunately, we cannot guarantee that Op is aligned. Use a
-    // second object to work around this.
-    JSObject *ptrobj = JS_NewObject(cx, &PointerHolderClass, nsnull, funobj);
-    if (!ptrobj)
-        return nsnull;
-    Op *popp = new Op;
-    if (!popp)
-        return nsnull;
-    *popp = pop;
-    JS_SetPrivate(ptrobj, popp);
-
-    js::SetFunctionNativeReserved(funobj, 0, OBJECT_TO_JSVAL(ptrobj));
-    js::SetFunctionNativeReserved(funobj, 1, js::IdToJsval(id));
-    return funobj;
-}
-
 static JSBool
 ReifyPropertyOps(JSContext *cx, JSObject *obj, jsid id, unsigned orig_attrs,
                  JSPropertyOp getter, JSStrictPropertyOp setter,
                  JSObject **getterobjp, JSObject **setterobjp)
 {
     // Generate both getter and setter and stash them in the prototype.
     jsval roots[2] = { JSVAL_NULL, JSVAL_NULL };
     JS::AutoArrayRooter tvr(cx, ArrayLength(roots), roots);
--- a/js/xpconnect/src/XPCQuickStubs.h
+++ b/js/xpconnect/src/XPCQuickStubs.h
@@ -40,16 +40,18 @@
 #ifndef xpcquickstubs_h___
 #define xpcquickstubs_h___
 
 #include "xpcpublic.h"
 #include "xpcprivate.h"
 
 #include "nsINode.h"
 
+#include "jsatom.h"
+
 /* XPCQuickStubs.h - Support functions used only by quick stubs. */
 
 class XPCCallContext;
 
 #define XPC_QS_NULL_INDEX  ((uint16_t) -1)
 
 struct xpc_qsPropertySpec {
     uint16_t name_index;
@@ -675,9 +677,90 @@ xpc_qsSameResult(PRInt32 result1, PRInt3
     return result1 == result2;
 }
 
 #define XPC_QS_ASSERT_CONTEXT_OK(cx) xpc_qsAssertContextOK(cx)
 #else
 #define XPC_QS_ASSERT_CONTEXT_OK(cx) ((void) 0)
 #endif
 
+// Apply |op| to |obj|, |id|, and |vp|. If |op| is a setter, treat the assignment as lenient.
+template<typename Op>
+static inline JSBool ApplyPropertyOp(JSContext *cx, Op op, JSObject *obj, jsid id, jsval *vp);
+
+template<>
+inline JSBool
+ApplyPropertyOp<JSPropertyOp>(JSContext *cx, JSPropertyOp op, JSObject *obj, jsid id, jsval *vp)
+{
+    return op(cx, obj, id, vp);
+}
+
+template<>
+inline JSBool
+ApplyPropertyOp<JSStrictPropertyOp>(JSContext *cx, JSStrictPropertyOp op, JSObject *obj,
+                                    jsid id, jsval *vp)
+{
+    return op(cx, obj, id, true, vp);
+}
+
+template<typename Op>
+JSBool
+PropertyOpForwarder(JSContext *cx, unsigned argc, jsval *vp)
+{
+    // Layout:
+    //   this = our this
+    //   property op to call = callee reserved slot 0
+    //   name of the property = callee reserved slot 1
+
+    JSObject *callee = JSVAL_TO_OBJECT(JS_CALLEE(cx, vp));
+    JSObject *obj = JS_THIS_OBJECT(cx, vp);
+    if (!obj)
+        return false;
+
+    jsval v = js::GetFunctionNativeReserved(callee, 0);
+
+    JSObject *ptrobj = JSVAL_TO_OBJECT(v);
+    Op *popp = static_cast<Op *>(JS_GetPrivate(ptrobj));
+
+    v = js::GetFunctionNativeReserved(callee, 1);
+
+    jsval argval = (argc > 0) ? JS_ARGV(cx, vp)[0] : JSVAL_VOID;
+    jsid id;
+    if (!JS_ValueToId(cx, argval, &id))
+        return false;
+    JS_SET_RVAL(cx, vp, argval);
+    return ApplyPropertyOp<Op>(cx, *popp, obj, id, vp);
+}
+
+extern JSClass PointerHolderClass;
+
+template<typename Op>
+JSObject *
+GeneratePropertyOp(JSContext *cx, JSObject *obj, jsid id, unsigned argc, Op pop)
+{
+    // The JS engine provides two reserved slots on function objects for
+    // XPConnect to use. Use them to stick the necessary info here.
+    JSFunction *fun =
+        js::NewFunctionByIdWithReserved(cx, PropertyOpForwarder<Op>, argc, 0, obj, id);
+    if (!fun)
+        return nsnull;
+
+    JSObject *funobj = JS_GetFunctionObject(fun);
+
+    JS::AutoObjectRooter tvr(cx, funobj);
+
+    // Unfortunately, we cannot guarantee that Op is aligned. Use a
+    // second object to work around this.
+    JSObject *ptrobj = JS_NewObject(cx, &PointerHolderClass, nsnull, funobj);
+    if (!ptrobj)
+        return nsnull;
+    Op *popp = new Op;
+    if (!popp)
+        return nsnull;
+    *popp = pop;
+    JS_SetPrivate(ptrobj, popp);
+
+    js::SetFunctionNativeReserved(funobj, 0, OBJECT_TO_JSVAL(ptrobj));
+    js::SetFunctionNativeReserved(funobj, 1, js::IdToJsval(id));
+    return funobj;
+}
+
 #endif /* xpcquickstubs_h___ */
--- a/js/xpconnect/tests/chrome/Makefile.in
+++ b/js/xpconnect/tests/chrome/Makefile.in
@@ -73,16 +73,17 @@ include $(topsrcdir)/config/rules.mk
 		test_bug743843.xul \
 		test_APIExposer.xul \
 		test_precisegc.xul \
 		test_nodelists.xul \
 		test_getweakmapkeys.xul \
 		test_weakmaps.xul \
 		test_exnstack.xul \
 		test_weakref.xul \
+		test_bug726949.xul \
 		$(NULL)
 
 # Disabled until this test gets updated to test the new proxy based
 # wrappers.
 #		test_wrappers-2.xul \
 
 # Disabled due to apparent conservative stack scanner false positives on Linux64 debug.
 #		test_watchpoints.xul \
new file mode 100644
--- /dev/null
+++ b/js/xpconnect/tests/chrome/test_bug726949.xul
@@ -0,0 +1,37 @@
+<?xml version="1.0"?>
+<?xml-stylesheet type="text/css" href="chrome://global/skin"?>
+<?xml-stylesheet type="text/css" href="chrome://mochikit/content/tests/SimpleTest/test.css"?>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=726949
+-->
+<window title="Mozilla Bug 726949"
+        xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
+  <script type="application/javascript" src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"/>
+
+  <!-- test results are displayed in the html:body -->
+  <body xmlns="http://www.w3.org/1999/xhtml">
+  <a href="https://bugzilla.mozilla.org/show_bug.cgi?id=726949"
+     target="_blank">Mozilla Bug 726949</a>
+  </body>
+
+  <!-- test code goes here -->
+  <script type="application/javascript">
+  <![CDATA[
+  /** Test for Bug 726949 **/
+      var Cu = Components.utils;
+      var s = new Cu.Sandbox(window, { sandboxPrototype: window } );
+      var t;
+      var desc;
+      try {
+          t = Cu.evalInSandbox('top', s);
+          is(t, window.top, "Should have gotten the right thing back");
+          desc = Cu.evalInSandbox('Object.getOwnPropertyDescriptor(Object.getPrototypeOf(this), "Cu")', s);
+          isnot(desc, undefined,
+		"Should have an own 'Cu' property");
+	  is(desc.value, Cu, "Should have the right value");
+      } catch (e) {
+          ok(false, "Should not get an exception: " + e);
+      }
+  ]]>
+  </script>
+</window>