From 72acf485fbda95cbc7750599da44fdb904d0fa6e Mon Sep 17 00:00:00 2001 From: Tom Schuster Date: Fri, 23 Mar 2018 13:09:04 +0100 Subject: [PATCH] Bug 1255800 - Make computeThis return a boolean for easier error handling. r=jorendorff --HG-- extra : rebase_source : 1b4d227aba2fd0e1a458849addb634df9d72111c extra : histedit_source : 29e8c520ae4d3644131b05794a447a0e7a32796f --- ipc/testshell/XPCShellEnvironment.cpp | 6 ++++-- js/public/CallArgs.h | 14 ++++++++------ js/src/jsapi.cpp | 9 +++++---- js/xpconnect/src/ExportHelpers.cpp | 5 ++++- js/xpconnect/src/Sandbox.cpp | 18 ++++++++++++------ js/xpconnect/src/XPCShellImpl.cpp | 6 ++++-- js/xpconnect/src/XPCWrappedNativeJSOps.cpp | 14 ++++---------- 7 files changed, 41 insertions(+), 31 deletions(-) diff --git a/ipc/testshell/XPCShellEnvironment.cpp b/ipc/testshell/XPCShellEnvironment.cpp index e31d3e77044f..70bedf67feaf 100644 --- a/ipc/testshell/XPCShellEnvironment.cpp +++ b/ipc/testshell/XPCShellEnvironment.cpp @@ -132,8 +132,10 @@ Load(JSContext *cx, { JS::CallArgs args = JS::CallArgsFromVp(argc, vp); - JS::RootedValue thisv(cx, args.computeThis(cx)); - if (!thisv.isObject() || !JS_IsGlobalObject(&thisv.toObject())) { + JS::RootedObject thisObject(cx); + if (!args.computeThis(cx, &thisObject)) + return false; + if (!JS_IsGlobalObject(thisObject)) { JS_ReportErrorASCII(cx, "Trying to load() into a non-global object"); return false; } diff --git a/js/public/CallArgs.h b/js/public/CallArgs.h index 1452bd6940f6..e32d5c8fc7f3 100644 --- a/js/public/CallArgs.h +++ b/js/public/CallArgs.h @@ -87,8 +87,8 @@ namespace detail { * Compute |this| for the |vp| inside a JSNative, either boxing primitives or * replacing with the global object as necessary. */ -extern JS_PUBLIC_API(Value) -ComputeThis(JSContext* cx, JS::Value* vp); +extern JS_PUBLIC_API(bool) +ComputeThis(JSContext* cx, JS::Value* vp, MutableHandleObject thisObject); #ifdef JS_DEBUG extern JS_PUBLIC_API(void) @@ -199,11 +199,13 @@ class MOZ_STACK_CLASS CallArgsBase return HandleValue::fromMarkedLocation(&argv_[-1]); } - Value computeThis(JSContext* cx) const { - if (thisv().isObject()) - return thisv(); + bool computeThis(JSContext* cx, MutableHandleObject thisObject) const { + if (thisv().isObject()) { + thisObject.set(&thisv().toObject()); + return true; + } - return ComputeThis(cx, base()); + return ComputeThis(cx, base(), thisObject); } // ARGUMENTS diff --git a/js/src/jsapi.cpp b/js/src/jsapi.cpp index 7465dfeb9a0a..b545ea1033e3 100644 --- a/js/src/jsapi.cpp +++ b/js/src/jsapi.cpp @@ -1339,17 +1339,18 @@ JS::CurrentGlobalOrNull(JSContext* cx) return cx->global(); } -JS_PUBLIC_API(Value) -JS::detail::ComputeThis(JSContext* cx, Value* vp) +JS_PUBLIC_API(bool) +JS::detail::ComputeThis(JSContext* cx, Value* vp, MutableHandleObject thisObject) { AssertHeapIsIdle(); assertSameCompartment(cx, JSValueArray(vp, 2)); MutableHandleValue thisv = MutableHandleValue::fromMarkedLocation(&vp[1]); if (!BoxNonStrictThis(cx, thisv, thisv)) - return NullValue(); + return false; - return thisv; + thisObject.set(&thisv.toObject()); + return true; } JS_PUBLIC_API(void*) diff --git a/js/xpconnect/src/ExportHelpers.cpp b/js/xpconnect/src/ExportHelpers.cpp index 9d3e60b537c3..3acad3bd8ffc 100644 --- a/js/xpconnect/src/ExportHelpers.cpp +++ b/js/xpconnect/src/ExportHelpers.cpp @@ -290,7 +290,10 @@ FunctionForwarder(JSContext* cx, unsigned argc, Value* vp) RootedValue thisVal(cx, NullValue()); if (!args.isConstructing()) { - thisVal.set(args.computeThis(cx)); + RootedObject thisObject(cx); + if (!args.computeThis(cx, &thisObject)) + return false; + thisVal.setObject(*thisObject); } { diff --git a/js/xpconnect/src/Sandbox.cpp b/js/xpconnect/src/Sandbox.cpp index 4c9014d0b38a..e434f82559e9 100644 --- a/js/xpconnect/src/Sandbox.cpp +++ b/js/xpconnect/src/Sandbox.cpp @@ -216,12 +216,10 @@ SandboxImport(JSContext* cx, unsigned argc, Value* vp) // We need to resolve the this object, because this function is used // unbound and should still work and act on the original sandbox. - RootedValue thisv(cx, args.computeThis(cx)); - if (!thisv.isObject()) { - XPCThrower::Throw(NS_ERROR_UNEXPECTED, cx); + RootedObject thisObject(cx); + if (!args.computeThis(cx, &thisObject)) return false; - } - RootedObject thisObject(cx, &thisv.toObject()); + if (!JS_SetPropertyById(cx, thisObject, id, args[0])) return false; @@ -562,7 +560,15 @@ xpc::SandboxCallableProxyHandler::call(JSContext* cx, JS::Handle prox // if the sandboxPrototype is an Xray Wrapper, which lets us appropriately // remap |this|. bool isXray = WrapperFactory::IsXrayWrapper(sandboxProxy); - RootedValue thisVal(cx, isXray ? args.computeThis(cx) : args.thisv()); + RootedValue thisVal(cx, args.thisv()); + if (isXray) { + RootedObject thisObject(cx); + if (!args.computeThis(cx, &thisObject)) { + return false; + } + thisVal.setObject(*thisObject); + } + if (thisVal == ObjectValue(*sandboxGlobal)) { thisVal = ObjectValue(*js::GetProxyTargetObject(sandboxProxy)); } diff --git a/js/xpconnect/src/XPCShellImpl.cpp b/js/xpconnect/src/XPCShellImpl.cpp index f77792df7356..9a2953cf7aa8 100644 --- a/js/xpconnect/src/XPCShellImpl.cpp +++ b/js/xpconnect/src/XPCShellImpl.cpp @@ -337,8 +337,10 @@ Load(JSContext* cx, unsigned argc, Value* vp) { CallArgs args = CallArgsFromVp(argc, vp); - RootedValue thisv(cx, args.computeThis(cx)); - if (!thisv.isObject() || !JS_IsGlobalObject(&thisv.toObject())) { + JS::RootedObject thisObject(cx); + if (!args.computeThis(cx, &thisObject)) + return false; + if (!JS_IsGlobalObject(thisObject)) { JS_ReportErrorASCII(cx, "Trying to load() into a non-global object"); return false; } diff --git a/js/xpconnect/src/XPCWrappedNativeJSOps.cpp b/js/xpconnect/src/XPCWrappedNativeJSOps.cpp index 67e1d536b210..a747779475b9 100644 --- a/js/xpconnect/src/XPCWrappedNativeJSOps.cpp +++ b/js/xpconnect/src/XPCWrappedNativeJSOps.cpp @@ -70,12 +70,9 @@ XPC_WN_Shared_ToString(JSContext* cx, unsigned argc, Value* vp) { CallArgs args = CallArgsFromVp(argc, vp); - RootedValue thisv(cx, args.computeThis(cx)); - if (!thisv.isObject()) { - JS_ReportErrorASCII(cx, "Called on incompatible |this|"); + RootedObject obj(cx); + if (!args.computeThis(cx, &obj)) return false; - } - RootedObject obj(cx, &thisv.toObject()); XPCCallContext ccx(cx, obj); if (!ccx.IsValid()) @@ -895,12 +892,9 @@ XPC_WN_CallMethod(JSContext* cx, unsigned argc, Value* vp) MOZ_ASSERT(JS_TypeOfValue(cx, args.calleev()) == JSTYPE_FUNCTION, "bad function"); RootedObject funobj(cx, &args.callee()); - RootedValue thisv(cx, args.computeThis(cx)); - if (!thisv.isObject()) { - JS_ReportErrorASCII(cx, "Called on incompatible |this|"); + RootedObject obj(cx); + if (!args.computeThis(cx, &obj)) return false; - } - RootedObject obj(cx, &thisv.toObject()); obj = FixUpThisIfBroken(obj, funobj); XPCCallContext ccx(cx, obj, funobj, JSID_VOIDHANDLE, args.length(),