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

Erik Arvidsson erik.arvidsson at gmail.com
Mon Sep 15 07:06:21 PDT 2014


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/fbc58e82/attachment.html>


More information about the es-discuss mailing list