From 7529e0b16b8916e710c1225f2d0fd6c6d3a711c4 Mon Sep 17 00:00:00 2001 From: Phil Ringnalda Date: Fri, 22 Feb 2013 08:41:37 -0800 Subject: [PATCH] Back out 4d301b2bcad0:e0632e639097 (bug 836301) for Windows build bustage CLOSED TREE --- js/src/js.msg | 4 +- js/src/jsapi.cpp | 3 - js/src/jscntxt.h | 6 +- js/src/jscompartment.cpp | 1 - js/src/jsfriendapi.h | 7 - js/src/jsproxy.cpp | 229 ++---------------- js/src/jsproxy.h | 89 ------- js/src/jswrapper.cpp | 195 ++++++++++++++- js/src/jswrapper.h | 49 +++- js/xpconnect/tests/chrome/test_bug760109.xul | 13 +- js/xpconnect/tests/unit/test_bug780370.js | 5 +- js/xpconnect/wrappers/AccessCheck.cpp | 18 ++ js/xpconnect/wrappers/AccessCheck.h | 24 +- js/xpconnect/wrappers/ChromeObjectWrapper.cpp | 90 ++----- js/xpconnect/wrappers/ChromeObjectWrapper.h | 3 - js/xpconnect/wrappers/FilteringWrapper.cpp | 29 +-- js/xpconnect/wrappers/XrayWrapper.cpp | 53 ++-- js/xpconnect/wrappers/XrayWrapper.h | 3 - 18 files changed, 356 insertions(+), 465 deletions(-) diff --git a/js/src/js.msg b/js/src/js.msg index c72d09521885..7b13a1cda096 100644 --- a/js/src/js.msg +++ b/js/src/js.msg @@ -79,8 +79,8 @@ MSG_DEF(JSMSG_TOO_DEEP, 25, 1, JSEXN_INTERNALERR, "{0} nested too MSG_DEF(JSMSG_OVER_RECURSED, 26, 0, JSEXN_INTERNALERR, "too much recursion") MSG_DEF(JSMSG_IN_NOT_OBJECT, 27, 1, JSEXN_TYPEERR, "invalid 'in' operand {0}") MSG_DEF(JSMSG_BAD_NEW_RESULT, 28, 1, JSEXN_TYPEERR, "invalid new expression result {0}") -MSG_DEF(JSMSG_OBJECT_ACCESS_DENIED, 29, 0, JSEXN_ERR, "Permission denied to access object") -MSG_DEF(JSMSG_PROPERTY_ACCESS_DENIED, 30, 1, JSEXN_ERR, "Permission denied to access property '{0}'") +MSG_DEF(JSMSG_UNUSED29, 29, 0, JSEXN_NONE, "") +MSG_DEF(JSMSG_UNUSED30, 30, 0, JSEXN_NONE, "") MSG_DEF(JSMSG_BAD_INSTANCEOF_RHS, 31, 1, JSEXN_TYPEERR, "invalid 'instanceof' operand {0}") MSG_DEF(JSMSG_BAD_BYTECODE, 32, 1, JSEXN_INTERNALERR, "unimplemented JavaScript bytecode {0}") MSG_DEF(JSMSG_BAD_RADIX, 33, 0, JSEXN_RANGEERR, "radix must be an integer at least 2 and no greater than 36") diff --git a/js/src/jsapi.cpp b/js/src/jsapi.cpp index 34b11a76dc42..e5ecfc9fd10d 100644 --- a/js/src/jsapi.cpp +++ b/js/src/jsapi.cpp @@ -881,9 +881,6 @@ JSRuntime::JSRuntime(JSUseHelperThreads useHelperThreads) ionReturnOverride_(MagicValue(JS_ARG_POISON)), useHelperThreads_(useHelperThreads), requestedHelperThreadCount(-1), -#ifdef DEBUG - enteredPolicy(NULL), -#endif rngNonce(0) { /* Initialize infallibly first, so we can goto bad and JS_DestroyRuntime. */ diff --git a/js/src/jscntxt.h b/js/src/jscntxt.h index 0172003b0081..368185490b6f 100644 --- a/js/src/jscntxt.h +++ b/js/src/jscntxt.h @@ -569,7 +569,7 @@ struct MallocProvider namespace gc { class MarkingValidator; } // namespace gc -class AutoEnterPolicy; + } // namespace js struct JSRuntime : js::RuntimeFriendFields, @@ -1301,11 +1301,7 @@ struct JSRuntime : js::RuntimeFriendFields, return 0; #endif } -#ifdef DEBUG - public: - js::AutoEnterPolicy *enteredPolicy; -#endif private: /* * Used to ensure that compartments created at the same time get different diff --git a/js/src/jscompartment.cpp b/js/src/jscompartment.cpp index a3c349c4d52c..680dd096ef04 100644 --- a/js/src/jscompartment.cpp +++ b/js/src/jscompartment.cpp @@ -432,7 +432,6 @@ JSCompartment::wrap(JSContext *cx, JSObject **objp, JSObject *existingArg) bool JSCompartment::wrapId(JSContext *cx, jsid *idp) { - MOZ_ASSERT(*idp != JSID_VOID, "JSID_VOID is an out-of-band sentinel value"); if (JSID_IS_INT(*idp)) return true; RootedValue value(cx, IdToValue(*idp)); diff --git a/js/src/jsfriendapi.h b/js/src/jsfriendapi.h index e3e0b4300302..0fb00e637e2c 100644 --- a/js/src/jsfriendapi.h +++ b/js/src/jsfriendapi.h @@ -1416,13 +1416,6 @@ class JS_FRIEND_API(AutoCTypesActivityCallback) { } }; -#ifdef DEBUG -extern JS_FRIEND_API(void) -assertEnteredPolicy(JSContext *cx, JSObject *obj, jsid id); -#else -inline void assertEnteredPolicy(JSContext *cx, JSObject *obj, jsid id) {}; -#endif - } /* namespace js */ #endif /* jsfriendapi_h___ */ diff --git a/js/src/jsproxy.cpp b/js/src/jsproxy.cpp index edea967b5c87..2cfb7cef5141 100644 --- a/js/src/jsproxy.cpp +++ b/js/src/jsproxy.cpp @@ -49,56 +49,9 @@ GetFunctionProxyConstruct(UnrootedObject proxy) return proxy->getSlotRef(JSSLOT_PROXY_CONSTRUCT); } -JS_FRIEND_API(void) -js::AutoEnterPolicy::reportError(JSContext *cx, jsid id) -{ - if (JSID_IS_VOID(id)) { - JS_ReportErrorNumber(cx, js_GetErrorMessage, NULL, - JSMSG_OBJECT_ACCESS_DENIED); - } else { - JSString *str = IdToString(cx, id); - const jschar *prop = str ? str->getCharsZ(cx) : NULL; - JS_ReportErrorNumberUC(cx, js_GetErrorMessage, NULL, - JSMSG_PROPERTY_ACCESS_DENIED, prop); - } -} - -#ifdef DEBUG -JS_FRIEND_API(void) -js::AutoEnterPolicy::recordEnter(JSContext *cx, JSObject *proxy, jsid id) -{ - if (allowed()) { - context = cx; - enteredProxy.construct(cx, proxy); - enteredId.construct(cx, id); - prev = cx->runtime->enteredPolicy; - cx->runtime->enteredPolicy = this; - } -} - -JS_FRIEND_API(void) -js::AutoEnterPolicy::recordLeave() -{ - if (!enteredProxy.empty()) { - JS_ASSERT(context->runtime->enteredPolicy == this); - context->runtime->enteredPolicy = prev; - } -} - -JS_FRIEND_API(void) -js::assertEnteredPolicy(JSContext *cx, JSObject *proxy, jsid id) -{ - MOZ_ASSERT(proxy->isProxy()); - MOZ_ASSERT(cx->runtime->enteredPolicy); - MOZ_ASSERT(cx->runtime->enteredPolicy->enteredProxy.ref().get() == proxy); - MOZ_ASSERT(cx->runtime->enteredPolicy->enteredId.ref().get() == id); -} -#endif - BaseProxyHandler::BaseProxyHandler(void *family) : mFamily(family), - mHasPrototype(false), - mHasPolicy(false) + mHasPrototype(false) { } @@ -106,18 +59,9 @@ BaseProxyHandler::~BaseProxyHandler() { } -bool -BaseProxyHandler::enter(JSContext *cx, JSObject *wrapper, jsid id, Action act, - bool *bp) -{ - *bp = true; - return true; -} - bool BaseProxyHandler::has(JSContext *cx, JSObject *proxy_, jsid id_, bool *bp) { - assertEnteredPolicy(cx, proxy_, id_); RootedObject proxy(cx, proxy_); RootedId id(cx, id_); AutoPropertyDescriptorRooter desc(cx); @@ -130,7 +74,6 @@ BaseProxyHandler::has(JSContext *cx, JSObject *proxy_, jsid id_, bool *bp) bool BaseProxyHandler::hasOwn(JSContext *cx, JSObject *proxy_, jsid id_, bool *bp) { - assertEnteredPolicy(cx, proxy_, id_); RootedObject proxy(cx, proxy_); RootedId id(cx, id_); AutoPropertyDescriptorRooter desc(cx); @@ -143,7 +86,6 @@ BaseProxyHandler::hasOwn(JSContext *cx, JSObject *proxy_, jsid id_, bool *bp) bool BaseProxyHandler::get(JSContext *cx, JSObject *proxy, JSObject *receiver_, jsid id_, Value *vp) { - assertEnteredPolicy(cx, proxy, id_); RootedObject receiver(cx, receiver_); RootedId id(cx, id_); @@ -185,7 +127,6 @@ BaseProxyHandler::getElementIfPresent(JSContext *cx, JSObject *proxy_, JSObject RootedId id(cx); if (!IndexToId(cx, index, &id)) return false; - assertEnteredPolicy(cx, proxy, id); if (!has(cx, proxy, id, present)) return false; @@ -202,7 +143,6 @@ bool BaseProxyHandler::set(JSContext *cx, JSObject *proxy_, JSObject *receiver_, jsid id_, bool strict, Value *vp) { - assertEnteredPolicy(cx, proxy_, id_); RootedObject proxy(cx, proxy_), receiver(cx, receiver_); RootedId id(cx, id_); @@ -279,7 +219,6 @@ BaseProxyHandler::set(JSContext *cx, JSObject *proxy_, JSObject *receiver_, jsid bool BaseProxyHandler::keys(JSContext *cx, JSObject *proxyArg, AutoIdVector &props) { - assertEnteredPolicy(cx, proxyArg, JSID_VOID); JS_ASSERT(props.length() == 0); RootedObject proxy(cx, proxyArg); @@ -293,7 +232,6 @@ BaseProxyHandler::keys(JSContext *cx, JSObject *proxyArg, AutoIdVector &props) for (size_t j = 0, len = props.length(); j < len; j++) { JS_ASSERT(i <= j); jsid id = props[j]; - AutoWaivePolicy policy(cx, proxy, id); if (!getOwnPropertyDescriptor(cx, proxy, id, &desc, 0)) return false; if (desc.obj && (desc.attrs & JSPROP_ENUMERATE)) @@ -309,7 +247,6 @@ BaseProxyHandler::keys(JSContext *cx, JSObject *proxyArg, AutoIdVector &props) bool BaseProxyHandler::iterate(JSContext *cx, JSObject *proxy_, unsigned flags, Value *vp) { - assertEnteredPolicy(cx, proxy_, JSID_VOID); RootedObject proxy(cx, proxy_); AutoIdVector props(cx); @@ -331,7 +268,6 @@ bool BaseProxyHandler::call(JSContext *cx, JSObject *proxy, unsigned argc, Value *vp) { - assertEnteredPolicy(cx, proxy, JSID_VOID); AutoValueRooter rval(cx); RootedValue call(cx, GetCall(proxy)); JSBool ok = Invoke(cx, vp[1], call, argc, JS_ARGV(cx, vp), rval.addr()); @@ -344,7 +280,6 @@ bool BaseProxyHandler::construct(JSContext *cx, JSObject *proxy_, unsigned argc, Value *argv, Value *rval) { - assertEnteredPolicy(cx, proxy_, JSID_VOID); RootedObject proxy(cx, proxy_); RootedValue fval(cx, GetConstruct(proxy_)); if (fval.isUndefined()) @@ -363,7 +298,6 @@ BaseProxyHandler::obj_toString(JSContext *cx, JSObject *proxy) JSString * BaseProxyHandler::fun_toString(JSContext *cx, JSObject *proxy, unsigned indent) { - assertEnteredPolicy(cx, proxy, JSID_VOID); Value fval = GetCall(proxy); if (IsFunctionProxy(proxy) && (fval.isPrimitive() || !fval.toObject().isFunction())) { @@ -409,7 +343,6 @@ BaseProxyHandler::nativeCall(JSContext *cx, IsAcceptableThis test, NativeImpl im bool BaseProxyHandler::hasInstance(JSContext *cx, HandleObject proxy, MutableHandleValue v, bool *bp) { - assertEnteredPolicy(cx, proxy, JSID_VOID); RootedValue val(cx, ObjectValue(*proxy.get())); js_ReportValueError(cx, JSMSG_BAD_INSTANCEOF_RHS, JSDVG_SEARCH_STACK, val, NullPtr()); @@ -446,8 +379,6 @@ bool DirectProxyHandler::getPropertyDescriptor(JSContext *cx, JSObject *proxy, jsid id, PropertyDescriptor *desc, unsigned flags) { - assertEnteredPolicy(cx, proxy, id); - JS_ASSERT(!hasPrototype()); // Should never be called if there's a prototype. RootedObject target(cx, GetProxyTargetObject(proxy)); return JS_GetPropertyDescriptorById(cx, target, id, 0, desc); } @@ -472,7 +403,6 @@ bool DirectProxyHandler::getOwnPropertyDescriptor(JSContext *cx, JSObject *proxy, jsid id, PropertyDescriptor *desc, unsigned flags) { - assertEnteredPolicy(cx, proxy, id); RootedObject target(cx, GetProxyTargetObject(proxy)); return GetOwnPropertyDescriptor(cx, target, id, 0, desc); } @@ -481,7 +411,6 @@ bool DirectProxyHandler::defineProperty(JSContext *cx, JSObject *proxy, jsid id_, PropertyDescriptor *desc) { - assertEnteredPolicy(cx, proxy, id_); RootedObject obj(cx, GetProxyTargetObject(proxy)); Rooted id(cx, id_); Rooted v(cx, desc->value); @@ -493,7 +422,6 @@ bool DirectProxyHandler::getOwnPropertyNames(JSContext *cx, JSObject *proxy, AutoIdVector &props) { - assertEnteredPolicy(cx, proxy, JSID_VOID); RootedObject target(cx, GetProxyTargetObject(proxy)); return GetPropertyNames(cx, target, JSITER_OWNONLY | JSITER_HIDDEN, &props); } @@ -502,7 +430,6 @@ bool DirectProxyHandler::delete_(JSContext *cx, JSObject *proxy, jsid id, bool *bp) { RootedValue v(cx); - assertEnteredPolicy(cx, proxy, id); RootedObject target(cx, GetProxyTargetObject(proxy)); if (!JS_DeletePropertyById2(cx, target, id, v.address())) return false; @@ -517,8 +444,6 @@ bool DirectProxyHandler::enumerate(JSContext *cx, JSObject *proxy, AutoIdVector &props) { - assertEnteredPolicy(cx, proxy, JSID_VOID); - JS_ASSERT(!hasPrototype()); // Should never be called if there's a prototype. RootedObject target(cx, GetProxyTargetObject(proxy)); return GetPropertyNames(cx, target, 0, &props); } @@ -540,7 +465,6 @@ bool DirectProxyHandler::hasInstance(JSContext *cx, HandleObject proxy, MutableHandleValue v, bool *bp) { - assertEnteredPolicy(cx, proxy, JSID_VOID); JSBool b; RootedObject target(cx, GetProxyTargetObject(proxy)); if (!JS_HasInstance(cx, target, v, &b)) @@ -561,7 +485,6 @@ DirectProxyHandler::objectClassIs(JSObject *proxy, ESClassValue classValue, JSString * DirectProxyHandler::obj_toString(JSContext *cx, JSObject *proxy) { - assertEnteredPolicy(cx, proxy, JSID_VOID); return obj_toStringHelper(cx, GetProxyTargetObject(proxy)); } @@ -569,7 +492,6 @@ JSString * DirectProxyHandler::fun_toString(JSContext *cx, JSObject *proxy, unsigned indent) { - assertEnteredPolicy(cx, proxy, JSID_VOID); RootedObject target(cx, GetProxyTargetObject(proxy)); return fun_toStringHelper(cx, target, indent); } @@ -605,8 +527,6 @@ DirectProxyHandler::DirectProxyHandler(void *family) bool DirectProxyHandler::has(JSContext *cx, JSObject *proxy, jsid id, bool *bp) { - assertEnteredPolicy(cx, proxy, id); - JS_ASSERT(!hasPrototype()); // Should never be called if there's a prototype. JSBool found; RootedObject target(cx, GetProxyTargetObject(proxy)); if (!JS_HasPropertyById(cx, target, id, &found)) @@ -618,7 +538,6 @@ DirectProxyHandler::has(JSContext *cx, JSObject *proxy, jsid id, bool *bp) bool DirectProxyHandler::hasOwn(JSContext *cx, JSObject *proxy, jsid id, bool *bp) { - assertEnteredPolicy(cx, proxy, id); RootedObject target(cx, GetProxyTargetObject(proxy)); AutoPropertyDescriptorRooter desc(cx); if (!JS_GetPropertyDescriptorById(cx, target, id, 0, &desc)) @@ -631,7 +550,6 @@ bool DirectProxyHandler::get(JSContext *cx, JSObject *proxy, JSObject *receiver_, jsid id_, Value *vp) { - assertEnteredPolicy(cx, proxy, id_); RootedObject receiver(cx, receiver_); RootedId id(cx, id_); RootedValue value(cx); @@ -647,7 +565,6 @@ bool DirectProxyHandler::set(JSContext *cx, JSObject *proxy, JSObject *receiverArg, jsid id_, bool strict, Value *vp) { - assertEnteredPolicy(cx, proxy, id_); RootedId id(cx, id_); Rooted receiver(cx, receiverArg); RootedValue value(cx, *vp); @@ -662,7 +579,6 @@ DirectProxyHandler::set(JSContext *cx, JSObject *proxy, JSObject *receiverArg, bool DirectProxyHandler::keys(JSContext *cx, JSObject *proxy, AutoIdVector &props) { - assertEnteredPolicy(cx, proxy, JSID_VOID); return GetPropertyNames(cx, GetProxyTargetObject(proxy), JSITER_OWNONLY, &props); } @@ -670,8 +586,6 @@ bool DirectProxyHandler::iterate(JSContext *cx, JSObject *proxy, unsigned flags, Value *vp) { - assertEnteredPolicy(cx, proxy, JSID_VOID); - JS_ASSERT(!hasPrototype()); // Should never be called if there's a prototype. Rooted target(cx, GetProxyTargetObject(proxy)); RootedValue value(cx); if (!GetIterator(cx, target, flags, &value)) @@ -2263,6 +2177,7 @@ ScriptedDirectProxyHandler ScriptedDirectProxyHandler::singleton; return protoCall; \ JS_END_MACRO \ + bool Proxy::getPropertyDescriptor(JSContext *cx, JSObject *proxy_, jsid id_, PropertyDescriptor *desc, unsigned flags) @@ -2271,10 +2186,6 @@ Proxy::getPropertyDescriptor(JSContext *cx, JSObject *proxy_, jsid id_, Property RootedObject proxy(cx, proxy_); RootedId id(cx, id_); BaseProxyHandler *handler = GetProxyHandler(proxy); - desc->obj = NULL; // default result if we refuse to perform this action - AutoEnterPolicy policy(cx, handler, proxy, id, BaseProxyHandler::GET, true); - if (!policy.allowed()) - return policy.returnValue(); if (!handler->hasPrototype()) return handler->getPropertyDescriptor(cx, proxy, id, desc, flags); if (!handler->getOwnPropertyDescriptor(cx, proxy, id, desc, flags)) @@ -2307,12 +2218,7 @@ Proxy::getOwnPropertyDescriptor(JSContext *cx, JSObject *proxy_, jsid id, Proper { JS_CHECK_RECURSION(cx, return false); RootedObject proxy(cx, proxy_); - BaseProxyHandler *handler = GetProxyHandler(proxy); - desc->obj = NULL; // default result if we refuse to perform this action - AutoEnterPolicy policy(cx, handler, proxy, id, BaseProxyHandler::GET, true); - if (!policy.allowed()) - return policy.returnValue(); - return handler->getOwnPropertyDescriptor(cx, proxy, id, desc, flags); + return GetProxyHandler(proxy)->getOwnPropertyDescriptor(cx, proxy, id, desc, flags); } bool @@ -2336,11 +2242,7 @@ bool Proxy::defineProperty(JSContext *cx, JSObject *proxy_, jsid id, PropertyDescriptor *desc) { JS_CHECK_RECURSION(cx, return false); - BaseProxyHandler *handler = GetProxyHandler(proxy_); RootedObject proxy(cx, proxy_); - AutoEnterPolicy policy(cx, handler, proxy, id, BaseProxyHandler::SET, true); - if (!policy.allowed()) - return policy.returnValue(); return GetProxyHandler(proxy)->defineProperty(cx, proxy, id, desc); } @@ -2359,11 +2261,7 @@ bool Proxy::getOwnPropertyNames(JSContext *cx, JSObject *proxy_, AutoIdVector &props) { JS_CHECK_RECURSION(cx, return false); - BaseProxyHandler *handler = GetProxyHandler(proxy_); RootedObject proxy(cx, proxy_); - AutoEnterPolicy policy(cx, handler, proxy, JSID_VOID, BaseProxyHandler::GET, true); - if (!policy.allowed()) - return policy.returnValue(); return GetProxyHandler(proxy)->getOwnPropertyNames(cx, proxy, props); } @@ -2371,12 +2269,7 @@ bool Proxy::delete_(JSContext *cx, JSObject *proxy_, jsid id, bool *bp) { JS_CHECK_RECURSION(cx, return false); - BaseProxyHandler *handler = GetProxyHandler(proxy_); RootedObject proxy(cx, proxy_); - *bp = true; // default result if we refuse to perform this action - AutoEnterPolicy policy(cx, handler, proxy, id, BaseProxyHandler::SET, true); - if (!policy.allowed()) - return policy.returnValue(); return GetProxyHandler(proxy)->delete_(cx, proxy, id, bp); } @@ -2406,9 +2299,6 @@ Proxy::enumerate(JSContext *cx, JSObject *proxy_, AutoIdVector &props) JS_CHECK_RECURSION(cx, return false); RootedObject proxy(cx, proxy_); BaseProxyHandler *handler = GetProxyHandler(proxy); - AutoEnterPolicy policy(cx, handler, proxy, JSID_VOID, BaseProxyHandler::GET, true); - if (!policy.allowed()) - return policy.returnValue(); if (!handler->hasPrototype()) return GetProxyHandler(proxy)->enumerate(cx, proxy, props); if (!handler->keys(cx, proxy, props)) @@ -2426,10 +2316,6 @@ Proxy::has(JSContext *cx, JSObject *proxy_, jsid id_, bool *bp) RootedObject proxy(cx, proxy_); RootedId id(cx, id_); BaseProxyHandler *handler = GetProxyHandler(proxy); - *bp = false; // default result if we refuse to perform this action - AutoEnterPolicy policy(cx, handler, proxy, id, BaseProxyHandler::GET, true); - if (!policy.allowed()) - return policy.returnValue(); if (!handler->hasPrototype()) return handler->has(cx, proxy, id, bp); if (!handler->hasOwn(cx, proxy, id, bp)) @@ -2447,12 +2333,7 @@ Proxy::hasOwn(JSContext *cx, JSObject *proxy_, jsid id, bool *bp) { JS_CHECK_RECURSION(cx, return false); RootedObject proxy(cx, proxy_); - BaseProxyHandler *handler = GetProxyHandler(proxy); - *bp = false; // default result if we refuse to perform this action - AutoEnterPolicy policy(cx, handler, proxy, id, BaseProxyHandler::GET, true); - if (!policy.allowed()) - return policy.returnValue(); - return handler->hasOwn(cx, proxy, id, bp); + return GetProxyHandler(proxy)->hasOwn(cx, proxy, id, bp); } bool @@ -2461,10 +2342,6 @@ Proxy::get(JSContext *cx, HandleObject proxy, HandleObject receiver, HandleId id { JS_CHECK_RECURSION(cx, return false); BaseProxyHandler *handler = GetProxyHandler(proxy); - vp.setUndefined(); // default result if we refuse to perform this action - AutoEnterPolicy policy(cx, handler, proxy, id, BaseProxyHandler::GET, true); - if (!policy.allowed()) - return policy.returnValue(); bool own; if (!handler->hasPrototype()) { own = true; @@ -2483,20 +2360,17 @@ Proxy::getElementIfPresent(JSContext *cx, HandleObject proxy, HandleObject recei { JS_CHECK_RECURSION(cx, return false); + BaseProxyHandler *handler = GetProxyHandler(proxy); + + if (!handler->hasPrototype()) { + return GetProxyHandler(proxy)->getElementIfPresent(cx, proxy, receiver, index, + vp.address(), present); + } + RootedId id(cx); if (!IndexToId(cx, index, &id)) return false; - BaseProxyHandler *handler = GetProxyHandler(proxy); - AutoEnterPolicy policy(cx, handler, proxy, id, BaseProxyHandler::GET, true); - if (!policy.allowed()) - return policy.returnValue(); - - if (!handler->hasPrototype()) { - return handler->getElementIfPresent(cx, proxy, receiver, index, - vp.address(), present); - } - bool hasOwn; if (!handler->hasOwn(cx, proxy, id, &hasOwn)) return false; @@ -2516,9 +2390,6 @@ Proxy::set(JSContext *cx, HandleObject proxy, HandleObject receiver, HandleId id { JS_CHECK_RECURSION(cx, return false); BaseProxyHandler *handler = GetProxyHandler(proxy); - AutoEnterPolicy policy(cx, handler, proxy, id, BaseProxyHandler::SET, true); - if (!policy.allowed()) - return policy.returnValue(); if (handler->hasPrototype()) { // If we're using a prototype, we still want to use the proxy trap unless // we have a non-own property with a setter. @@ -2546,11 +2417,7 @@ Proxy::keys(JSContext *cx, JSObject *proxy_, AutoIdVector &props) { JS_CHECK_RECURSION(cx, return false); RootedObject proxy(cx, proxy_); - BaseProxyHandler *handler = GetProxyHandler(proxy); - AutoEnterPolicy policy(cx, handler, proxy, JSID_VOID, BaseProxyHandler::GET, true); - if (!policy.allowed()) - return policy.returnValue(); - return handler->keys(cx, proxy, props); + return GetProxyHandler(proxy)->keys(cx, proxy, props); } bool @@ -2558,19 +2425,8 @@ Proxy::iterate(JSContext *cx, HandleObject proxy, unsigned flags, MutableHandleV { JS_CHECK_RECURSION(cx, return false); BaseProxyHandler *handler = GetProxyHandler(proxy); - vp.setUndefined(); // default result if we refuse to perform this action - if (!handler->hasPrototype()) { - AutoEnterPolicy policy(cx, handler, proxy, JSID_VOID, - BaseProxyHandler::GET, /* mayThrow = */ false); - // If the policy denies access but wants us to return true, we need - // to hand a valid (empty) iterator object to the caller. - if (!policy.allowed()) { - AutoIdVector props(cx); - return policy.returnValue() && - EnumeratedIdVectorToIterator(cx, proxy, flags, props, vp); - } - return handler->iterate(cx, proxy, flags, vp.address()); - } + if (!handler->hasPrototype()) + return GetProxyHandler(proxy)->iterate(cx, proxy, flags, vp.address()); AutoIdVector props(cx); // The other Proxy::foo methods do the prototype-aware work for us here. if ((flags & JSITER_OWNONLY) @@ -2586,19 +2442,7 @@ Proxy::call(JSContext *cx, JSObject *proxy_, unsigned argc, Value *vp) { JS_CHECK_RECURSION(cx, return false); RootedObject proxy(cx, proxy_); - BaseProxyHandler *handler = GetProxyHandler(proxy); - - // Because vp[0] is JS_CALLEE on the way in and JS_RVAL on the way out, we - // can only set our default value once we're sure that we're not calling the - // trap. - AutoEnterPolicy policy(cx, handler, proxy, JSID_VOID, - BaseProxyHandler::CALL, true); - if (!policy.allowed()) { - vp->setUndefined(); - return policy.returnValue(); - } - - return handler->call(cx, proxy, argc, vp); + return GetProxyHandler(proxy)->call(cx, proxy, argc, vp); } bool @@ -2606,19 +2450,7 @@ Proxy::construct(JSContext *cx, JSObject *proxy_, unsigned argc, Value *argv, Va { JS_CHECK_RECURSION(cx, return false); RootedObject proxy(cx, proxy_); - BaseProxyHandler *handler = GetProxyHandler(proxy); - - // Because vp[0] is JS_CALLEE on the way in and JS_RVAL on the way out, we - // can only set our default value once we're sure that we're not calling the - // trap. - AutoEnterPolicy policy(cx, handler, proxy, JSID_VOID, - BaseProxyHandler::CALL, true); - if (!policy.allowed()) { - rval->setUndefined(); - return policy.returnValue(); - } - - return handler->construct(cx, proxy, argc, argv, rval); + return GetProxyHandler(proxy)->construct(cx, proxy, argc, argv, rval); } bool @@ -2626,9 +2458,6 @@ Proxy::nativeCall(JSContext *cx, IsAcceptableThis test, NativeImpl impl, CallArg { JS_CHECK_RECURSION(cx, return false); Rooted proxy(cx, &args.thisv().toObject()); - // Note - we don't enter a policy here because our security architecture - // guards against nativeCall by overriding the trap itself in the right - // circumstances. return GetProxyHandler(proxy)->nativeCall(cx, test, impl, args); } @@ -2636,11 +2465,6 @@ bool Proxy::hasInstance(JSContext *cx, HandleObject proxy, MutableHandleValue v, bool *bp) { JS_CHECK_RECURSION(cx, return false); - BaseProxyHandler *handler = GetProxyHandler(proxy); - *bp = false; // default result if we refuse to perform this action - AutoEnterPolicy policy(cx, handler, proxy, JSID_VOID, BaseProxyHandler::GET, true); - if (!policy.allowed()) - return policy.returnValue(); return GetProxyHandler(proxy)->hasInstance(cx, proxy, v, bp); } @@ -2656,14 +2480,7 @@ Proxy::obj_toString(JSContext *cx, JSObject *proxy_) { JS_CHECK_RECURSION(cx, return NULL); RootedObject proxy(cx, proxy_); - BaseProxyHandler *handler = GetProxyHandler(proxy); - AutoEnterPolicy policy(cx, handler, proxy, JSID_VOID, - BaseProxyHandler::GET, /* mayThrow = */ false); - // Do the safe thing if the policy rejects. - if (!policy.allowed()) { - return handler->BaseProxyHandler::obj_toString(cx, proxy); - } - return handler->obj_toString(cx, proxy); + return GetProxyHandler(proxy)->obj_toString(cx, proxy); } JSString * @@ -2671,17 +2488,7 @@ Proxy::fun_toString(JSContext *cx, JSObject *proxy_, unsigned indent) { JS_CHECK_RECURSION(cx, return NULL); RootedObject proxy(cx, proxy_); - BaseProxyHandler *handler = GetProxyHandler(proxy); - AutoEnterPolicy policy(cx, handler, proxy, JSID_VOID, - BaseProxyHandler::GET, /* mayThrow = */ false); - // Do the safe thing if the policy rejects. - if (!policy.allowed()) { - if (proxy->isCallable()) - return JS_NewStringCopyZ(cx, "function () {\n [native code]\n}"); - ReportIsNotFunction(cx, ObjectValue(*proxy)); - return NULL; - } - return handler->fun_toString(cx, proxy, indent); + return GetProxyHandler(proxy)->fun_toString(cx, proxy, indent); } bool diff --git a/js/src/jsproxy.h b/js/src/jsproxy.h index c887d6593ac7..304f04a2a53b 100644 --- a/js/src/jsproxy.h +++ b/js/src/jsproxy.h @@ -51,11 +51,9 @@ class JS_FRIEND_API(Wrapper); class JS_FRIEND_API(BaseProxyHandler) { void *mFamily; bool mHasPrototype; - bool mHasPolicy; protected: // Subclasses may set this in their constructor. void setHasPrototype(bool aHasPrototype) { mHasPrototype = aHasPrototype; } - void setHasPolicy(bool aHasPolicy) { mHasPolicy = aHasPolicy; } public: explicit BaseProxyHandler(void *family); @@ -65,10 +63,6 @@ class JS_FRIEND_API(BaseProxyHandler) { return mHasPrototype; } - bool hasPolicy() { - return mHasPolicy; - } - inline void *family() { return mFamily; } @@ -77,24 +71,6 @@ class JS_FRIEND_API(BaseProxyHandler) { return false; } - /* Policy enforcement traps. - * - * enter() allows the policy to specify whether the caller may perform |act| - * on the proxy's |id| property. In the case when |act| is CALL, |id| is - * generally JSID_VOID. - * - * The |act| parameter to enter() specifies the action being performed. - * If |bp| is false, the trap suggests that the caller throw (though it - * may still decide to squelch the error). - */ - enum Action { - GET, - SET, - CALL - }; - virtual bool enter(JSContext *cx, JSObject *wrapper, jsid id, Action act, - bool *bp); - /* ES5 Harmony fundamental proxy traps. */ virtual bool getPropertyDescriptor(JSContext *cx, JSObject *proxy, jsid id, PropertyDescriptor *desc, unsigned flags) = 0; @@ -343,71 +319,6 @@ NewProxyObject(JSContext *cx, BaseProxyHandler *handler, const Value &priv, JSObject * RenewProxyObject(JSContext *cx, JSObject *obj, BaseProxyHandler *handler, Value priv); -class AutoEnterPolicy { - public: - typedef BaseProxyHandler::Action Action; - AutoEnterPolicy(JSContext *cx, BaseProxyHandler *handler, - JSObject *wrapper, jsid id, Action act, bool mayThrow) -#ifdef DEBUG - : context(NULL) -#endif - { - allow = handler->hasPolicy() ? handler->enter(cx, wrapper, id, act, &rv) - : true; - recordEnter(cx, wrapper, id); - if (!allow && !rv && mayThrow) - reportError(cx, id); - } - - virtual ~AutoEnterPolicy() { recordLeave(); } - inline bool allowed() { return allow; } - inline bool returnValue() { JS_ASSERT(!allowed()); return rv; } - - protected: - // no-op constructor for subclass - AutoEnterPolicy() -#ifdef DEBUG - : context(NULL) -#endif - {}; - void reportError(JSContext *cx, jsid id); - bool allow; - bool rv; - -#ifdef DEBUG - JSContext *context; - mozilla::Maybe enteredProxy; - mozilla::Maybe enteredId; - // NB: We explicitly don't track the entered action here, because sometimes - // SET traps do an implicit GET during their implementation, leading to - // spurious assertions. - AutoEnterPolicy *prev; - void recordEnter(JSContext *cx, JSObject *proxy, jsid id); - void recordLeave(); - - friend void assertEnteredPolicy(JSContext *cx, JSObject *proxy, jsid id); -#else - inline void recordEnter(JSContext *cx, JSObject *proxy, jsid id) {} - inline void recordLeave() {} -#endif - -}; - -#ifdef DEBUG -class AutoWaivePolicy : public AutoEnterPolicy { -public: - AutoWaivePolicy(JSContext *cx, JSObject *proxy, jsid id) - { - allow = true; - recordEnter(cx, proxy, id); - } -}; -#else -class AutoWaivePolicy { - public: AutoWaivePolicy(JSContext *cx, JSObject *proxy, jsid id) {}; -}; -#endif - } /* namespace js */ extern JS_FRIEND_API(JSObject *) diff --git a/js/src/jswrapper.cpp b/js/src/jswrapper.cpp index 332b4c3ae1ea..b05bbde51dfb 100644 --- a/js/src/jswrapper.cpp +++ b/js/src/jswrapper.cpp @@ -70,6 +70,13 @@ Wrapper::wrappedObject(RawObject wrapper) return GetProxyTargetObject(wrapper); } +bool +Wrapper::enter(JSContext *cx, JSObject *wrapper, jsid id, Action act, bool *bp) +{ + *bp = true; + return true; +} + JS_FRIEND_API(JSObject *) js::UnwrapObject(JSObject *wrapped, bool stopAtOuter, unsigned *flagsp) { @@ -115,6 +122,17 @@ js::IsCrossCompartmentWrapper(RawObject wrapper) !!(Wrapper::wrapperHandler(wrapper)->flags() & Wrapper::CROSS_COMPARTMENT); } +#define CHECKED(op, act) \ + JS_BEGIN_MACRO \ + bool status; \ + if (!enter(cx, wrapper, id, act, &status)) \ + return status; \ + return (op); \ + JS_END_MACRO + +#define SET(action) CHECKED(action, SET) +#define GET(action) CHECKED(action, GET) + Wrapper::Wrapper(unsigned flags, bool hasPrototype) : DirectProxyHandler(&sWrapperFamily) , mFlags(flags) , mSafeToUnwrap(true) @@ -126,6 +144,62 @@ Wrapper::~Wrapper() { } +bool +Wrapper::getPropertyDescriptor(JSContext *cx, JSObject *wrapperArg, + jsid id, PropertyDescriptor *desc, unsigned flags) +{ + RootedObject wrapper(cx, wrapperArg); + JS_ASSERT(!hasPrototype()); // Should never be called when there's a prototype. + desc->obj = NULL; // default result if we refuse to perform this action + CHECKED(DirectProxyHandler::getPropertyDescriptor(cx, wrapper, id, desc, flags), + (flags & JSRESOLVE_ASSIGNING) ? SET : GET); +} + +bool +Wrapper::getOwnPropertyDescriptor(JSContext *cx, JSObject *wrapperArg, + jsid id, PropertyDescriptor *desc, unsigned flags) +{ + RootedObject wrapper(cx, wrapperArg); + desc->obj = NULL; // default result if we refuse to perform this action + CHECKED(DirectProxyHandler::getOwnPropertyDescriptor(cx, wrapper, id, desc, flags), GET); +} + +bool +Wrapper::defineProperty(JSContext *cx, JSObject *wrapperArg, jsid id, + PropertyDescriptor *desc) +{ + RootedObject wrapper(cx, wrapperArg); + SET(DirectProxyHandler::defineProperty(cx, wrapper, id, desc)); +} + +bool +Wrapper::getOwnPropertyNames(JSContext *cx, JSObject *wrapperArg, + AutoIdVector &props) +{ + RootedObject wrapper(cx, wrapperArg); + // if we refuse to perform this action, props remains empty + jsid id = JSID_VOID; + GET(DirectProxyHandler::getOwnPropertyNames(cx, wrapper, props)); +} + +bool +Wrapper::delete_(JSContext *cx, JSObject *wrapperArg, jsid id, bool *bp) +{ + RootedObject wrapper(cx, wrapperArg); + *bp = true; // default result if we refuse to perform this action + SET(DirectProxyHandler::delete_(cx, wrapper, id, bp)); +} + +bool +Wrapper::enumerate(JSContext *cx, JSObject *wrapperArg, AutoIdVector &props) +{ + RootedObject wrapper(cx, wrapperArg); + JS_ASSERT(!hasPrototype()); // Should never be called when there's a prototype. + // if we refuse to perform this action, props remains empty + static jsid id = JSID_VOID; + GET(DirectProxyHandler::enumerate(cx, wrapper, props)); +} + /* * Ordinarily, the convert trap would require unwrapping. However, the default * implementation of convert, JS_ConvertStub, obtains a default value by calling @@ -160,6 +234,126 @@ Wrapper::defaultValue(JSContext *cx, JSObject *wrapperArg, JSType hint, Value *v return DirectProxyHandler::defaultValue(cx, wrapper, hint, vp); } +bool +Wrapper::has(JSContext *cx, JSObject *wrapperArg, jsid id, bool *bp) +{ + RootedObject wrapper(cx, wrapperArg); + JS_ASSERT(!hasPrototype()); // Should never be called when there's a prototype. + *bp = false; // default result if we refuse to perform this action + GET(DirectProxyHandler::has(cx, wrapper, id, bp)); +} + +bool +Wrapper::hasOwn(JSContext *cx, JSObject *wrapperArg, jsid id, bool *bp) +{ + RootedObject wrapper(cx, wrapperArg); + *bp = false; // default result if we refuse to perform this action + GET(DirectProxyHandler::hasOwn(cx, wrapper, id, bp)); +} + +bool +Wrapper::get(JSContext *cx, JSObject *wrapperArg, JSObject *receiver, jsid id, Value *vp) +{ + RootedObject wrapper(cx, wrapperArg); + vp->setUndefined(); // default result if we refuse to perform this action + GET(DirectProxyHandler::get(cx, wrapper, receiver, id, vp)); +} + +bool +Wrapper::set(JSContext *cx, JSObject *wrapperArg, JSObject *receiver, jsid id, bool strict, + Value *vp) +{ + RootedObject wrapper(cx, wrapperArg); + SET(DirectProxyHandler::set(cx, wrapper, receiver, id, strict, vp)); +} + +bool +Wrapper::keys(JSContext *cx, JSObject *wrapperArg, AutoIdVector &props) +{ + RootedObject wrapper(cx, wrapperArg); + // if we refuse to perform this action, props remains empty + const jsid id = JSID_VOID; + GET(DirectProxyHandler::keys(cx, wrapper, props)); +} + +bool +Wrapper::iterate(JSContext *cx, JSObject *wrapperArg, unsigned flags, Value *vp) +{ + RootedObject wrapper(cx, wrapperArg); + JS_ASSERT(!hasPrototype()); // Should never be called when there's a prototype. + vp->setUndefined(); // default result if we refuse to perform this action + const jsid id = JSID_VOID; + GET(DirectProxyHandler::iterate(cx, wrapper, flags, vp)); +} + +bool +Wrapper::call(JSContext *cx, JSObject *wrapperArg, unsigned argc, Value *vp) +{ + RootedObject wrapper(cx, wrapperArg); + vp->setUndefined(); // default result if we refuse to perform this action + const jsid id = JSID_VOID; + CHECKED(DirectProxyHandler::call(cx, wrapper, argc, vp), CALL); +} + +bool +Wrapper::construct(JSContext *cx, JSObject *wrapperArg, unsigned argc, Value *argv, Value *vp) +{ + RootedObject wrapper(cx, wrapperArg); + vp->setUndefined(); // default result if we refuse to perform this action + const jsid id = JSID_VOID; + CHECKED(DirectProxyHandler::construct(cx, wrapper, argc, argv, vp), CALL); +} + +bool +Wrapper::nativeCall(JSContext *cx, IsAcceptableThis test, NativeImpl impl, CallArgs args) +{ + const jsid id = JSID_VOID; + RootedObject wrapper(cx, &args.thisv().toObject()); + CHECKED(DirectProxyHandler::nativeCall(cx, test, impl, args), CALL); +} + +bool +Wrapper::hasInstance(JSContext *cx, HandleObject wrapper, MutableHandleValue v, bool *bp) +{ + *bp = false; // default result if we refuse to perform this action + const jsid id = JSID_VOID; + GET(DirectProxyHandler::hasInstance(cx, wrapper, v, bp)); +} + +JSString * +Wrapper::obj_toString(JSContext *cx, JSObject *wrapperArg) +{ + RootedObject wrapper(cx, wrapperArg); + bool status; + if (!enter(cx, wrapper, JSID_VOID, GET, &status)) { + if (status) { + // Perform some default behavior that doesn't leak any information. + return JS_NewStringCopyZ(cx, "[object Object]"); + } + return NULL; + } + JSString *str = DirectProxyHandler::obj_toString(cx, wrapper); + return str; +} + +JSString * +Wrapper::fun_toString(JSContext *cx, JSObject *wrapper, unsigned indent) +{ + bool status; + if (!enter(cx, wrapper, JSID_VOID, GET, &status)) { + if (status) { + // Perform some default behavior that doesn't leak any information. + if (wrapper->isCallable()) + return JS_NewStringCopyZ(cx, "function () {\n [native code]\n}"); + ReportIsNotFunction(cx, ObjectValue(*wrapper)); + return NULL; + } + return NULL; + } + JSString *str = DirectProxyHandler::fun_toString(cx, wrapper, indent); + return str; +} + Wrapper Wrapper::singleton((unsigned)0); Wrapper Wrapper::singletonWithPrototype((unsigned)0, true); @@ -644,7 +838,6 @@ SecurityWrapper::SecurityWrapper(unsigned flags) : Base(flags) { Base::setSafeToUnwrap(false); - BaseProxyHandler::setHasPolicy(true); } template diff --git a/js/src/jswrapper.h b/js/src/jswrapper.h index c3967026ef69..010b8e1414e6 100644 --- a/js/src/jswrapper.h +++ b/js/src/jswrapper.h @@ -32,7 +32,11 @@ class JS_FRIEND_API(Wrapper) : public DirectProxyHandler bool mSafeToUnwrap; public: - using BaseProxyHandler::Action; + enum Action { + GET, + SET, + CALL + }; enum Flags { CROSS_COMPARTMENT = 1 << 0, @@ -61,11 +65,54 @@ class JS_FRIEND_API(Wrapper) : public DirectProxyHandler return mFlags; } + /* Policy enforcement traps. + * + * enter() allows the policy to specify whether the caller may perform |act| + * on the underlying object's |id| property. In the case when |act| is CALL, + * |id| is generally JSID_VOID. + * + * The |act| parameter to enter() specifies the action being performed. + */ + virtual bool enter(JSContext *cx, JSObject *wrapper, jsid id, Action act, + bool *bp); + explicit Wrapper(unsigned flags, bool hasPrototype = false); virtual ~Wrapper(); /* ES5 Harmony fundamental wrapper traps. */ + virtual bool getPropertyDescriptor(JSContext *cx, JSObject *wrapper, + jsid id, PropertyDescriptor *desc, + unsigned flags) MOZ_OVERRIDE; + virtual bool getOwnPropertyDescriptor(JSContext *cx, JSObject *wrapper, + jsid id, PropertyDescriptor *desc, + unsigned flags) MOZ_OVERRIDE; + virtual bool defineProperty(JSContext *cx, JSObject *wrapper, jsid id, + PropertyDescriptor *desc) MOZ_OVERRIDE; + virtual bool getOwnPropertyNames(JSContext *cx, JSObject *wrapper, + AutoIdVector &props) MOZ_OVERRIDE; + virtual bool delete_(JSContext *cx, JSObject *wrapper, jsid id, + bool *bp) MOZ_OVERRIDE; + virtual bool enumerate(JSContext *cx, JSObject *wrapper, + AutoIdVector &props) MOZ_OVERRIDE; + + /* ES5 Harmony derived wrapper traps. */ + virtual bool has(JSContext *cx, JSObject *wrapper, jsid id, bool *bp) MOZ_OVERRIDE; + virtual bool hasOwn(JSContext *cx, JSObject *wrapper, jsid id, bool *bp) MOZ_OVERRIDE; + virtual bool get(JSContext *cx, JSObject *wrapper, JSObject *receiver, jsid id, Value *vp) MOZ_OVERRIDE; + virtual bool set(JSContext *cx, JSObject *wrapper, JSObject *receiver, jsid id, bool strict, + Value *vp) MOZ_OVERRIDE; + virtual bool keys(JSContext *cx, JSObject *wrapper, AutoIdVector &props) MOZ_OVERRIDE; + virtual bool iterate(JSContext *cx, JSObject *wrapper, unsigned flags, Value *vp) MOZ_OVERRIDE; + + /* Spidermonkey extensions. */ + virtual bool call(JSContext *cx, JSObject *wrapper, unsigned argc, Value *vp) MOZ_OVERRIDE; + virtual bool construct(JSContext *cx, JSObject *wrapper, unsigned argc, Value *argv, Value *rval) MOZ_OVERRIDE; + virtual bool nativeCall(JSContext *cx, IsAcceptableThis test, NativeImpl impl, + CallArgs args) MOZ_OVERRIDE; + virtual bool hasInstance(JSContext *cx, HandleObject wrapper, MutableHandleValue v, bool *bp) MOZ_OVERRIDE; + virtual JSString *obj_toString(JSContext *cx, JSObject *wrapper) MOZ_OVERRIDE; + virtual JSString *fun_toString(JSContext *cx, JSObject *wrapper, unsigned indent) MOZ_OVERRIDE; virtual bool defaultValue(JSContext *cx, JSObject *wrapper_, JSType hint, Value *vp) MOZ_OVERRIDE; diff --git a/js/xpconnect/tests/chrome/test_bug760109.xul b/js/xpconnect/tests/chrome/test_bug760109.xul index aa26158d5101..311ccb1e6ed0 100644 --- a/js/xpconnect/tests/chrome/test_bug760109.xul +++ b/js/xpconnect/tests/chrome/test_bug760109.xul @@ -31,13 +31,12 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=760109 ok(protoProto === Object.prototype, "Object prototype remapped properly"); - // Check |constructor|. - // Note that the 'constructor' property of the underlying chrome object - // will be resolved on SomeConstructor.prototype, which has an empty - // __exposedProps__. This means that we shouldn't remap the property, even - // though we'd also be able to find it on Object.prototype. Some recent - // refactoring has made it possible to do the right thing here. - is(typeof chromeObject.constructor, "undefined", "Object constructor does what we expect"); + // Check |constructor|. The semantics of this weird for the case of an + // object with a custom chrome-implemented prototype, because we'll end up + // bouncing up the prototype chain to Object, even though that's not fully + // accurate. It's unlikely to be a problem though, so we just verify that + // it does what we expect. + ok(chromeObject.constructor === Object, "Object constructor does what we expect"); ok(chromeArray.constructor === Array, "Array constructor remapped properly"); // We should be able to .forEach on the Array. diff --git a/js/xpconnect/tests/unit/test_bug780370.js b/js/xpconnect/tests/unit/test_bug780370.js index 027f13418898..1e335d51a452 100644 --- a/js/xpconnect/tests/unit/test_bug780370.js +++ b/js/xpconnect/tests/unit/test_bug780370.js @@ -16,8 +16,5 @@ function run_test() 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"); - // NB: We used to test for the following, but such behavior became very - // difficult to implement in a recent refactor. We're moving away from this - // API anyway, so we decided to explicitly drop support for this. - // do_check_eq(Cu.evalInSandbox('Object.prototype.bar = 10; obj.bar', sb), 10); + do_check_eq(Cu.evalInSandbox('Object.prototype.bar = 10; obj.bar', sb), 10); } diff --git a/js/xpconnect/wrappers/AccessCheck.cpp b/js/xpconnect/wrappers/AccessCheck.cpp index 3865c4aeb0f3..d5072d874990 100644 --- a/js/xpconnect/wrappers/AccessCheck.cpp +++ b/js/xpconnect/wrappers/AccessCheck.cpp @@ -267,6 +267,24 @@ AccessCheck::isScriptAccessOnly(JSContext *cx, JSObject *wrapper) return false; } +void +AccessCheck::deny(JSContext *cx, jsid id) +{ + if (id == JSID_VOID) { + JS_ReportError(cx, "Permission denied to access object"); + } else { + jsval idval; + if (!JS_IdToValue(cx, id, &idval)) + return; + JSString *str = JS_ValueToString(cx, idval); + if (!str) + return; + const jschar *chars = JS_GetStringCharsZ(cx, str); + if (chars) + JS_ReportError(cx, "Permission denied to access property '%hs'", chars); + } +} + enum Access { READ = (1<<0), WRITE = (1<<1), NO_ACCESS = 0 }; static bool diff --git a/js/xpconnect/wrappers/AccessCheck.h b/js/xpconnect/wrappers/AccessCheck.h index dd95e0407316..8a7dcc5a5b45 100644 --- a/js/xpconnect/wrappers/AccessCheck.h +++ b/js/xpconnect/wrappers/AccessCheck.h @@ -33,6 +33,8 @@ class AccessCheck { static bool needsSystemOnlyWrapper(JSObject *obj); static bool isScriptAccessOnly(JSContext *cx, JSObject *wrapper); + + static void deny(JSContext *cx, jsid id); }; struct Policy { @@ -43,7 +45,8 @@ struct Opaque : public Policy { static bool check(JSContext *cx, JSObject *wrapper, jsid id, js::Wrapper::Action act) { return act == js::Wrapper::CALL; } - static bool deny(js::Wrapper::Action act) { + static bool deny(JSContext *cx, jsid id, js::Wrapper::Action act) { + AccessCheck::deny(cx, id); return false; } static bool allowNativeCall(JSContext *cx, JS::IsAcceptableThis test, JS::NativeImpl impl) @@ -59,7 +62,8 @@ struct OnlyIfSubjectIsSystem : public Policy { return AccessCheck::isSystemOnlyAccessPermitted(cx); } - static bool deny(js::Wrapper::Action act) { + static bool deny(JSContext *cx, jsid id, js::Wrapper::Action act) { + AccessCheck::deny(cx, id); return false; } @@ -75,7 +79,8 @@ struct CrossOriginAccessiblePropertiesOnly : public Policy { static bool check(JSContext *cx, JSObject *wrapper, jsid id, js::Wrapper::Action act) { return AccessCheck::isCrossOriginAccessPermitted(cx, wrapper, id, act); } - static bool deny(js::Wrapper::Action act) { + static bool deny(JSContext *cx, jsid id, js::Wrapper::Action act) { + AccessCheck::deny(cx, id); return false; } static bool allowNativeCall(JSContext *cx, JS::IsAcceptableThis test, JS::NativeImpl impl) @@ -89,9 +94,13 @@ struct CrossOriginAccessiblePropertiesOnly : public Policy { struct ExposedPropertiesOnly : public Policy { static bool check(JSContext *cx, JSObject *wrapper, jsid id, js::Wrapper::Action act); - static bool deny(js::Wrapper::Action act) { - // Fail silently for GETs. - return act == js::Wrapper::GET; + static bool deny(JSContext *cx, jsid id, js::Wrapper::Action act) { + // For gets, silently fail. + if (act == js::Wrapper::GET) + return true; + // For sets,throw an exception. + AccessCheck::deny(cx, id); + return false; } static bool allowNativeCall(JSContext *cx, JS::IsAcceptableThis test, JS::NativeImpl impl); }; @@ -100,7 +109,8 @@ struct ExposedPropertiesOnly : public Policy { struct ComponentsObjectPolicy : public Policy { static bool check(JSContext *cx, JSObject *wrapper, jsid id, js::Wrapper::Action act); - static bool deny(js::Wrapper::Action act) { + static bool deny(JSContext *cx, jsid id, js::Wrapper::Action act) { + AccessCheck::deny(cx, id); return false; } static bool allowNativeCall(JSContext *cx, JS::IsAcceptableThis test, JS::NativeImpl impl) { diff --git a/js/xpconnect/wrappers/ChromeObjectWrapper.cpp b/js/xpconnect/wrappers/ChromeObjectWrapper.cpp index 68a84d0dc703..f2f6c79e6aff 100644 --- a/js/xpconnect/wrappers/ChromeObjectWrapper.cpp +++ b/js/xpconnect/wrappers/ChromeObjectWrapper.cpp @@ -14,18 +14,6 @@ namespace xpc { // their prototype, we have to instrument the traps to do this manually. ChromeObjectWrapper ChromeObjectWrapper::singleton; -using js::assertEnteredPolicy; - -static bool -AllowedByBase(JSContext *cx, JSObject *wrapper, jsid id, js::Wrapper::Action act) -{ - MOZ_ASSERT(js::Wrapper::wrapperHandler(wrapper) == - &ChromeObjectWrapper::singleton); - bool bp; - ChromeObjectWrapper *handler = &ChromeObjectWrapper::singleton; - return handler->ChromeObjectWrapperBase::enter(cx, wrapper, id, act, &bp); -} - static bool PropIsFromStandardPrototype(JSContext *cx, JSPropertyDescriptor *desc) { @@ -35,36 +23,16 @@ PropIsFromStandardPrototype(JSContext *cx, JSPropertyDescriptor *desc) return JS_IdentifyClassPrototype(cx, unwrapped) != JSProto_Null; } -// Note that we're past the policy enforcement stage, here, so we can query -// ChromeObjectWrapperBase and get an unfiltered view of the underlying object. -// This lets us determine whether the property we would have found (given a -// transparent wrapper) would have come off a standard prototype. -static bool -PropIsFromStandardPrototype(JSContext *cx, JSObject *wrapper, jsid id) -{ - MOZ_ASSERT(js::Wrapper::wrapperHandler(wrapper) == - &ChromeObjectWrapper::singleton); - JSPropertyDescriptor desc; - ChromeObjectWrapper *handler = &ChromeObjectWrapper::singleton; - if (!handler->ChromeObjectWrapperBase::getPropertyDescriptor(cx, wrapper, id, - &desc, 0) || - !desc.obj) - { - return false; - } - return PropIsFromStandardPrototype(cx, &desc); -} - bool ChromeObjectWrapper::getPropertyDescriptor(JSContext *cx, JSObject *wrapper, jsid id, js::PropertyDescriptor *desc, unsigned flags) { - assertEnteredPolicy(cx, wrapper, id); - // First, try a lookup on the base wrapper if permitted. + // 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 (AllowedByBase(cx, wrapper, id, Wrapper::GET) && - !ChromeObjectWrapperBase::getPropertyDescriptor(cx, wrapper, id, + if (!ChromeObjectWrapperBase::getPropertyDescriptor(cx, wrapper, id, desc, flags)) { return false; } @@ -90,13 +58,9 @@ ChromeObjectWrapper::getPropertyDescriptor(JSContext *cx, JSObject *wrapper, bool ChromeObjectWrapper::has(JSContext *cx, JSObject *wrapper, jsid id, bool *bp) { - assertEnteredPolicy(cx, wrapper, id); - // Try the lookup on the base wrapper if permitted. - if (AllowedByBase(cx, wrapper, id, js::Wrapper::GET) && - !ChromeObjectWrapperBase::has(cx, wrapper, id, bp)) - { + // Try the lookup on the base wrapper. + if (!ChromeObjectWrapperBase::has(cx, wrapper, id, bp)) return false; - } // If we found something or have no prototype, we're done. JSObject *wrapperProto; @@ -118,15 +82,19 @@ bool ChromeObjectWrapper::get(JSContext *cx, JSObject *wrapper, JSObject *receiver, jsid id, js::Value *vp) { - assertEnteredPolicy(cx, wrapper, id); - vp->setUndefined(); + // 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; - // Only call through to the get trap on the underlying object if we're - // allowed to see the property, and if what we'll find is not on a standard - // prototype. - if (AllowedByBase(cx, wrapper, id, js::Wrapper::GET) && - !PropIsFromStandardPrototype(cx, wrapper, id)) - { + if (!ChromeObjectWrapperBase::getPropertyDescriptor(cx, wrapper, id, &desc, + 0)) { + return false; + } + + // 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; @@ -147,26 +115,4 @@ ChromeObjectWrapper::get(JSContext *cx, JSObject *wrapper, JSObject *receiver, return js::GetGeneric(cx, wrapperProto, receiver, id, vp); } -// This mechanism isn't ideal because we end up calling enter() on the base class -// twice (once during enter() here and once during the trap itself), and policy -// enforcement or COWs isn't cheap. But it results in the cleanest code, and this -// whole proto remapping thing for COWs is going to be phased out anyway. -bool -ChromeObjectWrapper::enter(JSContext *cx, JSObject *wrapper, jsid id, - js::Wrapper::Action act, bool *bp) -{ - if (AllowedByBase(cx, wrapper, id, act)) - return true; - // COWs fail silently for GETs, and that also happens to be the only case - // where we might want to redirect the lookup to the home prototype chain. - *bp = (act == Wrapper::GET); - if (!*bp || id == JSID_VOID) - return false; - - // Note that PropIsFromStandardPrototype needs to invoke getPropertyDescriptor - // before we've fully entered the policy. Waive our policy. - js::AutoWaivePolicy policy(cx, wrapper, id); - return PropIsFromStandardPrototype(cx, wrapper, id); -} - } diff --git a/js/xpconnect/wrappers/ChromeObjectWrapper.h b/js/xpconnect/wrappers/ChromeObjectWrapper.h index 1a98f3a9026f..b5e0b7e9708e 100644 --- a/js/xpconnect/wrappers/ChromeObjectWrapper.h +++ b/js/xpconnect/wrappers/ChromeObjectWrapper.h @@ -37,9 +37,6 @@ class ChromeObjectWrapper : public ChromeObjectWrapperBase virtual bool get(JSContext *cx, JSObject *wrapper, JSObject *receiver, jsid id, js::Value *vp) MOZ_OVERRIDE; - virtual bool enter(JSContext *cx, JSObject *wrapper, jsid id, - js::Wrapper::Action act, bool *bp) MOZ_OVERRIDE; - // NB: One might think we'd need to implement enumerate(), keys(), iterate(), // and getPropertyNames() here. However, ES5 built-in properties aren't // enumerable (and SpiderMonkey's implementation seems to match the spec diff --git a/js/xpconnect/wrappers/FilteringWrapper.cpp b/js/xpconnect/wrappers/FilteringWrapper.cpp index d8d06841f927..da60024ffa2b 100644 --- a/js/xpconnect/wrappers/FilteringWrapper.cpp +++ b/js/xpconnect/wrappers/FilteringWrapper.cpp @@ -63,7 +63,6 @@ bool FilteringWrapper::getPropertyDescriptor(JSContext *cx, JSObject *wrapper, jsid id, js::PropertyDescriptor *desc, unsigned flags) { - assertEnteredPolicy(cx, wrapper, id); if (!Base::getPropertyDescriptor(cx, wrapper, id, desc, flags)) return false; return FilterSetter(cx, wrapper, id, desc); @@ -75,7 +74,6 @@ FilteringWrapper::getOwnPropertyDescriptor(JSContext *cx, JSObject js::PropertyDescriptor *desc, unsigned flags) { - assertEnteredPolicy(cx, wrapper, id); if (!Base::getOwnPropertyDescriptor(cx, wrapper, id, desc, flags)) return false; return FilterSetter(cx, wrapper, id, desc); @@ -85,7 +83,6 @@ template bool FilteringWrapper::getOwnPropertyNames(JSContext *cx, JSObject *wrapper, AutoIdVector &props) { - assertEnteredPolicy(cx, wrapper, JSID_VOID); return Base::getOwnPropertyNames(cx, wrapper, props) && Filter(cx, wrapper, props); } @@ -94,7 +91,6 @@ template bool FilteringWrapper::enumerate(JSContext *cx, JSObject *wrapper, AutoIdVector &props) { - assertEnteredPolicy(cx, wrapper, JSID_VOID); return Base::enumerate(cx, wrapper, props) && Filter(cx, wrapper, props); } @@ -103,7 +99,6 @@ template bool FilteringWrapper::keys(JSContext *cx, JSObject *wrapper, AutoIdVector &props) { - assertEnteredPolicy(cx, wrapper, JSID_VOID); return Base::keys(cx, wrapper, props) && Filter(cx, wrapper, props); } @@ -112,7 +107,6 @@ template bool FilteringWrapper::iterate(JSContext *cx, JSObject *wrapper, unsigned flags, Value *vp) { - assertEnteredPolicy(cx, wrapper, JSID_VOID); // We refuse to trigger the iterator hook across chrome wrappers because // we don't know how to censor custom iterator objects. Instead we trigger // the default proxy iterate trap, which will ask enumerate() for the list @@ -135,24 +129,13 @@ bool FilteringWrapper::enter(JSContext *cx, JSObject *wrapper, jsid id, Wrapper::Action act, bool *bp) { - // This is a super ugly hacky to get around Xray Resolve wonkiness. - // - // Basically, XPCWN Xrays sometimes call into the Resolve hook of the - // scriptable helper, and pass the wrapper itself as the object upon which - // the resolve is happening. Then, special handling happens in - // XrayWrapper::defineProperty to detect the resolve and redefine the - // property on the holder. Really, we should just pass the holder itself to - // NewResolve, but there's too much code in nsDOMClassInfo that assumes this - // isn't the case (in particular, code expects to be able to look up - // properties on the object, which doesn't work for the holder). Given that - // these hooks are going away eventually with the new DOM bindings, let's - // just hack around this for now. - if (XrayUtils::IsXrayResolving(cx, wrapper, id)) { - *bp = true; - return true; - } if (!Policy::check(cx, wrapper, id, act)) { - *bp = JS_IsExceptionPending(cx) ? false : Policy::deny(act); + if (JS_IsExceptionPending(cx)) { + *bp = false; + return false; + } + JSAutoCompartment ac(cx, wrapper); + *bp = Policy::deny(cx, id, act); return false; } *bp = true; diff --git a/js/xpconnect/wrappers/XrayWrapper.cpp b/js/xpconnect/wrappers/XrayWrapper.cpp index f20e04b1b6fa..a3d0610c5c5a 100644 --- a/js/xpconnect/wrappers/XrayWrapper.cpp +++ b/js/xpconnect/wrappers/XrayWrapper.cpp @@ -726,7 +726,8 @@ XPCWrappedNativeXrayTraits::resolveNativeProperty(JSContext *cx, JSObject *wrapp { MOZ_ASSERT(js::GetObjectJSClass(holder) == &HolderClass); XPCJSRuntime* rt = nsXPConnect::GetRuntimeInstance(); - if (id == rt->GetStringID(XPCJSRuntime::IDX_MOZMATCHESSELECTOR) && + if (!(flags & JSRESOLVE_ASSIGNING) && + id == rt->GetStringID(XPCJSRuntime::IDX_MOZMATCHESSELECTOR) && Is(wrapper)) { // XPC calling mechanism cannot handle call/bind properly in some cases @@ -983,8 +984,13 @@ XPCWrappedNativeXrayTraits::resolveOwnProperty(JSContext *cx, js::Wrapper &jsWra id == rt->GetStringID(XPCJSRuntime::IDX_NODEPRINCIPAL)) && Is(wrapper)) || (id == rt->GetStringID(XPCJSRuntime::IDX_DOCUMENTURIOBJECT) && - Is(wrapper)))) - { + Is(wrapper)))) { + bool status; + Wrapper::Action action = (flags & JSRESOLVE_ASSIGNING) ? Wrapper::SET : Wrapper::GET; + desc->obj = NULL; // default value + if (!jsWrapper.enter(cx, wrapper, id, action, &status)) + return status; + desc->obj = wrapper; desc->attrs = JSPROP_ENUMERATE|JSPROP_SHARED; if (id == rt->GetStringID(XPCJSRuntime::IDX_BASEURIOBJECT)) @@ -1313,19 +1319,6 @@ GetNativePropertiesObject(JSContext *cx, JSObject *wrapper) return holder; } -bool -IsXrayResolving(JSContext *cx, JSObject *wrapper, jsid id) -{ - if (!WrapperFactory::IsXrayWrapper(wrapper) || - GetXrayType(wrapper) != XrayForWrappedNative) - { - return false; - } - JSObject *holder = - XPCWrappedNativeXrayTraits::singleton.ensureHolder(cx, wrapper); - return XPCWrappedNativeXrayTraits::isResolving(cx, holder, id); -} - } static JSBool @@ -1410,13 +1403,18 @@ bool XrayWrapper::getPropertyDescriptor(JSContext *cx, JSObject *wrapper, jsid id, js::PropertyDescriptor *desc, unsigned flags) { - assertEnteredPolicy(cx, wrapper, id); JSObject *holder = Traits::singleton.ensureHolder(cx, wrapper); if (Traits::isResolving(cx, holder, id)) { desc->obj = NULL; return true; } + bool status; + Wrapper::Action action = (flags & JSRESOLVE_ASSIGNING) ? Wrapper::SET : Wrapper::GET; + desc->obj = NULL; // default value + if (!this->enter(cx, wrapper, id, action, &status)) + return status; + typename Traits::ResolvingIdImpl resolving(wrapper, id); // Redirect access straight to the wrapper if we should be transparent. @@ -1442,6 +1440,12 @@ XrayWrapper::getPropertyDescriptor(JSContext *cx, JSObject *wrappe XPCJSRuntime* rt = nsXPConnect::GetRuntimeInstance(); if (AccessCheck::wrapperSubsumes(wrapper) && id == rt->GetStringID(XPCJSRuntime::IDX_WRAPPED_JSOBJECT)) { + bool status; + Wrapper::Action action = (flags & JSRESOLVE_ASSIGNING) ? Wrapper::SET : Wrapper::GET; + desc->obj = NULL; // default value + if (!this->enter(cx, wrapper, id, action, &status)) + return status; + desc->obj = wrapper; desc->attrs = JSPROP_ENUMERATE|JSPROP_SHARED; desc->getter = wrappedJSObject_getter; @@ -1529,13 +1533,18 @@ bool XrayWrapper::getOwnPropertyDescriptor(JSContext *cx, JSObject *wrapper, jsid id, PropertyDescriptor *desc, unsigned flags) { - assertEnteredPolicy(cx, wrapper, id); JSObject *holder = Traits::singleton.ensureHolder(cx, wrapper); if (Traits::isResolving(cx, holder, id)) { desc->obj = NULL; return true; } + bool status; + Wrapper::Action action = (flags & JSRESOLVE_ASSIGNING) ? Wrapper::SET : Wrapper::GET; + desc->obj = NULL; // default value + if (!this->enter(cx, wrapper, id, action, &status)) + return status; + typename Traits::ResolvingIdImpl resolving(wrapper, id); // NB: Nothing we do here acts on the wrapped native itself, so we don't @@ -1576,7 +1585,6 @@ bool XrayWrapper::defineProperty(JSContext *cx, JSObject *wrapper, jsid id, js::PropertyDescriptor *desc) { - assertEnteredPolicy(cx, wrapper, id); // Redirect access straight to the wrapper if we should be transparent. if (XrayUtils::IsTransparent(cx, wrapper, id)) { JSObject *obj = Traits::getTargetObject(wrapper); @@ -1588,8 +1596,6 @@ XrayWrapper::defineProperty(JSContext *cx, JSObject *wrapper, jsid desc->attrs); } - // NB: We still need JSRESOLVE_ASSIGNING here for the time being, because it - // tells things like nodelists whether they should create the property or not. PropertyDescriptor existing_desc; if (!getOwnPropertyDescriptor(cx, wrapper, id, &existing_desc, JSRESOLVE_ASSIGNING)) return false; @@ -1629,7 +1635,6 @@ bool XrayWrapper::getOwnPropertyNames(JSContext *cx, JSObject *wrapper, JS::AutoIdVector &props) { - assertEnteredPolicy(cx, wrapper, JSID_VOID); return enumerate(cx, wrapper, JSITER_OWNONLY | JSITER_HIDDEN, props); } @@ -1637,7 +1642,6 @@ template bool XrayWrapper::delete_(JSContext *cx, JSObject *wrapper, jsid id, bool *bp) { - assertEnteredPolicy(cx, wrapper, id); // Redirect access straight to the wrapper if we should be transparent. if (XrayUtils::IsTransparent(cx, wrapper, id)) { JSObject *obj = Traits::getTargetObject(wrapper); @@ -1674,7 +1678,6 @@ bool XrayWrapper::enumerate(JSContext *cx, JSObject *wrapper, unsigned flags, JS::AutoIdVector &props) { - assertEnteredPolicy(cx, wrapper, JSID_VOID); // Redirect access straight to the wrapper if we should be transparent. if (XrayUtils::IsTransparent(cx, wrapper, JSID_VOID)) { JSObject *obj = Traits::getTargetObject(wrapper); @@ -1768,7 +1771,6 @@ template bool XrayWrapper::call(JSContext *cx, JSObject *wrapper, unsigned argc, js::Value *vp) { - assertEnteredPolicy(cx, wrapper, JSID_VOID); return Traits::call(cx, wrapper, argc, vp); } @@ -1777,7 +1779,6 @@ bool XrayWrapper::construct(JSContext *cx, JSObject *wrapper, unsigned argc, js::Value *argv, js::Value *rval) { - assertEnteredPolicy(cx, wrapper, JSID_VOID); return Traits::construct(cx, wrapper, argc, argv, rval); } diff --git a/js/xpconnect/wrappers/XrayWrapper.h b/js/xpconnect/wrappers/XrayWrapper.h index fb76135fa9a5..b1f3f9ddd823 100644 --- a/js/xpconnect/wrappers/XrayWrapper.h +++ b/js/xpconnect/wrappers/XrayWrapper.h @@ -38,9 +38,6 @@ IsTransparent(JSContext *cx, JSObject *wrapper, jsid id); JSObject * GetNativePropertiesObject(JSContext *cx, JSObject *wrapper); -bool -IsXrayResolving(JSContext *cx, JSObject *wrapper, jsid id); - } class XrayTraits;