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
authorJeff Walden <jwalden@mit.edu>
Fri, 18 Mar 2016 16:43:43 -0700
changeset 289955 16e429ad56969b36f7b7af0c2f1076771dba8652
parent 289954 4f1482e6da917442cfd73f8994ebac3c58f8fe5c
child 289956 928b0a26ff0f5468b3ffd8a4ff02c42d055c782c
push id19656
push usergwagner@mozilla.com
push dateMon, 04 Apr 2016 13:43:23 +0000
treeherderb2g-inbound@e99061fde28a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersevilpie
bugs1257102
milestone48.0a1
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
js/src/js.msg
js/src/proxy/ScriptedDirectProxyHandler.cpp
js/src/tests/ecma_6/Proxy/trap-null.js
--- a/js/src/js.msg
+++ b/js/src/js.msg
@@ -376,16 +376,17 @@ MSG_DEF(JSMSG_MUST_REPORT_SAME_VALUE,  0
 MSG_DEF(JSMSG_MUST_REPORT_UNDEFINED,   0, JSEXN_TYPEERR, "proxy must report undefined for a non-configurable accessor property without a getter")
 MSG_DEF(JSMSG_OBJECT_ACCESS_DENIED,    0, JSEXN_ERR, "Permission denied to access object")
 MSG_DEF(JSMSG_PROPERTY_ACCESS_DENIED,  1, JSEXN_ERR, "Permission denied to access property {0}")
 MSG_DEF(JSMSG_PROXY_CONSTRUCT_OBJECT,  0, JSEXN_TYPEERR, "proxy [[Construct]] must return an object")
 MSG_DEF(JSMSG_PROXY_EXTENSIBILITY,     0, JSEXN_TYPEERR, "proxy must report same extensiblitity as target")
 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")
 MSG_DEF(JSMSG_SC_BAD_SERIALIZED_DATA,  1, JSEXN_INTERNALERR, "bad serialized structured data ({0})")
 MSG_DEF(JSMSG_SC_DUP_TRANSFERABLE,     0, JSEXN_TYPEERR, "duplicate transferable for structured clone")
 MSG_DEF(JSMSG_SC_NOT_TRANSFERABLE,     0, JSEXN_TYPEERR, "invalid transferable array for structured clone")
 MSG_DEF(JSMSG_SC_UNSUPPORTED_TYPE,     0, JSEXN_TYPEERR, "unsupported type for structured data")
 MSG_DEF(JSMSG_SC_SHMEM_MUST_TRANSFER,  0, JSEXN_TYPEERR, "SharedArrayBuffer must be explicitly transfered during structured cloning")
--- a/js/src/proxy/ScriptedDirectProxyHandler.cpp
+++ b/js/src/proxy/ScriptedDirectProxyHandler.cpp
@@ -123,16 +123,48 @@ ValidatePropertyDescriptor(JSContext* cx
 // Get the [[ProxyHandler]] of a scripted direct proxy.
 static JSObject*
 GetDirectProxyHandlerObject(JSObject* proxy)
 {
     MOZ_ASSERT(proxy->as<ProxyObject>().handler() == &ScriptedDirectProxyHandler::singleton);
     return proxy->as<ProxyObject>().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
 ScriptedDirectProxyHandler::getPrototype(JSContext* cx, HandleObject proxy,
                                          MutableHandleObject protop) const
 {
     RootedObject target(cx, proxy->as<ProxyObject>().target());
     // Though handler is used elsewhere, spec mandates that both get set to null.
@@ -184,17 +216,17 @@ ScriptedDirectProxyHandler::preventExten
         return false;
     }
 
     // Step 4.
     RootedObject target(cx, proxy->as<ProxyObject>().target());
 
     // 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.
     if (trap.isUndefined())
         return PreventExtensions(cx, target, result);
 
     // Steps 8-9.
     Value argv[] = {
@@ -232,17 +264,17 @@ ScriptedDirectProxyHandler::isExtensible
         return false;
     }
 
     // step 3
     RootedObject target(cx, proxy->as<ProxyObject>().target());
 
     // 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
     if (trap.isUndefined())
         return IsExtensible(cx, target, extensible);
 
     // step 7, 9
     Value argv[] = {
@@ -285,17 +317,17 @@ ScriptedDirectProxyHandler::getOwnProper
         return false;
     }
 
     // step 4
     RootedObject target(cx, proxy->as<ProxyObject>().target());
 
     // 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
     if (trap.isUndefined())
         return GetOwnPropertyDescriptor(cx, target, id, desc);
 
     // step 8-9
     RootedValue propKey(cx);
@@ -405,17 +437,17 @@ ScriptedDirectProxyHandler::defineProper
         return false;
     }
 
     // step 5
     RootedObject target(cx, proxy->as<ProxyObject>().target());
 
     // 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
     if (trap.isUndefined())
         return DefineProperty(cx, target, id, desc, result);
 
     // step 9
     RootedValue descObj(cx);
@@ -532,17 +564,17 @@ ScriptedDirectProxyHandler::ownPropertyK
     }
     // Step 3. Superfluous assertion.
 
     // Step 4.
     RootedObject target(cx, proxy->as<ProxyObject>().target());
 
     // 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.
     if (trap.isUndefined())
         return GetPropertyKeys(cx, target, JSITER_OWNONLY | JSITER_HIDDEN | JSITER_SYMBOLS, &props);
 
     // Step 8.
     Value argv[] = {
@@ -669,17 +701,17 @@ ScriptedDirectProxyHandler::delete_(JSCo
         return false;
     }
 
     // steps 4-5
     RootedObject target(cx, proxy->as<ProxyObject>().target());
 
     // 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
     if (trap.isUndefined())
         return DeleteProperty(cx, target, id, result);
 
     // steps 9-10
     RootedValue value(cx);
@@ -726,17 +758,17 @@ ScriptedDirectProxyHandler::has(JSContex
         return false;
     }
 
     // step 4
     RootedObject target(cx, proxy->as<ProxyObject>().target());
 
     // 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
     if (trap.isUndefined())
         return HasProperty(cx, target, id, bp);
 
     // step 8,10
     RootedValue value(cx);
@@ -794,17 +826,17 @@ ScriptedDirectProxyHandler::get(JSContex
         return false;
     }
 
     // step 4
     RootedObject target(cx, proxy->as<ProxyObject>().target());
 
     // 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
     if (trap.isUndefined())
         return GetProperty(cx, target, receiver, id, vp);
 
     // step 8-9
     RootedValue value(cx);
@@ -859,17 +891,17 @@ ScriptedDirectProxyHandler::set(JSContex
     if (!handler) {
         JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_PROXY_REVOKED);
         return false;
     }
 
     // step 5-7
     RootedObject target(cx, proxy->as<ProxyObject>().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
     if (trap.isUndefined())
         return SetProperty(cx, target, id, v, receiver, result);
 
     // step 9-10
     RootedValue value(cx);
@@ -935,17 +967,17 @@ ScriptedDirectProxyHandler::call(JSConte
 
     // step 7
     RootedObject argsArray(cx, NewDenseCopiedArray(cx, args.length(), args.array()));
     if (!argsArray)
         return false;
 
     // 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
     if (trap.isUndefined()) {
         RootedValue targetv(cx, ObjectValue(*target));
         return Invoke(cx, args.thisv(), targetv, args.length(), args.array(), args.rval());
     }
 
@@ -978,17 +1010,17 @@ ScriptedDirectProxyHandler::construct(JS
 
     // step 7
     RootedObject argsArray(cx, NewDenseCopiedArray(cx, args.length(), args.array()));
     if (!argsArray)
         return false;
 
     // 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
     if (trap.isUndefined()) {
         ConstructArgs cargs(cx);
         if (!FillArgumentsFromArraylike(cx, cargs, args))
             return false;
 
new file mode 100644
--- /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");