Object.freezing proxies should freeze or throw?

[Tom Van Cutsem]

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 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
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.mozilla.org/pipermail/es-discuss/attachments/20160808/f943897b/attachment-0001.html>