Discuss
  • Object.prototype.get & bye bye Object.defineProperty

Object.prototype.get & bye bye Object.defineProperty

# Andrea Giammarchi (12 years ago)

This was not true at some point, but now it seems to be the case in every browser.

Just

Object.prototype.get = function(){};

and any further attempt to use Object.defineProperty(obj, key, {value:123}) will fail because defineProperty checks inherited get property, or set, and these cannot be used together with value ...

Is this a bug or kinda a joke ?

Scary stuff, IMHO, thanks for clarifications.

br

# Allen Wirfs-Brock (12 years ago)

On Nov 28, 2012, at 6:40 PM, Andrea Giammarchi wrote:

This was not true at some point, but now it seems to be the case in every browser.

Just Object.prototype.get = function(){};

and any further attempt to use Object.defineProperty(obj, key, {value:123}) will fail because defineProperty checks inherited get property, or set, and these cannot be used together with value ...

Is this a bug or kinda a joke ?

It's what the ES5/5.1 spec says. this is the first time I've seen this raised as an issue and as far as I know it was always been that way.

It conceivably could be changed to only check own properties but I wonder if that might not break more code than that which actually has this problem. There are at least semi-plausable reasons why you might want to intentionally use inherited properties in a property descriptor:

var defaultDataProperty = {enumerable: true, configurable: true, writable: true};

Object.defineProperty(obj,key, Object.create(defaultDataProperty, {value: 123});

Who knows whether anybody does things like this.

You can protect yourself against exposure to such Object.prototype attacks by using patterns like:

Object.defineProperty(obj,key,Object.create(null, {value: 123};

# Brandon Benvie (12 years ago)

To answer that question, I do. I reuse descriptors often for performance but also use prototypal inheritance as well.Since most descriptors share the same common properties for all but one (or two) fields, they are an ideal candidate for use with prototypal inheritance.

The answer though in that case is easy enough though, make sure the prototype descriptor is created with Object.create(null). This wouldn't solve compatibility issues with in-the-wild code but it solves the issue for most people who care enough to use anything dealing with descriptors.

Who knows whether anybody does things like this

# David Bruant (12 years ago)

Le 29/11/2012 06:20, Brandon Benvie a écrit :

The answer though in that case is easy enough though, make sure the prototype descriptor is created with Object.create(null). This wouldn't solve compatibility issues with in-the-wild code but it solves the issue for most people who care enough to use anything dealing with descriptors.

Object.freeze(Object.prototype) is also a solution. Incidentally, it protects also existing built-ins.

For property descriptors, I've also seen an interesting style in the dom.js code [1]. Basically, there are helper functions (attribute, constant) which generate the property descriptor to be used as second argument of Object.create. This case is very WebIDL-centric, but that could probably work in other cases. In these cases, the function generating property descriptors could use Object.create(null) under the hood.

David

[1] andreasgal/dom.js/blob/84b7ab6cf9fa2f41aa4dfbe661f2747b938ca292/src/impl/Node.js

# Nathan Wall (12 years ago)

Object.defineProperty(obj,key, Object.create(defaultDataProperty, {value: 123});

Er, shouldn't that be: Object.defineProperty(obj,key,Object.create(null, {value: {value: 123}})); Seems pretty excessive. It's probably too late to make defineProperty only look at own properties, but how about making it ignore properties inherited from Object.prototype? In addition, it'd be nice if there was an easier way to create an object with no [[Prototype]]... some sort of addition to object literal notation?

# David Bruant (12 years ago)

Le 29/11/2012 16:47, Nathan Wall a écrit :

In addition, it'd be nice if there was an easier way to create an object with no [[Prototype]]... some sort of addition to object literal notation?

I think ES6 will have such a notation with the proto de facto standardization:

 var o = {
     __proto__: null,
     a: 1,
     b: 2
 };

It's not mentionned in the strawman, but I think I've read it once on es-discuss.

# Allen Wirfs-Brock (12 years ago)

On Nov 29, 2012, at 7:47 AM, Nathan Wall wrote:

Object.defineProperty(obj,key, Object.create(defaultDataProperty, {value: 123});

Er, shouldn't that be:

Object.defineProperty(obj,key,Object.create(null, {value: {value: 123}}));

oops, yes create requires an addition object layer

Seems pretty excessive. It's probably too late to make defineProperty only look at own properties, but how about making it ignore properties inherited from Object.prototype?

Fairly complex to determine. Would it really be worth it?

# David Bruant (12 years ago)

Le 29/11/2012 17:02, Allen Wirfs-Brock a écrit :

On Nov 29, 2012, at 7:47 AM, Nathan Wall wrote:

Seems pretty excessive. It's probably too late to make defineProperty only look at own properties, but how about making it ignore properties inherited from Object.prototype? Fairly complex to determine. Would it really be worth it?

I don't believe so. Andreas exposed a potential attack, but there is already a standard and widely implemented way to prevent this attack (there are actually several!).

# Andrea Giammarchi (12 years ago)

I really don't understand what is exactly the advantage of considering inherited properties for descriptors.

If you create a new instance with those properties using a "class" to avoid writing them ... what are you gaining against a literal object?

If you are doing this in order to have less code so that you can new EnumerableWithValue(value) why don't you

function EnumerableWithValue(value) { return {enumerable:true, value:value}; }

If you want to reuse objects in order to avoid greedy GC operations, why don't you do the way I do which is to create a single object once and update its value during definition?

var descriptor = {value: null};

function withValue(value) { descriptor.value = value; return descriptor; }

Object.defineProperty(obj, "prop", withValue(123));

The reason I have discovered this problem is that I was trying to sandbox an enriched Object.prototype in order to implement get(key) / set(key, value) environment to show an example for the other discussion.

When I've developed polpetta for node.js I remember I could not use inherited properties then, as shown here, I've realized I don't need them at all.

Now even node.js consider inherited properties with defineProperty so I have double checked and realized that's how it should be accordingly with specs but I believe this is a problem

Descriptors are small enough to justify the usage of inheritance for them, imho, also in this way is too easy to break everything because accidentally some script polluted the Object.prototype with a value method, as example ... or an enumerable: true.

Correct, setting Object.prototype.enumerable = true; will make a whole environment enumerable for all those properties where the descriptor has no default because by default specs say enumerable should be false.

Then we have configurable and writable so this is in my opinion a massive security hole in the defineProperty logic.

Object.prototype.writable = Object.prototype.enumerable = Object.prototype.configurable = true;

Object.defineProperty(this, "notASecret", {value:"writable"});

for (var key in this) if(key === notASecret) alert("WTF"); // will show it

var descriptor = Object.getOwnPropertyDescriptor(this, "notASecret"); alert(descriptor.writable + descriptor.enumerable + descriptor.configurable); // is 3, should be zero

br

# Brendan Eich (12 years ago)

Andrea Giammarchi wrote:

I really don't understand what is exactly the advantage of considering inherited properties for descriptors.

It's easy to overreact here. Don't mess with Object.prototype still applies, or you will have other WTF moments.

ES5 shipped this and browsers shipped implementations. David and others use it. I don't think we can change it.

The ES5 meta-object APIs are pretty much power-tools, with obvious verbosity and usability taxes. Better (more concise and usable) veneer on top has been done and should bubble up on github. This is not to excuse anything, but any process (standardized de-jure or not) that releases platform iterations to millions of developers will face this pain. Node.js is facing it now.

So I say doc this, evangelize it, and build better APIs on top of ES5. TC39 people are tracking.

# Andrea Giammarchi (12 years ago)

I will ... but I still would like to know why other developers are using inheritance for this.

No memory, performance, size, benefits and more verbose code, i.e.

Object.defineProperty(o, p, Object.create(null, {value:{value:v}}))

OR

Object.defineProperty(o, p, new DefaultDescriptor(v));

VS

Object.defineProperty(o, p, {value:v})

So, let's say I am starting evangelizing this here.

br

# Brandon Benvie (12 years ago)

Here's a place I've made use of it, note Descriptor and Descriptors. Benvie/benvie.github.com/blob/master/client/modules/site/introspect.js#L221

# Andrea Giammarchi (12 years ago)

that's actually cool, but you suffer same problem? :-)

desc.enumerable = 'get' in desc;

# Andrea Giammarchi (12 years ago)

Updated MDN: developer.mozilla.org/en-US/docs/JavaScript/Reference/Global_Objects/Object/defineProperty#Description

Hope it's fine, feel fere to edit/add/change that,

# Herby Vojčík (12 years ago)

Maybe some little kind of protection would be helpful.

One of minimal solution I see is to have a well-known object that holds all of the descriptor defaults (Reflect.defaultPropertyDescriptor), and:

  • when returning descriptor from API, make it __proto__:R.dPP
  • when creating descriptor, evangelize users to create something like function pd(o) { o.__proto__=R.pdd; return o; } and use it for creating either plain descriptor inline, or roots for reuse in their own Object.create scenarios; or - you can even have it in Reflect as function on its own (safePropertyDescriptor), just people won't use it when it have long name so at least they should be evangelized to do let dp=Reflect.safePropertyDescriptor; and use pd(...).