Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement meta-object trap(s) to make an object's [[Prototype]] immutable #538

Open
jswalden opened this issue Apr 12, 2016 · 19 comments
Open
Labels
proposal This is related to a specific proposal, and will be closed/merged when the proposal reaches stage 4.

Comments

@jswalden
Copy link
Contributor

This is followup on making the global object's [[Prototype]] chain immutable. It was agreed that a proposal to extend this to be a full proxy trap, so that ordinary objects and arrays and so on could have immutable [[Prototype]], would be entertained, and I'd write up the spec-proposal for it. Said spec-proposal isn't complete yet, but I have an approximate start on it thus far.

Right now the gist of the idea is to add a [[SetImmutablePrototype]] trap and (on most objects) a [[MutablePrototype]] Boolean internal slot indicating mutability (initially true). I have this completed, generally sensibly enough, except for a few issues still. I'll post my current, un-landable progress after filing this issue and figuring out how to attach that progress.

@domenic
Copy link
Member

domenic commented Apr 12, 2016

Yay!

on most objects) a [[MutablePrototype]] Boolean internal slot indicating mutability (initially true).

I think this woudl be better integrated into [[Extensible]]. So instead of being a boolean, [[Extensible]] becomes a tri-state "extensible", "immutable prototype", "non-extensible". It isn't possible to have an object that is non-existensible but with a mutable prototype, so a two-boolean design gives too many degrees of freedom.

@ljharb
Copy link
Member

ljharb commented Apr 12, 2016

In order to add a proxy trap, a corresponding Reflect method would need to be added. I suspect this would need to be part of a full proposal and maybe wouldn't be able to be just a PR.

@bterlson
Copy link
Member

Agree with @domenic, s/[[Extensible]]/[[Extensibility]] and tri-state seems good.

jswalden added a commit to jswalden/ecma262 that referenced this issue Apr 12, 2016
…aving to have a [[SetPrototypeOf]] trap enforcing such behavior. Fixes tc39#538.
@jswalden
Copy link
Contributor Author

https://github.com/jswalden/ecma262/tree/setimmutableprototype is the current quasi-state of what I have. It adds the extra [[MutablePrototype]] thing, rather than the [[Extensibility]] thing mooted already. The issue is that it only adds one trap, [[SetImmutablePrototype]], right now. But another is needed (or so it seems) to expose whether the [[Prototype]] is immutable. See the XXX bits in the branch above (and tell me how to attach its current state here, rather than literally just linking it). Does it seem like two additional traps are required here, or is there some other better approach for this?

@littledan
Copy link
Member

I'd like to revive this thread, given its connection with tc39/proposal-class-fields#179 .

Could we do this without a separate couple traps, and instead repurposing preventExtensions, passing an options bag indicating that we're just talking about the prototype? In the future, maybe this trap could also be used with a different flag for "deep freezing".

Imagine that it would look like this:

let obj = {}
Object.isExtensible(obj);  // true
Object.isExtensible(obj, { proto: true });  // true

Object.preventExtensions(obj, { proto: true });
obj.property = 1; // Works
obj.__proto__ = {};  // throws in strict mode
Object.isExtensible(obj);  // true
Object.isExtensible(obj, { proto: true });  // false

Object.preventExtensions(obj);
obj.property = 1; // throws in strict mode
obj.__proto__ = {};  // throws in strict mode
Object.isExtensible(obj);  // false
Object.isExtensible(obj, { proto: true });  // false

On the Proxy side, the options bag could be passed as a second argument to the preventExtensions and isExtensible traps.

When running code that expects to be able to use this options bag when the JavaScript engine or Proxy doesn't support it, the observed behavior would only be more conservative: In the preventExtensions call, it would do even more freezing, and in the isExtensible call, it would return non-extensible in fewer cases. For this reason, from an ocap security perspective, I hope it would be considered safe. (A deep-freeze flag would not be conservative in the same way, for example.)

I don't know how other engines work, but the way V8 implemented immutable prototype exotic objects already supports objects transitioning into an immutable prototype state (though maybe caching of the map should be implemented if we go this way); I don't think the Proxy or Reflect changes will be particularly taxing to implement. But of course we should think this through and be confident that this is a good design.

cc @erights @allenwb

@bakkot
Copy link
Contributor

bakkot commented Dec 17, 2018

I'm in favor of bringing this back too; thanks for starting the discussion, @littledan.

Could we do this without a separate couple traps

Out of curiosity, what's the motivation for this? I suspect we might be able to, but new traps seem easier to me, to spec correctly, to use correctly, and to feature-detect.

the observed behavior would only be more conservative: In the preventExtensions call, it would do even more freezing, and in the isExtensible call, it would return non-extensible in fewer cases

How so? It looks like it wouldn't do more freezing; rather it would do different freezing. Freezing the properties but not the prototype when you intended to freeze the prototype but not the properties seems undesirable.

@littledan
Copy link
Member

Out of curiosity, what's the motivation for this? I suspect we might be able to, but new traps seem easier to me, to spec correctly, to use correctly, and to feature-detect.

I'd be fine with separate traps as well; I just got the sense that it'd be annoying to add two extra Proxy traps. Maybe, when we bring this to the committee, we can give them the multiple options to bikeshed over as well. I don't have a strong opinion here. Agree with you about option bags not being as easily feature-testable, but I'm not sure that means we shouldn't add options.

How so? It looks like it wouldn't do more freezing; rather it would do different freezing. Freezing the properties but not the prototype when you intended to freeze the prototype but not the properties seems undesirable.

This would be "conservative" in the sense that, if you have an old browser which doesn't support the options bag, it would silently do more freezing. And, for objects which are just prototype-frozen, like Object.prototype, it would report that they are non-extensible.

@ljharb
Copy link
Member

ljharb commented Dec 17, 2018

It seems like a more robust approach is throwing when the desired functionality isn’t present - which separate methods/traps provides.

If we overloaded, I’d expect the safer fallback to be freeze, not preventExtensions, because then it freezes more than intended (instead of less).

@littledan
Copy link
Member

@ljharb Note that making the prototype immutable is even less strong than preventExtensions. No one has proposed a MOP operation for freeze in this thread.

@ljharb
Copy link
Member

ljharb commented Dec 17, 2018

That's very true - the only current operation that makes the prototype immutable is Object.freeze, so if we went with an options bag, I'd want the object to be guaranteed to make the prototype immutable, and make freezing the keys the backwards-compatible side effect.

@erights
Copy link

erights commented Dec 17, 2018

the only current operation that makes the prototype immutable is Object.freeze

Also Object.seal and Object.preventExtensions

@littledan
Copy link
Member

Right, preventExtensions is our MOP operation to freeze the prototype, and passing the options bag would do a strict subset of what that operation does.

@ljharb
Copy link
Member

ljharb commented Dec 17, 2018

ah, my mistake :-) i'd forgotten that all of them freeze the prototype as well; in which case preventExtensions would be the right place to add the option.

That leaves this opinion:

It seems like a more robust approach is throwing when the desired functionality isn’t present - which separate methods/traps provides.

@littledan
Copy link
Member

By throwing when it's not in place, you mean like Object.freezePrototype would throw as undefined is not a function?

@ljharb
Copy link
Member

ljharb commented Dec 17, 2018

Yup! That also (as mentioned earlier in the thread) makes feature detection and polyfilling much simpler.

@zenparsing
Copy link
Member

What's the use case for immutable-prototype-but-not-sealed?

@ljharb
Copy link
Member

ljharb commented Dec 17, 2018

@zenparsing for one, it would make Object.prototype no longer need to be exotic.

@littledan
Copy link
Member

littledan commented Dec 17, 2018

@zenparsing This could let a class always call its parent constructor, while continuing to let additional static methods be monkey-patched on, as discussed in tc39/proposal-class-fields#179

@ljharb
Copy link
Member

ljharb commented Apr 26, 2019

@ljharb ljharb added the proposal This is related to a specific proposal, and will be closed/merged when the proposal reaches stage 4. label Apr 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal This is related to a specific proposal, and will be closed/merged when the proposal reaches stage 4.
Projects
None yet
Development

No branches or pull requests

8 participants