super.prop assignment can silently overwrite non-writable properties

[Caitlin Potter]

Right, good point. No need to care about `existingDescriptor.[[Get]]` or `existingDescriptor.[[Set]]`

> On Apr 20, 2015, at 10:12 PM, Allen Wirfs-Brock <allen at wirfs-brock.com> wrote:
> 
> 
> On Apr 20, 2015, at 6:52 PM, Caitlin Potter wrote:
> 
>> >>If the prop property accessed by super.prop is an accessor, super.prop = x; should invoke its setter. super.prop should invoke its getter.
>> >It does.  This is about what happens when that property is a data property doesn't exist. What happens when we do  [[HomeObject]].[[GetPrototypeOf]]().[[Set]]('prop', x, this)
>> 
>> I don’t think the accessor case does work. `ownDesc` never refers to the property descriptor of the receiver when O[P] is a SuperReference, so if there’s an `this.prop` is an accessor, and `super.prop` doesn’t exist, the data descriptor path is taken.
> 
> `ownDexc` refers to the property descriptor of the [[Prototype]] in this case and if "super.prop is an accessor" that will be an accessor property descriptor. That falls throw steps 4 and 5 and eventually invokes its setter in step 9.  The Receiver (the original `this` value only is involved as the `this` value passed in the call to the setter.
> 
> Allen
> 
> 
> 
> 
> 
> 
> 
> 
> 
>> 
>>> On Apr 20, 2015, at 9:37 PM, Allen Wirfs-Brock <allen at wirfs-brock.com <mailto:allen at wirfs-brock.com>> wrote:
>>> 
>>> 
>>> On Apr 20, 2015, at 6:21 PM, Mark Miller wrote:
>>> 
>>>> If the prop property accessed by super.prop is an accessor, super.prop = x; should invoke its setter. super.prop should invoke its getter.
>>> 
>>> It does.  This is about what happens when that property is a data property doesn't exist. What happens when we do  [[HomeObject]].[[GetPrototypeOf]]().[[Set]]('prop', x, this)
>>> 
>>> Allen
>>>    
>>> 
>>> 
>>> 
>>> 
>>> 
>>> 
>>> 
>>> 
>>>> 
>>>> On Tue, Apr 21, 2015 at 4:18 AM, Allen Wirfs-Brock <allen at wirfs-brock.com <mailto:allen at wirfs-brock.com>> wrote:
>>>> 
>>>> On Apr 20, 2015, at 12:39 PM, Jason Orendorff wrote:
>>>> 
>>>>> On Mon, Apr 20, 2015 at 12:44 PM, Allen Wirfs-Brock
>>>>> <allen at wirfs-brock.com <mailto:allen at wirfs-brock.com>> wrote:
>>>>>>> In the spec, 9.1.9 step 4.d.i. is where `super.prop = 2` ends up, with
>>>>>>> O=X.prototype.
>>>>>> 
>>>>>> 4.d.1 doesn't set the property, it just comes up with the property descriptor to use, if the `Receiver` does not already have a corresponding own property.
>>>>>> 
>>>>>> 5.c+5.e checks if the corresponding own property actually exists on the `Receiver`.
>>>>>> 
>>>>>> If it already exits then it does a [[DefineOwnProperty]] that only specifies the `value` attribute. This should respect the current `writable` attribute of the property and hence reject the attempt to change the value.
>>>>> 
>>>>> I agree with all of this, except I don't see where the attempt is
>>>>> rejected. Since the property is configurable, I think
>>>>> [[DefineOwnProperty]] succeeds.
>>>>> 
>>>>> The property is still non-writable afterwards. Only the value changes.
>>>>> 
>>>>> So this isn't breaking the object invariants: the property in question
>>>>> is configurable, so it's OK (I guess) to change the value. It's just
>>>>> surprising for assignment syntax to succeed in doing it.
>>>> 
>>>> I think it's bogus and needs to be corrected.  Not only does it allow (in weird cases for [[Set]] (ie, assignment) to change the value of a non-writable property.  It also means there are cases where [[Set]] will convert an accessor property to a data property.
>>>> 
>>>> In combination, I think this is a serious bug that needs to be fix in the final published ES6 spec.  The fix I propose is in 9.1.9 to replace Set 5.e as follows:
>>>> 
>>>> 5.e If existingDescriptor is not undefined, then
>>>>        i.   If IsAccessorDescript(existingDescript), return false.
>>>>        ii.  If existingDescriptor.[[Writable]] is false, return false.
>>>>        iii.  Let valueDesc be the PropertyDescriptor{[[Value]]: V}.
>>>>        iv.  Return Receiver.[[DefineOwnProperty]](P, valueDesc).
>>>> 
>>>> Lines 5.e.i and 5.e.ii are new additions.
>>>>  
>>>> Thoughts?
>>>> Allen
>>>> 
>>>> 
>>>> _______________________________________________
>>>> es-discuss mailing list
>>>> es-discuss at mozilla.org <mailto:es-discuss at mozilla.org>
>>>> https://mail.mozilla.org/listinfo/es-discuss <https://mail.mozilla.org/listinfo/es-discuss>
>>>> 
>>>> 
>>>> 
>>>> 
>>>> -- 
>>>> Text by me above is hereby placed in the public domain
>>>> 
>>>>   Cheers,
>>>>   --MarkM
>>> 
>>> _______________________________________________
>>> es-discuss mailing list
>>> es-discuss at mozilla.org <mailto:es-discuss at mozilla.org>
>>> https://mail.mozilla.org/listinfo/es-discuss <https://mail.mozilla.org/listinfo/es-discuss>
>> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.mozilla.org/pipermail/es-discuss/attachments/20150420/9f165579/attachment-0001.html>