Object.freezing proxies should freeze or throw?

[Mark S. Miller]

On Mon, Aug 8, 2016 at 1:35 PM, Tom Van Cutsem <tomvc.be at gmail.com> 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 <claude.pache at gmail.com>:
>
>>
>> > Le 8 août 2016 à 11:02, Claude Pache <claude.pache at gmail.com> 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.mozilla.org/pipermail/es-discuss/attachments/20160808/2fc3c80c/attachment.html>