Making operations on property descriptors more robust against Object.prototype hazards

Andrea Giammarchi andrea.giammarchi at gmail.com
Mon Sep 15 07:54:42 PDT 2014


I have simply shown how the logic would most likely be implemented via JS,
not how C++ engine should do that so performance on JS side are not "the"
concern but like I've said, the whole thing is probably not worth a fix.

However, I do believe nobody in the real world use inheritance for
descriptors ... so I'd rather simply drop this potentially dangerous
behavior and ask Brandon Benvie to simply returns {properties} in his
constructors used as descriptors.

I find that a complete anti pattern ... how do you even call them,
EnumerableButNotConfigurable, EnumerableAndConfigurable ... what the hack ?

Just my opinion, it should not have been considered inherited properties
since the beginning.

Best Regards





On Mon, Sep 15, 2014 at 3:06 PM, Erik Arvidsson <erik.arvidsson at gmail.com>
wrote:

> Adding a new way to interpret properties is a bad idea. No engine is going
> to optimize this new special form of [[Get]] and this will deopt all calls
> that directly depend on descriptors.
> On Sep 15, 2014 4:56 AM, "Andrea Giammarchi" <andrea.giammarchi at gmail.com>
> wrote:
>
>> IIRC Allen proposed to change specs so that inheritance was considered
>> down to Object.prototype **but** without including it.
>>
>> A snippet that some how describes the logic would be something like the
>> following
>>
>> ```js
>> function defineProperty(obj, prop, descriptor) {
>>   var
>>     withOwnDesc = {
>>       __proto__: null,
>>       enumerable: findProperty(descriptor, 'enumerable'),
>>       configurable: findProperty(descriptor, 'configurable')
>>     },
>>     get = findProperty(descriptor, 'get'),
>>     set = findProperty(descriptor, 'set')
>>   ;
>>   if (!get && !set) {
>>     withOwnDesc.writable = findProperty(descriptor, 'writable');
>>     withOwnDesc.value = findProperty(descriptor, 'value');
>>   } else {
>>     if (get) {
>>       withOwnDesc.get = get;
>>     }
>>     if (set) {
>>       withOwnDesc.set = set;
>>     }
>>   }
>>   return Object.defineProperty(obj, prop, withOwnDesc);
>> }
>>
>> function findProperty(obj, prop) {
>>   var has = Object.prototype.hasOwnProperty;
>>   while (!has.call(obj, prop)) {
>>     obj = Object.getPrototypeOf(obj);
>>     if (!obj || obj === Object.prototype) {
>>       return false;
>>     }
>>   }
>>   return obj[prop];
>> }
>>
>> Object.prototype.configurable = true;
>> var p = {enumerable: true};
>> var o = Object.create(p);
>> o.writable = true;
>> o.value = 123;
>>
>>
>> defineProperty({}, 'test', o);
>>
>> // descriptor would be
>> {
>>   writable: true,
>>   enumerable: true,
>>   configurable: false,
>>   value: 123
>> }
>> ```
>>
>> Moved into native might not be such huge problem, performance speaking,
>> but the web survived until now so maybe it's OK to keep it fragile as it
>> is, it has also the benefit of making suddenly everything enumerable and
>> configurable for testing purposes so ...
>>
>> Best Regards
>>
>>
>>
>>
>> On Sun, Sep 14, 2014 at 1:58 AM, Mark S. Miller <erights at google.com>
>> wrote:
>>
>>> +1
>>>
>>> Adding string-named properties to Object.prototype will create all sorts
>>> of hazards. The only way to avoid such hazards is not to do that. We do not
>>> need to pervert other APIs to make this fatally bad practice slightly less
>>> fatal.
>>>
>>> If you want to actually defend against such hazards rather than blindly
>>> trusting all you code not to add properties to Object.prototype, do
>>>
>>>     Object.preventExtensions(Object.prototype);
>>>
>>> However, this will also prevent the addition of symbol-named properties,
>>> which are still problematic but much less so.
>>>
>>> (Or you could load SES and not have to worry about any such problems ;).)
>>>
>>>
>>>
>>> On Sat, Sep 13, 2014 at 1:55 PM, Tom Van Cutsem <tomvc.be at gmail.com>
>>> wrote:
>>>
>>>> 2014-09-13 17:49 GMT+02:00 Brendan Eich <brendan at mozilla.org>:
>>>>
>>>>> It's not nice to lose .toString, though.
>>>>>
>>>>> Tom, didn't you have an idea for a fix? I can't find it but vaguely
>>>>> recall it (or dreamt it!).
>>>>
>>>>
>>>> No, not that I know of. As highlighted in the old thread linked to by
>>>> Claude, there is a legitimate use for considering inherited attributes:
>>>> inheriting defaults. Brandon Benvie stepped in to acknowledge he actually
>>>> used this pattern.
>>>>
>>>> Of course, in the old thread we were talking about Object.{dP, gOPD}
>>>> which we can't change because of backwards-compat issues. Claude is
>>>> suggesting we can still change Reflect.{dP, gOPD} in this regard. I'm
>>>> hesitant to do so though, because I feel the root cause of the problem is
>>>> the monkey-patching of Object.prototype, rather than property descriptors
>>>> being "badly designed" just because they consider inherited attributes.
>>>> Even if we would fix this particular part of the Reflect API, code that
>>>> does `Object.prototype.get = ...` is likely to cause other bugs as well.
>>>>
>>>> Speaking about that particular example: if one adds
>>>> `Object.prototype.get = function(){}`, then trying to define a data
>>>> descriptor will immediately fail noisily because the spec explicitly checks
>>>> that a descriptor can't be both a data and accessor property at the same
>>>> time, so this particular conflict is likely to be detected very soon.
>>>>
>>>> Cheers,
>>>> Tom
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>>
>>>>>
>>>>> /be
>>>>>
>>>>>
>>>>> Claude Pache wrote:
>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> As noted some time ago, there is an issue with
>>>>>> `Object.defineProperty` when it happens that someone has the facetious idea
>>>>>> to give some value to `Object.prototype.get` [1].
>>>>>>
>>>>>> While it is hardly an issue in day-to-day programming, I think that
>>>>>> it is a good idea to make sure that specialized API (Reflect, proxy traps)
>>>>>> are robust by design against that bug^H^H^H feature. Concretely, I propose:
>>>>>>
>>>>>> (1) proxy.[[DefineOwnProperty]] — When a proxy defines the
>>>>>> corresponding trap, it shall receive an object with no prototype as
>>>>>> property descriptor.
>>>>>>
>>>>>> (2) Reflect.getOwnPropertyDescriptor —That method shall return an
>>>>>> object with no prototype, so that it can be readily used elsewhere, e.g. in
>>>>>> Reflect.defineProperty.
>>>>>>
>>>>>
>>>>
>>>> _______________________________________________
>>>> es-discuss mailing list
>>>> es-discuss at mozilla.org
>>>> https://mail.mozilla.org/listinfo/es-discuss
>>>>
>>>>
>>>
>>>
>>> --
>>>     Cheers,
>>>     --MarkM
>>>
>>> _______________________________________________
>>> es-discuss mailing list
>>> es-discuss at mozilla.org
>>> https://mail.mozilla.org/listinfo/es-discuss
>>>
>>>
>>
>> _______________________________________________
>> es-discuss mailing list
>> es-discuss at mozilla.org
>> https://mail.mozilla.org/listinfo/es-discuss
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.mozilla.org/pipermail/es-discuss/attachments/20140915/0ccf33fe/attachment-0001.html>


More information about the es-discuss mailing list