On Mon, Aug 8, 2016 at 1:35 PM, Tom Van Cutsem <[email protected]> wrote:
> Claude's additional example is indeed evidence that Object.freeze is not > to blame, but rather that the invariant checks of > [[GetOwnPropertyDescriptor]] and [[DefineOwnProperty]] are too weak. The > culprit is, as far as I can tell, that we re-used the state transitions > allowed by DefineOwnProperty, which allows the transition from > non-configurable,writable (-c+w) to non-configurable,non-writable (-c-w) > and which is unwanted here, as Claude rightfully concludes *[to better > understand the state transitions involved, see MarkM's lucid state > transition chart of property attributes > <http://wiki.ecmascript.org/doku.php?id=es3.1:attribute_states > <http://wiki.ecmascript.org/doku.php?id=es3.1:attribute_states>>. I wish > this diagram were in the spec btw, once you know how to read it, it is > significantly easier to understand than the DefineOwnProperty algorithm > itself]* > I would like to see it in the spec too. > > I like the simplicity of Claude's suggestion, but I think that check is > too tight. Technically if the descriptors are both -c+w data descriptors, > their value attributes need not be identical to honor the spec invariants. > Instead I would propose to tackle the problem head-on and explicitly > disallow a proxy from reporting a -c-w property if the corresponding target > property is -c+w. We already have a similar check in place in precisely > those two algorithms to test if the configurable attribute matches. It is > just a matter of tightening that check to also verify the writable > attribute. > > This would entail the following changes to the ES2015 spec (new or > modified text in bold): > > > 9.5.5 [[GetOwnProperty]] (P) > ... > 22. If resultDesc.[[Configurable]] is false, then > a. If targetDesc is undefined or targetDesc.[[Configurable]] is true, > then > i. Throw a TypeError exception. > * b. If resultDesc.[[Writable]] is false and targetDesc.[[Writable]] is > true, then* > * i. Throw a TypeError exception.* > > NOTE [[GetOwnProperty]] for proxy objects enforces the following > invariants: > > ... > * * A property cannot be reported as non-configurable, non-writable if it > exists as a non-configurable, writable own property on the target object.* > > > 9.5.6 [[DefineOwnProperty]] (P, Desc) > ... > 20. Else targetDesc is not undefined, > a. If IsCompatiblePropertyDescriptor(extensibleTarget, Desc , > targetDesc) is false, throw a TypeError exception. > * b. If settingConfigFalse is true* > * i. If targetDesc.[[Configurable]] is true, throw a TypeError > exception.* > * ii. If Desc.[[Writable]] is false and targetDesc.[[Writable]] is > true, throw a TypeError exception.* > > NOTE [[DefineOwnProperty]] for proxy objects enforces the following > invariants: > > ... > * * A property cannot be successfully set to non-configurable, > non-writable if the corresponding own property of the target object is > non-configurable, writable.* > > WDYT? > > Regards, > Tom > > 2016-08-08 15:37 GMT+02:00 Claude Pache <[email protected]>: > >> >> > Le 8 août 2016 à 11:02, Claude Pache <[email protected]> a écrit : >> > >> > Here is another test case, with [[GetOwnPropertyDescriptor]]: >> > >> > ```js >> > var target = Object.seal({x: 2}); >> > var proxy = new Proxy(target, { >> > getOwnPropertyDescriptor(o, p) { >> > var d = Reflect.getOwnPropertyDescriptor(o, p) >> > if (d && 'writable' in d) >> > d.writable = false >> > return d >> > } >> > }); >> > >> > Object.getOwnPropertyDescriptor(proxy, 'x'); // { value: 2, writable: >> false, configurable: false } >> > >> > Object.defineProperty(proxy, 'x', { value: 3 }) >> > >> > Object.getOwnPropertyDescriptor(proxy, 'x'); // { value: 3, writable: >> false, configurable: false } >> > ``` >> > >> > IMHO, the most robust fix is the following: Both the >> [[DefineOwnProperty]] and the [[GetOwnPropertyDescriptor]] contain the >> following check: >> > >> > * If IsCompatiblePropertyDescriptor(extensibleTarget, resultDesc, >> targetDesc) is false, throw a TypeError exception. >> > >> > I think there should be also the following check (except when >> targetDesc is undefined, in which case another appropriate check is used): >> > >> > * If IsCompatiblePropertyDescriptor(extensibleTarget, targetDesc, >> resultDesc) is false, throw a TypeError exception. >> > >> > That would replace the weaker adhoc check about the [[Configurable]] >> attribute (search for settingConfigFalse) in those algorithms. >> > >> > (Alternatively, in order to avoid double checks, define an >> AreBothWaysCompatiblePropertyDescriptors(extensibleTarget, desc1, desc2) >> abstract operation.) >> > >> > —Claude >> > >> >> Looking closer, it seems that using IsCompatiblePropertyDescriptor is in >> fact an overkill, because we probably don’t want the special case of >> conditionally mutable [[Writable]] attribute for nonconfigurable properties. >> >> That is, I think that the following condition must hold: If either the >> target has a nonconfigurable property, or the proxy claims to have a >> nonconfigurable property, then every attribute of the property descriptor >> claimed by the proxy must be identical to the corresponding attribute of >> the property descriptor of the target. >> >> —Claude >> >> >> > -- Cheers, --MarkM
_______________________________________________ es-discuss mailing list [email protected] https://mail.mozilla.org/listinfo/es-discuss
