Skip to content

Commit

Permalink
added bugfixes reported in tc39 issue666 <tc39/ecma262#666>, missing …
Browse files Browse the repository at this point in the history 
…proxy invariant checks in getOwnPropertyDescriptor, defineProperty and deleteProperty
  • Loading branch information
Tom Van Cutsem committed Aug 17, 2016
1 parent e781f35 commit 99ee7f5
Show file tree
Hide file tree
Showing 2 changed files with 130 additions and 17 deletions.
74 changes: 58 additions & 16 deletions reflect.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -652,15 +652,30 @@ Validator.prototype = {
"for property '"+name+"'");
}
}

if (desc.configurable === false && !isSealedDesc(targetDesc)) {
// if the property is configurable or non-existent on the target,
// but is reported as a non-configurable property, it may later be
// reported as configurable or non-existent, which violates the
// invariant that if the property might change or disappear, the
// configurable attribute must be true.
throw new TypeError("cannot report a non-configurable descriptor "+
"for configurable or non-existent property '"+name+"'");

if (desc.configurable === false) {
if (targetDesc === undefined || targetDesc.configurable === true) {
// if the property is configurable or non-existent on the target,
// but is reported as a non-configurable property, it may later be
// reported as configurable or non-existent, which violates the
// invariant that if the property might change or disappear, the
// configurable attribute must be true.
throw new TypeError(
"cannot report a non-configurable descriptor " +
"for configurable or non-existent property '" + name + "'");
}
if ('writable' in desc && desc.writable === false) {
if (targetDesc.writable === true) {
// if the property is non-configurable, writable on the target,
// but is reported as non-configurable, non-writable, it may later
// be reported as non-configurable, writable again, which violates
// the invariant that a non-configurable, non-writable property
// may not change state.
throw new TypeError(
"cannot report non-configurable, writable property '" + name +
"' as non-configurable, non-writable");
}
}
}

return desc;
Expand Down Expand Up @@ -751,8 +766,8 @@ Validator.prototype = {
}

name = String(name);
desc = normalizePropertyDescriptor(desc);
var success = trap.call(this.handler, this.target, name, desc);
var descObj = normalizePropertyDescriptor(desc);
var success = trap.call(this.handler, this.target, name, descObj);
success = !!success; // coerce to Boolean

if (success === true) {
Expand All @@ -775,6 +790,21 @@ Validator.prototype = {
throw new TypeError("cannot define incompatible property "+
"descriptor for property '"+name+"'");
}
if (isDataDescriptor(targetDesc) &&
targetDesc.configurable === false &&
targetDesc.writable === true) {
if (desc.configurable === false && desc.writable === false) {
// if the property is non-configurable, writable on the target
// but was successfully reported to be updated to
// non-configurable, non-writable, it can later be reported
// again as non-configurable, writable, which violates
// the invariant that non-configurable, non-writable properties
// cannot change state
throw new TypeError(
"cannot successfully define non-configurable, writable " +
" property '" + name + "' as non-configurable, non-writable");
}
}
}

if (desc.configurable === false && !isSealedDesc(targetDesc)) {
Expand All @@ -783,9 +813,10 @@ Validator.prototype = {
// it may later be reported as configurable or non-existent, which violates
// the invariant that if the property might change or disappear, the
// configurable attribute must be true.
throw new TypeError("cannot successfully define a non-configurable "+
"descriptor for configurable or non-existent property '"+
name+"'");
throw new TypeError(
"cannot successfully define a non-configurable " +
"descriptor for configurable or non-existent property '" +
name + "'");
}

}
Expand Down Expand Up @@ -829,11 +860,22 @@ Validator.prototype = {
var res = trap.call(this.handler, this.target, name);
res = !!res; // coerce to Boolean

var targetDesc;
if (res === true) {
if (isSealed(name, this.target)) {
throw new TypeError("property '"+name+"' is non-configurable "+
targetDesc = Object.getOwnPropertyDescriptor(this.target, name);
if (targetDesc !== undefined && targetDesc.configurable === false) {
throw new TypeError("property '" + name + "' is non-configurable "+
"and can't be deleted");
}
if (targetDesc !== undefined && !Object_isExtensible(this.target)) {
// if the property still exists on a non-extensible target but
// is reported as successfully deleted, it may later be reported
// as present, which violates the invariant that an own property,
// deleted from a non-extensible object cannot reappear.
throw new TypeError(
"cannot successfully delete existing property '" + name +
"' on a non-extensible object");
}
}

return res;
Expand Down
73 changes: 72 additions & 1 deletion test/testProxies.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ load('../reflect.js');
testUpdatePropertyDescriptor();
testDeprecatedGetOwnPropertyNames();
testProxiesForArrays();
testMissingInvariants();
//testInvokeTrap();

for (var testName in TESTS) {
Expand Down Expand Up @@ -884,7 +885,77 @@ load('../reflect.js');
// below test fails because JSON.stringify uses a [[Class]] check
// to test whether p is an array, and we can't intercept that
// assert(Array.isArray(JSON.parse(JSON.stringify(p))), 'JSON stringify array');
};
}

// see https://github.com/tc39/ecma262/pull/666
function testMissingInvariants() {
// a property reported as nonwritable, nonconfigurable must not
// change its value
(function() {
var target = Object.seal({x: 2});
var proxy = new Proxy(target, {
getOwnPropertyDescriptor: function(o, p) {
var desc = Reflect.getOwnPropertyDescriptor(o, p);
if (desc && 'writable' in desc) {
desc.writable = false;
}
return desc;
}
});

assertThrows(
"cannot report non-configurable, writable property 'x'" +
" as non-configurable, non-writable", function() {
Object.getOwnPropertyDescriptor(proxy, 'x'); // !!! should throw
// { value: 2, configurable: false, writable: false }
Object.defineProperty(proxy, 'x', { value: 3 });
var d = Object.getOwnPropertyDescriptor(proxy, 'x'); // { value: 3 }
});
}());

// a property defined as nonwritable, nonconfigurable
// must not change its value
(function() {
var target = Object.seal({x: 2});
var proxy = new Proxy(target, {
defineProperty: function(o, p, desc) {
delete desc.writable;
return Reflect.defineProperty(o, p, desc);
}
});

assertThrows(
"cannot successfully define non-configurable, writable " +
" property 'x' as non-configurable, non-writable", function() {
Object.defineProperty(proxy, 'x', {
value: 2,
configurable: false,
writable: false });
// !!! should throw
proxy.x = 3
Object.getOwnPropertyDescriptor(proxy, 'x'); // { value: 3 }
});
}());

// a successfuly deleted property on a nonextensible object
// must not reappear
(function() {
"use strict";
var target = Object.preventExtensions({ x: 1 });
var proxy = new Proxy(target, {
deleteProperty: function() { return true; }
});

assert(!Object.isExtensible(proxy), 'proxy is non-extensible');
assertThrows(
"cannot successfully delete existing property 'x'" +
" on a non-extensible object", function() {
delete proxy.x; // true !!! should throw
proxy.hasOwnProperty('x'); // true
});
}());

}

// invoke experiment
/*function testInvokeTrap() {
Expand Down

0 comments on commit 99ee7f5

Please sign in to comment.