Bug 1132522, part 2 - Treat false return from proxyHandler.set() as strict mode failure. r=efaust.

This commit is contained in:
Jason Orendorff 2015-02-13 09:49:31 -06:00
parent d8b9ff41ec
commit 0d62712dc6
8 changed files with 73 additions and 39 deletions

View File

@ -91,6 +91,7 @@ Handler.prototype = {
let [wrapped, path] = this.unwrap(target, key, value);
target[key] = value;
this._emitter.emit("set", path, value);
return true;
},
getOwnPropertyDescriptor: function(target, key) {
let desc = Object.getOwnPropertyDescriptor(target, key);

View File

@ -0,0 +1,40 @@
// Test handling of false return from a handler.set() hook.
load(libdir + "asserts.js");
var obj = {x: 1};
var p = new Proxy(obj, {
set(target, key, value, receiver) { return false; }
});
// Failing to set a property is a no-op in non-strict code.
assertEq(p.x = 2, 2);
assertEq(obj.x, 1);
// It's a TypeError in strict mode code.
assertThrowsInstanceOf(() => { "use strict"; p.x = 2; }, TypeError);
assertEq(obj.x, 1);
// Even if the value doesn't change.
assertThrowsInstanceOf(() => { "use strict"; p.x = 1; }, TypeError);
assertEq(obj.x, 1);
// Even if the target property doesn't already exist.
assertThrowsInstanceOf(() => { "use strict"; p.z = 1; }, TypeError);
assertEq("z" in obj, false);
// [].sort() mutates its operand only by doing strict [[Set]] calls.
var arr = ["not", "already", "in", "order"];
var p2 = new Proxy(arr, {
get(target, key, receiver) {
if (key === "length" && new Proxy([1], {}).length === 1) {
throw new Error("Congratulations! You have fixed bug 895223 at last! " +
"Please delete this whole get() method which is a " +
"workaround for that bug.");
}
return target[key];
},
set(target, key, value, receiver) { return false; }
});
assertThrowsInstanceOf(() => p2.sort(), TypeError);
assertDeepEq(arr, ["not", "already", "in", "order"]);

View File

@ -345,6 +345,7 @@ MSG_DEF(JSMSG_CANT_DEFINE_NE_AS_NC, 0, JSEXN_TYPEERR, "proxy can't define a n
MSG_DEF(JSMSG_PROXY_DEFINE_RETURNED_FALSE, 1, JSEXN_TYPEERR, "proxy defineProperty handler returned false for property '{0}'")
MSG_DEF(JSMSG_PROXY_DELETE_RETURNED_FALSE, 1, JSEXN_TYPEERR, "can't delete property '{0}': proxy deleteProperty handler returned false")
MSG_DEF(JSMSG_PROXY_PREVENTEXTENSIONS_RETURNED_FALSE, 0, JSEXN_TYPEERR, "proxy preventExtensions handler returned false")
MSG_DEF(JSMSG_PROXY_SET_RETURNED_FALSE, 1, JSEXN_TYPEERR, "proxy set handler returned false for property '{0}'")
MSG_DEF(JSMSG_CANT_REPORT_AS_NON_EXTENSIBLE, 0, JSEXN_TYPEERR, "proxy can't report an extensible object as non-extensible")
MSG_DEF(JSMSG_CANT_REPORT_C_AS_NC, 0, JSEXN_TYPEERR, "proxy can't report existing configurable property as non-configurable")
MSG_DEF(JSMSG_CANT_REPORT_E_AS_NE, 0, JSEXN_TYPEERR, "proxy can't report an existing own property as non-existent on a non-extensible object")

View File

@ -925,33 +925,29 @@ ScriptedDirectProxyHandler::get(JSContext *cx, HandleObject proxy, HandleObject
return true;
}
// ES6 (22 May, 2014) 9.5.9 Proxy.[[SetP]](P, V, Receiver)
// ES6 draft rev 32 (2015 Feb 2) 9.5.9 Proxy.[[Set]](P, V, Receiver)
bool
ScriptedDirectProxyHandler::set(JSContext *cx, HandleObject proxy, HandleObject receiver,
HandleId id, MutableHandleValue vp, ObjectOpResult &result) const
{
// step 2
// step 2-3 (Steps 1 and 4 are irrelevant assertions.)
RootedObject handler(cx, GetDirectProxyHandlerObject(proxy));
// step 3
if (!handler) {
JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_PROXY_REVOKED);
return false;
}
// step 4
// step 5-7
RootedObject target(cx, proxy->as<ProxyObject>().target());
// step 5-6
RootedValue trap(cx);
if (!GetProperty(cx, handler, handler, cx->names().set, &trap))
return false;
// step 7
// step 8
if (trap.isUndefined())
return DirectProxyHandler::set(cx, proxy, receiver, id, vp, result);
// step 8,10
// step 9-10
RootedValue value(cx);
if (!IdToStringOrSymbol(cx, id, &value))
return false;
@ -965,43 +961,37 @@ ScriptedDirectProxyHandler::set(JSContext *cx, HandleObject proxy, HandleObject
if (!Invoke(cx, ObjectValue(*handler), trap, ArrayLength(argv), argv, &trapResult))
return false;
// FIXME - bug 1132522: Step 9 is not implemented yet.
// if (!ToBoolean(trapResult))
// return result.fail(JSMSG_PROXY_SET_RETURNED_FALSE);
bool success = ToBoolean(trapResult);
// step 11
if (!ToBoolean(trapResult))
return result.fail(JSMSG_PROXY_SET_RETURNED_FALSE);
if (success) {
// step 12-13
Rooted<PropertyDescriptor> desc(cx);
if (!GetOwnPropertyDescriptor(cx, target, id, &desc))
return false;
// step 12-13
Rooted<PropertyDescriptor> desc(cx);
if (!GetOwnPropertyDescriptor(cx, target, id, &desc))
return false;
// step 14
if (desc.object()) {
if (IsDataDescriptor(desc) && desc.isPermanent() && desc.isReadonly()) {
bool same;
if (!SameValue(cx, vp, desc.value(), &same))
return false;
if (!same) {
JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_CANT_SET_NW_NC);
return false;
}
}
if (IsAccessorDescriptor(desc) && desc.isPermanent() && !desc.hasSetterObject()) {
JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_CANT_SET_WO_SETTER);
// step 14
if (desc.object()) {
if (IsDataDescriptor(desc) && desc.isPermanent() && desc.isReadonly()) {
bool same;
if (!SameValue(cx, vp, desc.value(), &same))
return false;
if (!same) {
JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_CANT_SET_NW_NC);
return false;
}
}
if (IsAccessorDescriptor(desc) && desc.isPermanent() && !desc.hasSetterObject()) {
JS_ReportErrorNumber(cx, GetErrorMessage, nullptr, JSMSG_CANT_SET_WO_SETTER);
return false;
}
}
// step 11, 15
// XXX FIXME - This use of vp is wrong. Bug 1132522 can clean it up.
vp.setBoolean(success);
// step 15
return result.succeed();
}
// ES6 (22 May, 2014) 9.5.13 Proxy.[[Call]]
bool
ScriptedDirectProxyHandler::call(JSContext *cx, HandleObject proxy, const CallArgs &args) const

View File

@ -59,7 +59,7 @@ var objWithSetter = {set "0"(val) { setterCalled = true}, length: 1};
assertEq(setterCalled, true);
var setHandlerCallCount = 0;
var proxy = new Proxy({length: 3}, {set: function(value) {setHandlerCallCount++;}});
var proxy = new Proxy({length: 3}, {set(t, i, v, r) { setHandlerCallCount++; return true; }});
[].fill.call(proxy, 2);
assertEq(setHandlerCallCount, 3);

View File

@ -20,6 +20,7 @@ function LoggingProxy(target) {
set: function (t, id, v) {
log.push("set", id);
t[id] = v;
return true;
}
};
return new Proxy(target || [], h);

View File

@ -31,6 +31,7 @@ for (var constructor of constructors) {
set: function (t, id, v) {
log.push("set", id);
t[id] = v;
return true;
}
};
return new Proxy(Object(target), h);

View File

@ -29,11 +29,11 @@ namespace js {
*
* https://developer.mozilla.org/en-US/docs/SpiderMonkey/Internals/Bytecode
*/
static const uint32_t XDR_BYTECODE_VERSION_SUBTRAHEND = 246;
static const uint32_t XDR_BYTECODE_VERSION_SUBTRAHEND = 247;
static const uint32_t XDR_BYTECODE_VERSION =
uint32_t(0xb973c0de - XDR_BYTECODE_VERSION_SUBTRAHEND);
static_assert(JSErr_Limit == 385,
static_assert(JSErr_Limit == 386,
"GREETINGS, POTENTIAL SUBTRAHEND INCREMENTER! If you added or "
"removed MSG_DEFs from js.msg, you should increment "
"XDR_BYTECODE_VERSION_SUBTRAHEND and update this assertion's "