Bug 1085071. Stop handling JSPropertyOp getters/setters in sandbox code, since we no longer have those around for any of the objects we care about. r=bholley
authorBoris Zbarsky <bzbarsky@mit.edu>
Mon, 20 Oct 2014 13:04:45 -0400
changeset 211345 ad5d5e03408fa62bf8f003efdd10e8cd45c1389c
parent 211344 e9c8e96a3b97a10e2cf24e3c5c0d2321e9562fa2
child 211346 f1ecd7a2f170296d06afdfcb0436245573c0d0ba
push id27673
push userkwierso@gmail.com
push dateTue, 21 Oct 2014 01:57:45 +0000
treeherdermozilla-central@29fbfc1b31aa [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbholley
bugs1085071
milestone36.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 1085071. Stop handling JSPropertyOp getters/setters in sandbox code, since we no longer have those around for any of the objects we care about. r=bholley
js/xpconnect/src/Sandbox.cpp
js/xpconnect/src/XPCQuickStubs.cpp
js/xpconnect/src/XPCQuickStubs.h
--- a/js/xpconnect/src/Sandbox.cpp
+++ b/js/xpconnect/src/Sandbox.cpp
@@ -614,73 +614,57 @@ WrapCallable(JSContext *cx, HandleObject
 
     RootedValue priv(cx, ObjectValue(*callable));
     return js::NewProxyObject(cx, &xpc::sandboxCallableProxyHandler,
                               priv, nullptr,
                               sandboxProtoProxy);
 }
 
 template<typename Op>
-bool BindPropertyOp(JSContext *cx, Op &op, JSPropertyDescriptor *desc, HandleId id,
-                    unsigned attrFlag, HandleObject sandboxProtoProxy)
+bool WrapAccessorFunction(JSContext *cx, Op &op, JSPropertyDescriptor *desc,
+                          unsigned attrFlag, HandleObject sandboxProtoProxy)
 {
     if (!op) {
         return true;
     }
 
-    RootedObject func(cx);
-    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;
-        RootedObject obj(cx, desc->obj);
-        func = GeneratePropertyOp(cx, obj, id, args, op);
-        if (!func)
-            return false;
+    if (!(desc->attrs & attrFlag)) {
+        XPCThrower::Throw(NS_ERROR_UNEXPECTED, cx);
+        return false;
     }
+
+    RootedObject func(cx, JS_FUNC_TO_DATA_PTR(JSObject *, op));
     func = WrapCallable(cx, func, sandboxProtoProxy);
     if (!func)
         return false;
     op = JS_DATA_TO_FUNC_PTR(Op, func.get());
-    desc->attrs |= attrFlag;
     return true;
 }
 
-extern bool
-XPC_WN_Helper_GetProperty(JSContext *cx, HandleObject obj, HandleId id, MutableHandleValue vp);
-extern bool
-XPC_WN_Helper_SetProperty(JSContext *cx, HandleObject obj, HandleId id, bool strict, MutableHandleValue vp);
-
 bool
 xpc::SandboxProxyHandler::getPropertyDescriptor(JSContext *cx,
                                                 JS::Handle<JSObject*> proxy,
                                                 JS::Handle<jsid> id,
                                                 JS::MutableHandle<JSPropertyDescriptor> desc) const
 {
     JS::RootedObject obj(cx, wrappedObject(proxy));
 
     MOZ_ASSERT(js::GetObjectCompartment(obj) == js::GetObjectCompartment(proxy));
     if (!JS_GetPropertyDescriptorById(cx, obj, id, desc))
         return false;
 
     if (!desc.object())
         return true; // No property, nothing to do
 
     // Now fix up the getter/setter/value as needed to be bound to desc->obj.
-    //
-    // Don't mess with XPC_WN_Helper_GetProperty and XPC_WN_Helper_SetProperty,
-    // because that could confuse our access to expandos.
-    if (desc.getter() != XPC_WN_Helper_GetProperty &&
-        !BindPropertyOp(cx, desc.getter(), desc.address(), id, JSPROP_GETTER, proxy))
+    if (!WrapAccessorFunction(cx, desc.getter(), desc.address(),
+                              JSPROP_GETTER, proxy))
         return false;
-    if (desc.setter() != XPC_WN_Helper_SetProperty &&
-        !BindPropertyOp(cx, desc.setter(), desc.address(), id, JSPROP_SETTER, proxy))
+    if (!WrapAccessorFunction(cx, desc.setter(), desc.address(),
+                              JSPROP_SETTER, proxy))
         return false;
     if (desc.value().isObject()) {
         RootedObject val (cx, &desc.value().toObject());
         if (JS::IsCallable(val)) {
             val = WrapCallable(cx, val, proxy);
             if (!val)
                 return false;
             desc.value().setObject(*val);
--- a/js/xpconnect/src/XPCQuickStubs.cpp
+++ b/js/xpconnect/src/XPCQuickStubs.cpp
@@ -74,30 +74,16 @@ HasBitInInterfacesBitmap(JSObject *obj, 
 {
     MOZ_ASSERT(IS_WN_REFLECTOR(obj), "Not a wrapper?");
 
     const XPCWrappedNativeJSClass *clasp =
       (const XPCWrappedNativeJSClass*)js::GetObjectClass(obj);
     return (clasp->interfacesBitmap & (1 << interfaceBit)) != 0;
 }
 
-static void
-PointerFinalize(JSFreeOp *fop, JSObject *obj)
-{
-    JSPropertyOp *popp = static_cast<JSPropertyOp *>(JS_GetPrivate(obj));
-    delete popp;
-}
-
-const JSClass
-PointerHolderClass = {
-    "Pointer", JSCLASS_HAS_PRIVATE,
-    JS_PropertyStub, JS_DeletePropertyStub, JS_PropertyStub, JS_StrictPropertyStub,
-    JS_EnumerateStub, JS_ResolveStub, JS_ConvertStub, PointerFinalize
-};
-
 bool
 xpc_qsDefineQuickStubs(JSContext *cx, JSObject *protoArg, unsigned flags,
                        uint32_t ifacec, const nsIID **interfaces,
                        uint32_t tableSize, const xpc_qsHashEntry *table,
                        const xpc_qsPropertySpec *propspecs,
                        const xpc_qsFunctionSpec *funcspecs,
                        const char *stringTable)
 {
--- a/js/xpconnect/src/XPCQuickStubs.h
+++ b/js/xpconnect/src/XPCQuickStubs.h
@@ -524,92 +524,9 @@ xpc_qsSameResult(int32_t result1, int32_
     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>
-inline bool ApplyPropertyOp(JSContext *cx, Op op, JS::HandleObject obj, JS::HandleId id,
-                              JS::MutableHandleValue vp);
-
-template<>
-inline bool
-ApplyPropertyOp<JSPropertyOp>(JSContext *cx, JSPropertyOp op, JS::HandleObject obj, JS::HandleId id,
-                              JS::MutableHandleValue vp)
-{
-    return op(cx, obj, id, vp);
-}
-
-template<>
-inline bool
-ApplyPropertyOp<JSStrictPropertyOp>(JSContext *cx, JSStrictPropertyOp op, JS::HandleObject obj,
-                                    JS::HandleId id, JS::MutableHandleValue vp)
-{
-    return op(cx, obj, id, true, vp);
-}
-
-template<typename Op>
-bool
-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
-
-    JS::CallArgs args = CallArgsFromVp(argc, vp);
-
-    JS::RootedObject callee(cx, &args.callee());
-    JS::RootedObject obj(cx, JS_THIS_OBJECT(cx, vp));
-    if (!obj)
-        return false;
-
-    JS::RootedValue v(cx, js::GetFunctionNativeReserved(callee, 0));
-
-    JSObject *ptrobj = v.toObjectOrNull();
-    Op *popp = static_cast<Op *>(JS_GetPrivate(ptrobj));
-
-    v = js::GetFunctionNativeReserved(callee, 1);
-
-    JS::RootedValue argval(cx, args.get(0));
-    JS::RootedId id(cx);
-    if (!JS_ValueToId(cx, v, &id))
-        return false;
-    args.rval().set(argval);
-    return ApplyPropertyOp<Op>(cx, *popp, obj, id, args.rval());
-}
-
-extern const JSClass PointerHolderClass;
-
-template<typename Op>
-JSObject *
-GeneratePropertyOp(JSContext *cx, JS::HandleObject obj, JS::HandleId 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 nullptr;
-
-    JS::RootedObject funobj(cx, JS_GetFunctionObject(fun));
-
-    // Unfortunately, we cannot guarantee that Op is aligned. Use a
-    // second object to work around this.
-    JSObject *ptrobj = JS_NewObject(cx, &PointerHolderClass, JS::NullPtr(), funobj);
-    if (!ptrobj)
-        return nullptr;
-    Op *popp = new Op;
-    if (!popp)
-        return nullptr;
-    *popp = pop;
-    JS_SetPrivate(ptrobj, popp);
-
-    js::SetFunctionNativeReserved(funobj, 0, OBJECT_TO_JSVAL(ptrobj));
-    js::SetFunctionNativeReserved(funobj, 1, js::IdToValue(id));
-    return funobj;
-}
-
 #endif /* xpcquickstubs_h___ */