From 4fceffbb3c40a792198fa46fc440be6c6f4d1c98 Mon Sep 17 00:00:00 2001 From: Jeff Walden Date: Fri, 18 Mar 2016 16:43:43 -0700 Subject: [PATCH] Bug 1257102 - Invoking a trap on a proxy, where the handler has null as the value of that trap, should fall back to operating on the target just like undefined would. r=evilpie --HG-- extra : rebase_source : b77e0d98e710345192e95d4128fc82963dba403b --- js/src/js.msg | 1 + js/src/proxy/ScriptedDirectProxyHandler.cpp | 54 +++++++--- js/src/tests/ecma_6/Proxy/trap-null.js | 103 ++++++++++++++++++++ 3 files changed, 147 insertions(+), 11 deletions(-) create mode 100644 js/src/tests/ecma_6/Proxy/trap-null.js diff --git a/js/src/js.msg b/js/src/js.msg index e67b3e3b206f..b8407aad32d0 100644 --- a/js/src/js.msg +++ b/js/src/js.msg @@ -381,6 +381,7 @@ MSG_DEF(JSMSG_PROXY_EXTENSIBILITY, 0, JSEXN_TYPEERR, "proxy must report same MSG_DEF(JSMSG_PROXY_GETOWN_OBJORUNDEF, 0, JSEXN_TYPEERR, "proxy [[GetOwnProperty]] must return an object or undefined") MSG_DEF(JSMSG_PROXY_REVOKED, 0, JSEXN_TYPEERR, "illegal operation attempted on a revoked proxy") MSG_DEF(JSMSG_PROXY_ARG_REVOKED, 1, JSEXN_TYPEERR, "argument {0} cannot be a revoked proxy") +MSG_DEF(JSMSG_BAD_TRAP, 1, JSEXN_TYPEERR, "proxy handler's {0} trap wasn't undefined, null, or callable") // Structured cloning MSG_DEF(JSMSG_SC_BAD_CLONE_VERSION, 0, JSEXN_ERR, "unsupported structured clone version") diff --git a/js/src/proxy/ScriptedDirectProxyHandler.cpp b/js/src/proxy/ScriptedDirectProxyHandler.cpp index 25a68438eabb..0ece17e63096 100644 --- a/js/src/proxy/ScriptedDirectProxyHandler.cpp +++ b/js/src/proxy/ScriptedDirectProxyHandler.cpp @@ -128,6 +128,38 @@ GetDirectProxyHandlerObject(JSObject* proxy) return proxy->as().extra(ScriptedDirectProxyHandler::HANDLER_EXTRA).toObjectOrNull(); } +// ES7 rev 0c1bd3004329336774cbc90de727cd0cf5f11e93 7.3.9 GetMethod, +// reimplemented for proxy handler trap-getting to produce better error +// messages. +static bool +GetProxyTrap(JSContext* cx, HandleObject handler, HandlePropertyName name, MutableHandleValue func) +{ + // Steps 2, 5. + if (!GetProperty(cx, handler, handler, name, func)) + return false; + + // Step 3. + if (func.isUndefined()) + return true; + + if (func.isNull()) { + func.setUndefined(); + return true; + } + + // Step 4. + if (!IsCallable(func)) { + JSAutoByteString bytes(cx, name); + if (!bytes) + return false; + + JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_BAD_TRAP, bytes.ptr()); + return false; + } + + return true; +} + // ES6 implements both getPrototype and setPrototype traps. We don't have them yet (see bug // 888969). For now, use these, to account for proxy revocation. bool @@ -189,7 +221,7 @@ ScriptedDirectProxyHandler::preventExtensions(JSContext* cx, HandleObject proxy, // Steps 5-6. RootedValue trap(cx); - if (!GetProperty(cx, handler, handler, cx->names().preventExtensions, &trap)) + if (!GetProxyTrap(cx, handler, cx->names().preventExtensions, &trap)) return false; // Step 7. @@ -237,7 +269,7 @@ ScriptedDirectProxyHandler::isExtensible(JSContext* cx, HandleObject proxy, bool // step 4-5 RootedValue trap(cx); - if (!GetProperty(cx, handler, handler, cx->names().isExtensible, &trap)) + if (!GetProxyTrap(cx, handler, cx->names().isExtensible, &trap)) return false; // step 6 @@ -290,7 +322,7 @@ ScriptedDirectProxyHandler::getOwnPropertyDescriptor(JSContext* cx, HandleObject // step 5-6 RootedValue trap(cx); - if (!GetProperty(cx, handler, handler, cx->names().getOwnPropertyDescriptor, &trap)) + if (!GetProxyTrap(cx, handler, cx->names().getOwnPropertyDescriptor, &trap)) return false; // step 7 @@ -410,7 +442,7 @@ ScriptedDirectProxyHandler::defineProperty(JSContext* cx, HandleObject proxy, Ha // steps 6-7 RootedValue trap(cx); - if (!GetProperty(cx, handler, handler, cx->names().defineProperty, &trap)) + if (!GetProxyTrap(cx, handler, cx->names().defineProperty, &trap)) return false; // step 8 @@ -537,7 +569,7 @@ ScriptedDirectProxyHandler::ownPropertyKeys(JSContext* cx, HandleObject proxy, // Steps 5-6. RootedValue trap(cx); - if (!GetProperty(cx, handler, handler, cx->names().ownKeys, &trap)) + if (!GetProxyTrap(cx, handler, cx->names().ownKeys, &trap)) return false; // Step 7. @@ -674,7 +706,7 @@ ScriptedDirectProxyHandler::delete_(JSContext* cx, HandleObject proxy, HandleId // steps 6-7 RootedValue trap(cx); - if (!GetProperty(cx, handler, handler, cx->names().deleteProperty, &trap)) + if (!GetProxyTrap(cx, handler, cx->names().deleteProperty, &trap)) return false; // step 8 @@ -731,7 +763,7 @@ ScriptedDirectProxyHandler::has(JSContext* cx, HandleObject proxy, HandleId id, // step 5-6 RootedValue trap(cx); - if (!GetProperty(cx, handler, handler, cx->names().has, &trap)) + if (!GetProxyTrap(cx, handler, cx->names().has, &trap)) return false; // step 7 @@ -799,7 +831,7 @@ ScriptedDirectProxyHandler::get(JSContext* cx, HandleObject proxy, HandleValue r // step 5-6 RootedValue trap(cx); - if (!GetProperty(cx, handler, handler, cx->names().get, &trap)) + if (!GetProxyTrap(cx, handler, cx->names().get, &trap)) return false; // step 7 @@ -864,7 +896,7 @@ ScriptedDirectProxyHandler::set(JSContext* cx, HandleObject proxy, HandleId id, // step 5-7 RootedObject target(cx, proxy->as().target()); RootedValue trap(cx); - if (!GetProperty(cx, handler, handler, cx->names().set, &trap)) + if (!GetProxyTrap(cx, handler, cx->names().set, &trap)) return false; // step 8 @@ -940,7 +972,7 @@ ScriptedDirectProxyHandler::call(JSContext* cx, HandleObject proxy, const CallAr // step 4-5 RootedValue trap(cx); - if (!GetProperty(cx, handler, handler, cx->names().apply, &trap)) + if (!GetProxyTrap(cx, handler, cx->names().apply, &trap)) return false; // step 6 @@ -983,7 +1015,7 @@ ScriptedDirectProxyHandler::construct(JSContext* cx, HandleObject proxy, const C // step 4-5 RootedValue trap(cx); - if (!GetProperty(cx, handler, handler, cx->names().construct, &trap)) + if (!GetProxyTrap(cx, handler, cx->names().construct, &trap)) return false; // step 6 diff --git a/js/src/tests/ecma_6/Proxy/trap-null.js b/js/src/tests/ecma_6/Proxy/trap-null.js new file mode 100644 index 000000000000..703649e7c839 --- /dev/null +++ b/js/src/tests/ecma_6/Proxy/trap-null.js @@ -0,0 +1,103 @@ +/* + * Any copyright is dedicated to the Public Domain. + * http://creativecommons.org/licenses/publicdomain/ + */ + +var gTestfile = 'trap-null.js'; +var BUGNUMBER = 1257102; +var summary = "null as a trap value on a handler should operate on the target"; + +print(BUGNUMBER + ": " + summary); + +/************** + * BEGIN TEST * + **************/ + +// This might seem like overkill, but this proxying trick caught typos of +// several trap names before this test landed. \o/ /o\ +var allTraps = { + getPrototypeOf: null, + setPrototypeOf: null, + isExtensible: null, + preventExtensions: null, + getOwnPropertyDescriptor: null, + defineProperty: null, + has: null, + get: null, + set: null, + deleteProperty: null, + ownKeys: null, + apply: null, + construct: null, +}; + +var complainingHandler = new Proxy(allTraps, { + get(target, p, receiver) { + var v = Reflect.get(target, p, receiver); + if (v !== null) + throw new TypeError("failed to set one of the traps to null? " + p); + + return v; + } +}); + +var proxyTarget = function(x) { + "use strict"; + var str = this + x; + str += new.target ? "constructing" : "calling"; + return new.target ? new String(str) : str; +}; +proxyTarget.prototype.toString = () => "###"; +proxyTarget.prototype.valueOf = () => "@@@"; + +var proxy = new Proxy(proxyTarget, complainingHandler); + +assertEq(Reflect.getPrototypeOf(proxy), Function.prototype); + +assertEq(Object.getPrototypeOf(proxyTarget), Function.prototype); +assertEq(Reflect.setPrototypeOf(proxy, null), true); +assertEq(Object.getPrototypeOf(proxyTarget), null); + +assertEq(Reflect.isExtensible(proxy), true); + +assertEq(Reflect.isExtensible(proxyTarget), true); +assertEq(Reflect.preventExtensions(proxy), true); +assertEq(Reflect.isExtensible(proxy), false); +assertEq(Reflect.isExtensible(proxyTarget), false); + +var desc = Reflect.getOwnPropertyDescriptor(proxy, "length"); +assertEq(desc.value, 1); + +assertEq(desc.configurable, true); +assertEq(Reflect.defineProperty(proxy, "length", { value: 3, configurable: false }), true); +desc = Reflect.getOwnPropertyDescriptor(proxy, "length"); +assertEq(desc.configurable, false); + +assertEq(Reflect.has(proxy, "length"), true); + +assertEq(Reflect.get(proxy, "length"), 3); + +assertEq(Reflect.set(proxy, "length", 3), false); + +assertEq(Reflect.deleteProperty(proxy, "length"), false); + +var keys = Reflect.ownKeys(proxy); +assertEq(keys.length, 3); +keys.sort(); +assertEq(keys[0], "length"); +assertEq(keys[1], "name"); +assertEq(keys[2], "prototype"); + +assertEq(Reflect.apply(proxy, "hi!", [" "]), "hi! calling"); + +var res = Reflect.construct(proxy, [" - "]); +assertEq(typeof res, "object"); +assertEq(res instanceof String, true); +assertEq(res.valueOf(), "@@@ - constructing"); + +/******************************************************************************/ + +if (typeof reportCompare === "function") + reportCompare(true, true); + +print("Tests complete");