PrivateName design: null-proto with methods vs. object-with-functions

[domenic]

The private name stuff has evolved a lot recently, but I think I'm caught up. The latest is that you create an object with:

• An internal slot
• A null prototype
• A toStringTag
• get/set functions that look at an internal slot, but are shared among all private names (so e.g. privateName.get.call(otherPrivateName) will work).

This seems like a strange design that's halfway between a class and an object.

Have you considered instead just using an object that contains two closures? That is, a normal ordinary object, with two functions that each themselves have an internal slot pointing to the private name.

The main user-observable differences would be:

• Normal Object.prototype (like other "here's some stuff" objects in the spec, notably iter-result objects or property descriptors)
• No toStringTag (same)
• Separate "bound" get/set functions per object (so privateName.get.call(otherPrivateName) will be equivalent to privateName.get(); this is ignored, as they are functions, not methods)

This seems like a much more normal design to me.

[nicolo-ribaudo]

Normal Object.prototype (like other "here's some stuff" objects in the spec, notably iter-result objects or property descriptors)

The problem with this is that well-known symbols (e.g. Symbol.hasInstance) can be overwritten on Object.prototype to steal private names.

No toStringTag (same)

Having toStringTag makes private name objects consistent with the other object created for decorators (e.g. descriptors). I don't think that it is strictly needed thought, since it can't be used as a reliable brand-checking mechanism.

Separate "bound" get/set functions per object (so privateName.get.call(otherPrivateName) will be equivalent to privateName.get(); this is ignored, as they are functions, not methods)

This feels a little bit strange to me, since every native JavaScript function can be used on a different this value using call/apply. Also, if they are shared between all the private name objects they can be used to check if an object is actually a private name in a secure and reliable way.

[jridgewell]

Agree with @nicolo-ribaudo on his first two points.

Separate "bound" get/set functions per object (so privateName.get.call(otherPrivateName) will be equivalent to privateName.get(); this is ignored, as they are functions, not methods)

I'm neutral on the idea, but wouldn't this increase memory usage? I can't think of another constructor/factory that pre-binds the methods.

[domenic]

The problem with this is that well-known symbols (e.g. Symbol.hasInstance) can be overwritten on Object.prototype to steal private names.

Can you give an example?

Having toStringTag makes private name objects consistent with the other object created for decorators (e.g. descriptors).

I am supremely surprised that decorator-descriptors have a toStringTag, given that property descriptors do not.

This feels a little bit strange to me, since every native JavaScript function can be used on a different this value using call/apply.

People seem to have been misled by my use of the word "bound". There's no binding involved. What I meant was closures. E.g.

let x = 5;
function get() {
  return x;
}
function set() {
  x = 5;
}

These functions cannot be used on a different this value using call/apply, because they don't reference this at all. Same for many existing functions in the spec, e.g. the promise resolving functions.

Also, if they are shared between all the private name objects they can be used to check if an object is actually a private name in a secure and reliable way.

It's unclear to me how that would be important when writing a decorator.

[jridgewell]

Can you give an example?

@rbuckton's comment, and my follow up.

Allowing PrivateName instances to inherit from monkey-patched builtins leaks the privacy.

People seem to have been misled by my use of the word "bound". There's no binding involved. What I meant was closures. E.g.

Isn't that effectively the same thing?

Same for many existing functions in the spec, e.g. the promise resolving functions

Which promise resolving functions?

[ljharb]

@jridgewell the ThenFinally and CatchFinally functions, for example.

[jridgewell]

Excellent, I see now what you mean by promise resolving functions. Doing a simple grep, we currently have the following that create closed over functions:

• https://tc39.github.io/ecma262/#await
• https://tc39.github.io/ecma262/#sec-makearggetter
• https://tc39.github.io/ecma262/#sec-makeargsetter
• https://tc39.github.io/ecma262/#sec-async-from-sync-iterator-continuation
• https://tc39.github.io/ecma262/#sec-asyncgeneratorresumenext
• https://tc39.github.io/ecma262/#sec-createresolvingfunctions
• https://tc39.github.io/ecma262/#sec-newpromisecapability
• https://tc39.github.io/ecma262/#sec-performpromiseall
• https://tc39.github.io/ecma262/#sec-promise.prototype.finally
• https://tc39.github.io/ecma262/#sec-proxy.revocable

Await, AsyncFromSyncIteratorContinuation, CreateResolvingFunctions, NewPromiseCapability, PerformPromiseAll, and Promise.prototype.finally are all necessarily using functions, though, because the are passed as parameters to other functions. There was never an object to associate with it.

MakeArgGetter and MakeArgSetter are super legacy loose mode arguments remappers. Let's ignore those...

Proxy.revocable is directly related. It does something similar:

Proxy.revokable = function(target, handler) {
  const proxy = new Proxy(target, handlers);
  function revoke() {
    proxy.[[ProxyTarget]] = null;
    proxy.[[ProxyHandler]] = null;
  }

  return { proxy, revoke };
}

So at least there's precedent. But it kinda feels weird...

[ljharb]

It’s basically just spec fiction to represent closures ¯\_(ツ)_/¯

[littledan]

There are a bunch of different issues here, which I'd like to treat separately (unfortunately they were conflated into a single patch).

Internal slot on the object

A PrivateName is more than just a way to get and set it--it can also be used as a key in a class element descriptor which a decorator returns. This is how you add new private fields to an object. The decorator system needs some way to get from the object that you pass in as a key to the underlying private name value. In the specification's logic, this is done in step 6 of ToElementDescriptor.

Do you have an idea for how we would get at the underlying private name value in the case of using bound get and set functions instead? It feels a bit hacky to me to reach into the closure to get out its internal slot--what if the get and set functions somehow differ in their internal slot? It seems cleaner to me to ignore those functions in finding the underlying private name.

Bound functions

If we do have an internal slot on the object, and if (ergonomically) it usually makes sense to call this as a method on the private name object, it seems like bound functions are sort of unnecessary. It doesn't hurt to save a couple allocations--I'd like to avoid introducing cases where private fields have much more runtime overhead than public fields (though this probably wouldn't be that much--just two closures at class definition time). If there's a good reason to make these bound functions in addition to having the internal slot in the private name object, I'd be fine to reconsider, but it seems more complicated than optimal.

toStringTag

Lots of decorators end up being interested in figuring out whether the key is a Private Name or not. One way to check this is typeof key === "object" (since the other possibilities are Symbol or String), but this has a couple downsides:

• It's kinda opaque what's going on
• If we ever introduce another kind of key in decorators, we might want it to be an object, and this logic would get confused

My understanding is that the "branding" mechanism that @domenic , @allenwb and others would prefer is not based on adding more things that query internal slots, but rather @@toStringTag and mechanisms like it, if we want to do these checks. In this case, we don't need a 100% reliable brand check; we just need an impressionistic, practical, quick way to check what's going on. @@toStringTag fits the bill.

All the same things are going on for decorator descriptors, and that's why these have a @@toStringTag value of "Descriptor": It's common to want to overload between things that handle decorator descriptors and other objects. It's acceptable if the mechanism is spoofable, but it should be relatively ergonomic.

Note, property descriptors and iteration results don't have @@toStringTag values, but I believe these are not as commonly used in contexts that need to overload between different types.

I'd be fine replacing the @@toStringTag usage with some other mechanism that achieves the same goals described above. However, relying solely on "duck typing" (e.g., checking for the presence of a whole bunch of properties) has been criticized as awkward and error-prone in discussions about this mechanism. (e.g., by @wycats).

EDIT: See https://github.com/tc39/proposal-decorators/issues/56 for earlier discussion of "branding" and decorator descriptors.

null prototype

@jridgewell gave good cross-references above. I'm not as strident as @jridgewell and @rbuckton on protecting against this sort of hazard as being completely critical--there's no way we could absolutely protect against all ways programmers could leak names accidentally, and it doesn't seem like an ocap issue.

However, it seems like a realistic, practical programming risk--you could accidentally leak things this way, and that could lead to broken privacy in practice. Given that it's pretty easy to defend against with a null prototype, I'd say, why not?

[domenic]

A PrivateName is more than just a way to get and set it--it can also be used as a key in a class element descriptor which a decorator returns.

I see. In that case, it seems like the design should lean more toward a class instance. So, the shared functions should go on a prototype, and not use a null prototype.

Given that it's pretty easy to defend against with a null prototype, I'd say, why not?

Because it's inconsistent with the rest of the language, for (as you point out) dubious gain. The pattern for method sharing is not this weird null-proto-with-triple-equals-functions-and-internal-slots, but classes, with prototypes, constructors, and methods on the prototypes. That's the pattern that should be used for PrivateName.

The above thread cites "module namespace exotic objects" as a precedent for PrivateName's weird design. But that's not a good precedent. First of all, PrivateNames are not exotic. Second, as has been explained previously, the design of module namespace exotic objects is specially tuned to support a single ability, dotted access to module imported bindings. What you have here is instead first-class domain objects in your problem domain, which are best represented by a class.

Right now this proposal seems to be attempting to smuggle in some kind of pseudo-defensible-objects framework, unprecedented in the language, and use it for one of your domain objects. That's a much larger discussion, that needs to happen with full committee participation. I anticipate this being a big topic if this proposal is going for stage 3 advancement.

[littledan]

OK, looking forward to committee discussion! I hope we can settle on one of these two possibilities. If you want to come to the decorator call as well, you'd be welcome; let me know and I'll send you an invite.

[pwhissell]

Timeout, I havent read all the threads linked to this issue but why is this a blocker for decorators and not https://github.com/tc39/proposal-class-fields sorry if the answer is obvious

[trotyl]

@pwhissell PrivateName was introduced by decorator and designed to interact with private fields (only used by decorator), it's not a built-in part of private fields.

Also, I guess you mean https://github.com/tc39/proposal-class-fields rather then https://github.com/tc39/proposal-private-methods.

[pwhissell]

@trotyl Ok thanks, but could the decorators exist without private fields therefore private fields not being a blocker for stage 3 decorators or would that break their implementation?

Also, I guess you mean https://github.com/tc39/proposal-class-fields rather then https://github.com/tc39/proposal-private-methods.

thanks, i edited my post!

[ljharb]

There would be compatibility concerns if decorators were introduced without any private field support, and then we tried to add it later.

[littledan]

We discussed this issue in the July 2018 TC39 meeting. Various people expressed interest in having some sort of integrity properties, but we didn't come to a strong conclusion that we should definitely have these properties, or what form they should take exactly.

On balance, some kind of integrity for PrivateName seems like a reasonable idea to me. Although reliable isolation in general requires additional work, it doesn't hurt to have some integrity here, and it seems like it would be nice in practice to have some of these issues already guarded against, given the practical difficulty of being "first" and installing the protection.

In the July 2018 TC39 meeting, I asked about whether this should be part of a general defensible classes proposal. I haven't heard back yet from anyone who's interested in championing defensible classes. If someone is interested in pushing that forward, I would love to discuss this all with you in detail.

Short of that, I'd suggest that we not hold back decorating private class elements for a general defensible classes mechanism. We have all the mechanisms we need in the ordinary JavaScript object toolbox to make PrivateName defensible, and decorators will any class declaration to opt into this sort of user-defined defensibility. Unless someone is working on a defensible classes proposal now, I'm thinking that we should do our best here to make PrivateName as defensible-style as we can, check this against the intuitions of committee members for what defensible classes might eventually be, and forge ahead.

@trotyl In addition to the compatibility concerns @ljharb mentions, we also expect decorators for private class elements to be pretty useful. In the July 2018 TC39 meeting, we discussed again whether it makes sense to have private class elements be decoratable, and I felt like we had renewed consensus for the current path of including this functionality. Note that this isn't the only issue we're still discussing for decorators--the ordering export vs decorators (#69) is still a hot topic for debate.

[erights]

Some initial thoughts on defensible classes.

I understand that the immediate purpose of wanting clarity on defensible classes is to get clarity on what PrivateName should be. But since this group are the right experts, I'd also like to verify that everything in the following sketch could be implemented by a user-written @def decorator. Aside from using a decorator, I think everything I'm about to suggest just reiterates the old proposal in the old wiki.ecmascript.org , but now is an opportune time to revive it for a number of reasons. Here goes:

  @def
  class Point {
    #x, #y;
    constructor(x, y) {
      #x = +x;
      #y = +y;
    }
    toString() {
      return `<${this.#x},${this.#y}>`;
    }
    add(other) {
      return new Point(this.#x + other.#x, this.#y + other.#y);
    }
  }

The @def decorator makes this class into a defensible class. The code above makes use of this defensibility to express a defensively consistent class simply and intuitively, where the class makes defensively consistent instances.

• The class/constructor is frozen. The prototype is frozen, all the methods, including getters and setters, are frozen. Presumably this can just be done by the decorator late in the initialization process.
• The class makes frozen instances. This is trickier because the constructor code does not express this; which would require that the class/constructor be replaced with a wrapper. However, this wrapper should not freeze the instance if called by super chaining from a subclass. Only when this constructor is invoked as the new.target should it freeze the instance before returning it.
• The binding of the variable name "Point" to this class is a const binding, i.e., it is not assignable. Is this already the case for classes in general? I don't remember.

With all of this locked down, multiple mutually suspicious adversaries can share both the Point class, and share some of the instances it made, without violating any of the following invariants. Many of these invariants will simply be true of classes in general, but it is interesting to write them down. None of these are true for vanilla ES6 classes that use public properties rather than private state.

• If p is a direct instance of Point and p.add(q) returns successfully, then the value of q must have been a direct or indirect instance of Point. IOW, if p were an instance of a subclass of Point, then the subclass could have overridden add, so we have no invariant for that case. But even if q is an instance of a subclass which could have overridden arbitrary methods, the subclass has no influence on the behavior of q that would be used during the above expression.
• If Reflect.apply(Point.prototype.call.apply(p, q)) succeeds, then both p and q must be direct or indirect instances of Point.
• The #x and #y state variables can only be initialized to numbers.
• If construction succeeds, both #x and #y will have been initialized to numbers.
• Neither #x nor #y can change once initialized. This has nothing to do with the instance being frozen and it not a consequence simply of the @def decorator. Rather, it follows from the visible lack of any assignment to these variables without the source text of the class.
• An individual point cannot vary its behavior over time.
• If two otherwise isolated subgraphs of objects share the same Point class, this does not enable them to communicate.
• If two otherwise isolated subgraphs of objects share the same direct instance of Point, then they can only communicate with each other according to the semantics explicitly stated in the source text of Point. In this case, because no mutable behavior is explicitly expressed, these subgraphs cannot communicate.

Attn @warner @tribble @jfparadis

[ljharb]

The class makes frozen instances

I'm not sure this is a capability that's needed by default for every use case. A nondefensible class could produce defensible instances, and a defensible class seems like it could produce nondefensible instances (forcing the newer to make it so) - can you elaborate?

[erights]

I'm trying to make the simple case simple and easy. The user can still express all these other combinations by other means.

A non-exotic non-frozen object whose non-frozenness is part of its intended semantics cannot be defensively consistent, since any of its clients can freeze it. A non-exotic object has no way to refuse to be frozen.

[erights]

All the smart contract code we're writing at Agoric freezes everything that could possibly matter. This significantly reduces the number of things we need to worry about.

[ljharb]

I'm not contesting that there's plenty of use cases that need a defensible class that produces a defensible instance - i'm suggesting that that might be too strong a requirement for the class itself - which is just the constructor function and its prototype - to be considered "defensible".

[littledan]

@erights Thanks so much for this prompt and thorough guidance. All of this makes sense to me. I am happy that we can accomplish all of this with the current object model and decorators.

Two things which were not obvious to me, but which sound totally reasonable, were:

• Freezing methods, so the entire class is deeply immutable (as long as it doesn't close over anything mutable, which is the case here)
• Only freezing the instance if new.target === PrivateName. This makes perfect sense to me--it is the only way we could make subclassing work. The subclass is responsible for freezing the final instance.

I share your intuition that the wrapping for instance freezing is unfortunate; I think we could consider addressing this in a follow-on proposal for a decorator to replace the construct behavior. For PrivateName, I'd say that we don't need to emulate this wrapping. What do you think?

[littledan]

The PR https://github.com/tc39/proposal-decorators/pull/134 is meant to convert PrivateName to a defensible class with the design documented by @erights above. Reviews are appreciated.

[pwhissell]

@littledan I know it sounds like I'm popping out of nowhere and I don't want to be the one slowing everyone down, but I'm not sure I understand how decorators absolutely need private members to exist. I know that having decorators on private members would be awesome and would probably be necessary in the future. However I fail to understand how having decorators implemented first would complicate the definition of private member semantics.

Say decorators already existed, would it not be possible to define/implement "privateness" of members afterwards?

[ljharb]

@pwhissell the list of class elements would suddenly change to include private members, so class decorators which depended on only public members being in the list could easily break.

[pwhissell]

@ljharb I don't understand how a decorator could be implemented to depend on only public members when private members don't yet exist. Say such an implementation exists and does some logic depending on the visibility of the element descriptor (private/public) and say the logic to determine whether or not this member is private or not is implemented in a method called "isPublic". How could a custom isPublic method's return value change as soon as private members start to exist?

@red
class Wine {

}

function red(classDescriptor) {
    classDescriptor.elements.forEach(function(elementDescriptor){
        if(isPublic(elementDescriptor)) {  //<-- how could this return value change when private members are implemented
            doSomething();
        }
    })

}

[ljharb]

@pwhissell the difference is the implementation of isPublic - it might not expect a key value that's not a string or a Symbol (let's say it has branching like "is typeof string, else" - instead of "is typeof string, else if typeof symbol")

[pwhissell]

@ljharb but the user has to make a change in his class to make some fields private once "privateness" exists. This user's decorators will always work untill he changes his class to make some members private in which case I believe he is also responsible for changing the isPublic method.

I just feel that any future keyword that describes class members would have the same effect, "static" or "final" for example.

[erights]

... a defensible class seems like it could produce nondefensible instances (forcing the newer to make it so) - can you elaborate?

... i'm suggesting that that might be too strong a requirement for the class itself - which is just the constructor function and its prototype - to be considered "defensible".

The word "defensible" is tricky. Let's start with "defensive" with the standard being defensive consistency:

... a defensively consistent object will never give incorrect service to well-behaved clients despite arbitrary behavior of its other clients.

The class and its instances represent some abstraction. For the class to be defensively consistent, the instances it produces should themselves be defensively consistent. If an instance gets corrupted, then the class has not provided good service.

The point of defensive consistency is it enables us to reason more about the abstraction and less about how the abstraction is implemented. That's why my example above is followed by a long list of invariants --- guaranteed properties that stay true despite fallible clients. Since a non-frozen non-exotic object cannot resist being frozen by its fallible clients, its non-frozenness cannot be part of its intended API surface.

[pwhissell]

@erights I'm so happy people like you are thinking about security subjects like these because the are important issues and the structure and implementation of visibility should definitely be part of a proposal. Even if how privateness is implemented can change how users implement their visibility dependant decorators, I'm still becoming more and more convinced that member visibility shouldn't be a blocker for decorators. What do you think?

[littledan]

@erights, @fudco, @warner and I had a conversation about PrivateName as a defensible class. @erights reiterated his reasoning from https://github.com/tc39/proposal-decorators/issues/129#issuecomment-409060485 , which explains why not just the prototype should be frozen but also instances and methods. We concluded that we should also give the methods null prototypes.

[ljharb]

um, i disagree with that last one - then they can’t be used with .call, .apply, or .bind, or .toString, etc, without borrowing methods. They’ll also lose toStringTag, and the name accessor.

[littledan]

This isn't an argument for or against the change, but the PrivateName constructor also has a null prototype, in tip of tree.

[erights]

This isn't an argument for or against the change, but the PrivateName constructor also has a null prototype, in tip of tree.

Just want to make clear here the conclusion I stated in #139 : That none of the [[Prototype]] links should be severed, including that of the constructor. Why was the constructor ever treated differently than the methods?

[littledan]

@erights Thanks for the summary and detailed look at this issue.

We used a null prototype to prevent cases that @jridgewell and @rbuckton raised, e.g., if someone does key instanceof PrivateName and Function.prototype[Symbol.hasInstance] has been defined, then the name would be sent to that method.

However, I have never been sure that it's necessary to defend against that particular case. We will need to do some education anyway--it's always possible to leak these names, e.g., by passing a name as an argument to another function; there are other ways to tell whether something is a PrivateName.

[erights]

The more general problem is that key instanceof Foo asks Foo about key and thereby gives Foo access to key. This cauterization addresses only the leakage of the case where you are testing the (would be) true case.

I do not suggest we do the following, but it is worth making the observation.

A less violent way to address this one case would be a special case of @ljharb 's suggestion at https://github.com/tc39/proposal-decorators/pull/139#issuecomment-419690322 : Define a frozen PrivateName[Symbol.hasInstance] method that explicitly does the default behavior, to override any monkey patch that would have been inherited from Function.prototype.

Rather, we need to make clear:

• In a non-frozen realm, nothing can be defensive in a broad sense, as shown by the pnset example at https://github.com/tc39/proposal-decorators/pull/139#issuecomment-419689317
• In a non-frozen realm, making PrivateName follow the defensible classes rules does result in this abstraction specifically achieving genuine defensiveness, but only narrowly.
• Cauterizing the constructor makes this defensiveness slightly broader, but only in a hard to explain way. It is not a technique we would apply to other defensible classes in non-frozen realms.
• Even for the best achievable cases, defensible classes in frozen realms, x instanceof y gives y access to x. We should have caught this hazard when Symbol.hasInstance was proposed. That was our only opportunity to avoid this hazard and we missed it.

[littledan]

Thanks @erights for this thorough analysis and practical suggestion for what we can do today. Let's review this idea in this coming week's decorators call, in addition to continuing to discuss it here in this thread.

[littledan]

@erights Quick question, is it a problem if these method/constructor objects are frozen and don't permit rewriting away the link to Function.prototype?

[littledan]

We've settled on using a prototype which is deeply frozen for PrivateName.