Making operations on property descriptors more robust against Object.prototype hazards
Le 13 sept. 2014 à 11:47, Claude Pache <claude.pache at gmail.com> a écrit :
Hi,
As noted some time ago, there is an issue with
Object.definePropertywhen it happens that someone has the facetious idea to give some value toObject.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.
—Claude
[1] esdiscuss.org/topic/object-prototype-get-bye-bye-object-defineproperty
One can also (either alternatively or in addition) correct the other end of the API: (1) in proxy.[[GetOwnProperty]], ignore non-own properties of the result of the trap, and (2) in Reflect.defineProperty, ignore non-own properties of the object descriptor.
The core problem is that the two internal operations that convert between an internal representation of a property descriptor on the one side, and a concrete object representing a property descriptor on the other side, are not the inverse one of the other, because of the interference of Object.prototype.
Le 13 sept. 2014 à 14:19, Claude Pache <claude.pache at gmail.com> a écrit :
Le 13 sept. 2014 à 11:47, Claude Pache <claude.pache at gmail.com> a écrit :
Hi,
As noted some time ago, there is an issue with
Object.definePropertywhen it happens that someone has the facetious idea to give some value toObject.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.
—Claude
[1] esdiscuss.org/topic/object-prototype-get-bye-bye-object-defineproperty
One can also (either alternatively or in addition) correct the other end of the API: (1) in proxy.[[GetOwnProperty]], ignore non-own properties of the result of the trap, and (2) in Reflect.defineProperty, ignore non-own properties of the object descriptor.
The core problem is that the two internal operations that convert between an internal representation of a property descriptor on the one side, and a concrete object representing a property descriptor on the other side, are not the inverse one of the other, because of the interference of Object.prototype.
—Claude
Sorry for the spam, but I thought that a concrete example is good to illustrate the issue. Currently, under the assumption that someone will write Object.prototype.get = function() {}, the following proxy:
Proxy(target, {
defineProperty: (target, prop, desc) => Reflect.defineProperty(target, prop, desc)
, getOwnPropertyDescriptor: (target, prop) => Reflect.getOwnPropertyDescriptor(target, prop)
})
is broken, and the following code:
desc = Reflect.getOwnPropertyDescriptor(source, prop)
if (desc)
status = Reflect.defineProperty(target, prop, desc)
is broken as well.
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!).
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.
+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 ;).)
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
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
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.
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
Le 14 sept. 2014 à 01:58, Mark S. Miller <erights at google.com> a écrit :
+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.
Yes, and it would be nice to have more fine-grained methods than Object.preventExtensions. For example:
Object.forbidProperties(Object.prototype, ['value', 'writable', 'get', 'set', 'configurable', 'enumerable'])
Object.forbidNumericalProperties(Array.prototype)
This could be experimented with proxies... although it will be easy to circumvent the proxy by using Object.getPrototypeOf({}) instead of Object.prototype, unless we monkey-patch Object.getPrototypeOf, Object.prototype.__proto__, Reflect.getPrototypeOf...
―Claude
not a bad idea, here something to play with:
Object.forbidProperties = (function(descriptor){
function makeForbidden(property) {
Object.defineProperty(this, property, descriptor);
}
descriptor.get = function () {
return void 0;
};
descriptor.set = function (value) {
throw new Error('unable to set: ' + value);
};
descriptor.enumerable = false;
descriptor.configurable = false;
return function forbidProperties(obj, properties) {
properties.forEach(makeForbidden, obj);
return obj;
};
}(Object.create(null)));
makes this possible:
Object.forbidProperties(Object.prototype, ['value', 'writable', 'get',
'set', 'configurable', 'enumerable']);
although all those will show up through getOwnPropertyNames ... slightly
bummer
As noted some time ago, there is an issue with
Object.definePropertywhen it happens that someone has the facetious idea to give some value toObject.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.
—Claude
[1] esdiscuss.org/topic/object