tc39 / proposal-class-fields

Orthogonally-informed combination of public and private fields proposals
https://arai-a.github.io/ecma262-compare/?pr=1668
1.72k stars 113 forks source link

[Private] Metaprogramming vs Security #134

Closed Igmat closed 6 years ago

Igmat commented 6 years ago

I was kinda late to this discussion since it's already Stage 3, but still there is a hope that some decisions could be revisited. I've carefully read FAQ and some related discussion, like #100, #106, #122 and others plus a lot of links, including other syntax proposals, that were mentioned there.

Probably I missed something (there are TOO many discussions and ideas and it's non-trivial task to cover all of them, if it isn't part of your day-to-day work), but it seems that I've finally found core misunderstood between committee and community. From my point of view broken proxies (#106), when target uses #-privates and proxy author (probably some framework/library maintainer) can't control full class and instance lifecycle is a HUGE limitation to how we can apply metaprogramming patterns to ES real-world applications and reason why this happens is security, which de-facto was a hard requirement for privates but wasn't clearly articulated to community. (if you're intersted I can try to find comments that lead me to such conclusion)

#-syntax proposal advantages

  1. Security : a. I (as code author) can safely pass an object built with my class to untrusted execution context, knowing that there is no way to read or even examine its private properties b. Nobody can apply my methods to something that isn't instance of my class if those methods use privates (somewhere it was mentioned as brand-check)
  2. No violation of invariants defined with privates
  3. Strict public API, controlled by code author without worrying that somebody will start depending on implementation details that are private

private-keyword syntax disadvantages


All of the above is good motivation (assuming that security is more important) to choose #-syntax, even though it leads to HUGE limitation of metaprogramming using proxies (#106).

Initially my motivation for writing this post was to find out the reason of such misunderstood between community (I believe I'm a part of community too) and committee.
While thinking about it I also tried to find simplest solution for problem described in #106, because it relates to HUGE amount of developers directly (Evan (@yyx990803), Rob (@EisenbergEffect) as maintainers of popular projects that want to use proxies for implementing proper reactivity and me as person who experiments with metaprogramming capabilities for kinda same purposes) or indirectly (developers who want/need frameworks/libraries/tooling that provides such higher-order functionality).

Reading issues in this repo I've found out that security pattern based on WeakMap was taken for implementing private proposal. But what if we'll take another approach, based on Symbols and closures.

Syntax

class A {
    // this actually not property by itself, but rather
    // property name based on Symbol and lexical scope of 
    // class definition
    private somePrivate;
    // declaring public and private property with same name 
    // is syntaxError, like in majority of other languages
    somePrivate;
    // but computed property could have same name, which also
    // grants symmetry for declaring and accessing properties
    // with different access levels
    ['somePrivate'];

    method(otherInstanceOfA) {
        // to avoid surprising of developers and silent bugs,
        // most intuitive syntax should just work
        this.somePrivate; // resolved to private property
        // since `somePrivate` actually is variable that exists
        // in scope of class it could be used in such way
        this[somePrivate]; // resolved to private property
        // this is the only way to access public property with same name
        // inside class definition - and it's ok, since any other language
        // with similar concept just restrict creating public and privates
        // with same name inside of one definition
        this['somePrivate'] // resolved to public property

        // we have to add small asymmetry for accessing privates of other object
        // and it is normal, because otherInstance could be not an instance
        // of this class. And even more accessing privates of other instances
        // isn't something common (but obviously has use cases)
        otherInstanceOfA.somePrivate; // resolved to public property
        // so previous access, has least surprising behavior
        // and work as expected in most cases - you can't have access to 
        // privates from outside, only following syntax shows that developer
        // tries to do it from inside
        otherInstanceOfA[somePrivate]; // resolved to private property
        // and this resolved as usual
        otherInstanceOfA['somePrivate']; // resolved to public property
    }
}
const a = new A();
a.somePrivate; // resolved to public property
a['somePrivate']; // resolved to public property
a[somePrivate]; // ReferenceError: somePrivate is not defined
class B extends A {
    // all this will work same way as described above, without causing
    // any kind of errors, because inherited methods will use
    // somePrivate Symbol defined in class A, while new methods of class B
    // will use somePrivate Symbol declared here
    private somePrivate;
    somePrivate;
    ['somePrivate'];
}

Such kind of syntax doesn't have disadvantages mentioned above, because based on existing property lookup mechanism and existing closures securing.

But it doesn't have advantages of #-sigil based syntax, since usual Symbol could be easily found by getOwnPropertySymbols and/or Proxy.
In order to have same security - it should be another type of Symbol. I've started to collect requirements to such kind of Symbol, but then I've found proposals that actually did it already (like this one https://github.com/zenparsing/proposal-private-symbols, I believe that @zenparsing will describe it better than I can).

Advantages

PLEASE

Let's try to have constructive discussion, because as we can see (for example in #133) there are a lot of developers that wasn't convinced by committee's arguments.
I, personally, did my best trying to find reasons behind existing proposal, and I hope that committee will do the same for community, especially after @littledan meet some of open-source maintainers in person.


P.S.

@ljharb, we all know that you hate protected and don't like OOP paradigm in general, so please don't try to move this topic into discussing whether or not we need it in ES.

ljharb commented 6 years ago

If you're going to use protected as an advantage in this thread, then it's perfectly reasonable to debate its merits here. If you want to remove that from the list of advantages, then we don't have to discuss it here ¯\_(ツ)_/¯

As for "don't like OOP paradigm in general", that's both untrue and irrelevant.

rdking commented 6 years ago

@Igmat You've made a few mistakes that may alter what you're trying to achieve here.

private-keyword syntax disadvantages Performance: if declaration will look like private somePrivate; and access - this.somePrivate, then property lookup will become VERY difficult, untrivial and probably unsecure

True but irrelevant. Everyone I know of in these debates has accepted that this.x access is for public use only. There's no chance at all that any accepted form of private will not contain an extra character in its access notation.

OR Assymetry: if declaration will look like private somePrivate; and access this.#somePrivate or private(this).somePrivate or whatever that not just this.somePrivate it will lead to one of the following: a. Silent bugs: (example omitted)

Sadly true, but also irrelevant. The fact that developers are accustom to the obj.x notation style means that there will be instances (increasing with complexity of code) where the "extra character" to access private fields will be forgotten, thus causing hard to find bugs. Symmetry may help to reduce this somewhat but has significant other costs. Regardless of symmetry, the risk of this is greatly reduced as developers become accustom to the implemented syntax.

b. Security breach: if example above throws SyntaxError, anybody outside could examine object's privates at least for their names, since const a = new A(); a.somePrivate; outside class definition is the same as this.somePrivate somewhere in class definition.

Patently false. None of the standing counter-proposals allow a private field to be publicly accessed.

Igmat commented 6 years ago

@ljharb, it was offtopic from your side, but since I mentioned you directly, I'll answer - and I hope, you'll continue in more proper way for technical discussion.


It's only one of several advantages and even more - it's only a small part of one of several advantages (there could be other access levels, that we'll want/need).

Since I read dozens comments from you and found nothing constructive about protected, only your own opinion (probably, based on something like I don't use and don't need it) - you even don't provide any arguments while responding to big posts with argumentation for adding protected.

I believe you're great professional, and obviously you're part of the committee (while I'm - not), but it doesn't make your own opinion more valuable than mine or anybody's else, if you don't provide proper argumentation. You don't own this language to behave like that.

Igmat commented 6 years ago

True but irrelevant. Everyone I know of in these debates has accepted that this.x access is for public use only. There's no chance at all that any accepted form of private will not contain an extra character in its access notation.

I'm not the only one who hasn't accepted it - there are plenty of developers who would prefer this.x to point to private x in lexical scope of class if x was declared as private, and a.x to point to public property x of instance a whether or not class of a has private x. Everybody here should understand that there are no chance to have big enough selection of developers in this github repo to judge about what is preferable for majority.

While I could easily accept this.#x access and #x declaration IF they won't break proxies (and MAIN reason why I wrote this proposal is resolving #106), Symbol-based approach doesn't need # for working properly, so why we have to use it?

Sadly true, but also irrelevant. The fact that developers are accustom to the obj.x notation style means that there will be instances (increasing with complexity of code) where the "extra character" to access private fields will be forgotten, thus causing hard to find bugs. Symmetry may help to reduce this somewhat but has significant other costs. Regardless of symmetry, the risk of this is greatly reduced as developers become accustom to the implemented syntax.

This looks like paraphrasing my argument against keyword-based syntax, or I missed something?

Patently false. None of the standing counter-proposals allow a private field to be publicly accessed.

Obviously, probably I didn't explain it clear enough. I didn't meant that somebody proposed a.somePrivate be accessible publicly, but rather that their proposals wasn't aware about all edge-cases protecting private values.

rdking commented 6 years ago

@Igmat Ok. I get that there are those who still want to see private x and this.x notation. It can even be done, but once again, there are trade-offs. To get that notation you'd need to

So I get it, but the TC39 board has defined what they want on this very clearly, and that's both inaccessibility and undetectability. I think their overdoing it, but they won't budge on this issue. So it's fair to say that no proposal including detectability of private fields will pass.

Symbol-based approach doesn't need # for working properly, so why we have to use it?

The problem with the symbol-based approach is that if the symbol is leaked, so is access. Even the possibility of leaking access is a big NO for any private field proposal.

This looks like paraphrasing my argument against keyword-based syntax, or I missed something?

You missed the last line of that paragraph. What I'm saying is that the (#x, obj.#x) declaration/access pair have the same issue. The symmetry of the notation does little if nothing to prevent the developer from making that kind of mistake.

Obviously, probably I didn't explain it clear enough. I didn't meant that somebody proposed a.somePrivate be accessible publicly, but rather that their proposals wasn't aware about all edge-cases protecting private values.

Wherein this proposal restricts accessibility, it's pretty thorough. I won't say that all edge cases have been found, but I have yet to find one that hasn't been handled. I'm sure @bakkot, @littledan, & @ljharb would all appreciate an example of an edge case you think they missed if you have one.

Igmat commented 6 years ago

@rdking probably I wasn't clear enough, but my proposal requires another kind of symbols (like in this proposal), that leads to somewhat other conclusions.

I won't repeat whole proposal about Symbol.private but rather list few important requirements to it for being base for this proposal:

  1. Such kind of Symbol isn't listed in any kind of reflection (e.g. getOwnPropertySymbols, getOwnPropertyDescriptors, Object.keys or any other)
  2. Such kind of Symbol applied directly to target of Proxy instead of reaching any of Proxy traps.

So this:

give up on wanting "undetectability" for private fields

is irrelevant. Could you please clarify why my proposal lead to this:

seal object instances after construction

because for now it also seems to be irrelevant.


The problem with the symbol-based approach is that if the symbol is leaked, so is access. Even the possibility of leaking access is a big NO for any private field proposal.

The only way how private symbol could leak in my proposal is following:

class A {
    private somePrivate;
    method() {
        return somePrivate;
    }
}

If that is a problem (which I'm not sure yet), it could be restricted, causing Syntax or Reference error, unless method is private too:

class A {
    private somePrivate;
    private method() {
        return somePrivate;
    }
}

You missed the last line of that paragraph. What I'm saying is that the (#x, obj.#x) declaration/access pair have the same issue. The symmetry of the notation does little if nothing to prevent the developer from making that kind of mistake.

Could you please clarify which kind of mistake will developer make with syntax proposed in this topic?


To be honest, I'm pretty satisfied by soft private provided for example with TypeScript. There are few issues I faced with it and solved with usual Symbol or Object.defineProperty. From my perspective it's the thing majority of developers expect from private (just take a look at number of TS repos which use its private and number of repos that use WeakMap to get hard private, spoiler: first is MUCH bigger). But I'm not going to argue about priorities of committee and reasons why they put Security higher than Metaprogramming. I really appreciate the number of efforts put into making really secure private possible and I'm not trying to go back from that point.

Difference in terms of Security between my proposal and existing in this repo as Stage3 is on simple trade-off:

  1. #-based proposal in current state leads to that we could examine some class A for using privates in its implementation, by creating mostly transparent proxy on top of instance of class A and calling methods on that proxy - any method that use privates will throw.
  2. My symbol-based proposal doesn't have previous issue, but instead dismiss using privates for brand-check.

IMO it's reasonable trade-off, because implementing my proposal keeps EXISTING option for breand-check (using WeakSet or WeakMap), while #-based proposal ruins EXISTING way of metaprogramming using proxies (which seems to be highly underestimated by committee).

Igmat commented 6 years ago

To prove my point just try to google brand checking in javascript and js metaprogramming with proxy.

Igmat commented 6 years ago

And one more. I realized that I've seen here a lot of quotes like this "let's use decorators for other kinds of access levels". And while it's seems to be a very tricky to create decorators for this purpose, I already know how to create a simple decorator for brand-checking - it will took few hours only.

From my point of view it seems to be a strong argument against adding unnecessary feature directly into language, which also breaks something that is heavily required by community.

rdking commented 6 years ago

@Igmat you might be interested in #115. To answer your questions, Symbol.private doesn't follow the private x -> this.x notation pattern. The arguments I made regarding sealing the object were only with respect to that, not using Symbol.private.

The only way how private symbol could leak in my proposal is following: (example omitted)

Not true at all. The private symbol could be:

There's probably other ways as well, but this should be enough to show how easily they can be leaked.

If that is a problem (which I'm not sure yet), it could be restricted, causing Syntax or Reference error, unless method is private too:

That would be counter-productive and unnecessarily increase the amount of code required. The better approach would be to require that only member functions present in the class declaration are allowed to use the private symbols. But as soon as you do that, you've got PrivateNames, which are already an implementation detail of the existing proposal.

Right now, in #133, I'm discussing with @bakkot an alternate approach to this proposal that resolves the proxy issue without significantly altering much else about the proposal. We haven't reached that point in the discussion yet since he seems to have a severe hang-up regarding the possibility of developers accidentally generating a public field when they meant to access a private one. The suggestions I'm offering show neither of the "trade-offs" you've mentioned without giving up anything else.

Igmat commented 6 years ago

To answer your questions, Symbol.private doesn't follow the private x -> this.x notation pattern. The arguments I made regarding sealing the object were only with respect to that, not using Symbol.private.

My proposal is syntax sugar for using Symbol.private with further usage of private x -> this.x notation pattern, so there is no need in sealing the object.

Not true at all. The private symbol could be:

Yeap, definitely, I meant that leaking of private symbols could happen ONLY if code author will explicitly do it. Risk that it will happen unintentionally is nearly the same as risk of unintentional returning of private value.

littledan commented 6 years ago

In the most recent TC39 meeting, we considered an alternative proposal, Private Symbols, which have a somewhat weaker sense of undetectability. The committee decided that a very strong sense of undetectability is an important feature of private fields. At this point, implementations are moving forward with the private fields proposal in this repository; see the README for details.

Igmat commented 6 years ago

@littledan I've already checked data about latest meeting, you had wrong assumptions in your presentation, while I haven't seen what was discussed as alternative.

littledan commented 6 years ago

@lgmat What assumptions were wrong?

Igmat commented 6 years ago

@littledan Wrong assumptions:

  1. Main concern is syntax
  2. Strong boundaries requires brand-checking
  3. Proxy issue could be handled with decorators

Personally I don't like #, but I'll definitely will get used to it - while the main concern is WeakMap semantic that mixes 2 separate features (encapsulation and brand-checking) in order to support some invariants that almost never appear in actual code and could be easily supported by 20-lines decorator.

littledan commented 6 years ago
  1. I understand that some people don't want strong encapsulation boundaries; is that what you're referring to?
  2. I agree with you if you're saying that brand checking shouldn't be a core concern of the private fields proposal. I see the exceptions thrown as more of a convenience for programmers, to avoid mistakes using private fields that don't exist. (I can't stop other people from developing their own philosophies about the same language semantics.)
  3. To investigate the Proxy issue, we had a meeting with the framework authors who raised the concern, and we concluded, on further thought, that they will be able to make the current semantics work for them, though the situation is not optimal for them. (They didn't want anything like the capability to read the private fields of their users' classes.) See https://github.com/tc39/proposal-class-fields/issues/106#issuecomment-421556491 for more details of the discussion.
Igmat commented 6 years ago

@littledan, sorry. I'm obviously not a native speaker for English, so I'm curious is the way I'm expressing my concerns and issues about this proposal unclear? I'm asking, because your (committee's) answers sometimes sound like talking with a baby - and it confuses me.

  1. No, my point is that you shouldn't focus on syntax trying to gather feedback - it's obvious that privates even with bad syntax is better than no privates at all. So when you try to find out how community reacts to this proposal by providing them with two alternatives #x or private x you're creating false dillema, because question isn't in syntax. NOTE: I even won't talk about that biased presentation also affects answers
  2. Great, you agreed on that. But this proposal ALREADY INCLUDES implicit brand-checking for every private property. By forcing such brand-checking you DON'T PREVENT any type of common mistakes you were talking about, but rather create following problem!
  3. I know what is proxy issue very well and I've read that topic very carefully. I'm also working on something like reactive properties for my own framework and there is no other way except proxies to implement it. But if this proposal lands, only thing that I'll be able to workaround it is putting following not to my readme file.
    NOTE: this library doesn't work with private fields.
    Actually it doesn't need access to them, but thanks to the fact, 
    that privates implicitly adds brand-checking, proxies, that heavily
    used in this library, won't work properly.
    Oh, you don't know what is `brand-checking`
    and how it relates to private and proxies, here some links:
    https://github.com/tc39/proposal-class-fields/issues/106
    https://github.com/tc39/proposal-class-fields/issues/134

The only reason (BTW it's unexpressed) to continue with existing proposal is the fact that it's already implemented! You're trying save time of committee/V8 team/babel team/any other implementers, by DESTROYING EXISTING METAPROGRAMMING CAPABILITIES for everybody else. Is it reasonable trade-off???

Everybody will use private for encapsulation and everybody will suffer from the fact that they have to choose between metaprogramming proxies or ecapsulated (and implicitly brand-checked) implementation details.

hax commented 6 years ago

@littledan

Main concern is syntax

I guess there may be some misunderstandings in TC39 members.

# syntax is a issue, and surely many programmers disagree this proposal ONLY because of that.

But there are still many other reasons why many experienced js programmers disagree this proposal. You can't say the main concern of them is syntax just because they also dislike #. And more complex, some concerns are related to implication of semantics and the mental model of this.#priv syntax, with the consequences in real world engineering. It's theoretically impossible to separate the syntax/semantic in such issues.


And even the disagreements purely to # are not ignorable. Assume all js programmers will eventually accommodate it is unrealistically optimistic and under rated the risk of the break of the community.

It's also add the distrust between the community and the committee which I think may have a negative far-reaching influence to all of us. And it's already begun.

hax commented 6 years ago

@rdking

There's no chance at all that any accepted form of private will not contain an extra character in its access notation.

There is a chance --- classes 1.1 with the shorthand extension. I don't want to say it's a perfect solution now but I believe there is a possibility we could investigate and find a much better and acceptable syntax in this path.

rdking commented 6 years ago

@hax The problem is that TC39 is holding fast to their "undetectability of private fields" requirement. The only way to accomplish this is to let the syntax for accessing a private field be distinct from the syntax for accessing a public field. Unless someone can convince them that this requirement is pointless, my statement stands. I've pointed out the logical contradiction of this requirement, but that contradiction has been ignored. So good luck.

hax commented 6 years ago

@rdking I know the requirement of "undetectability", and shorthand syntax of class 1.1 satisfy this with this.x vs x (the shorthand of this->x). Though the full form this->x require a distinct notation, the shorthand form x just drop all extra notations. And in 90% cases, you just need shorthand form.

littledan commented 6 years ago

My big concern about -> is that it's too annoying to confuse with .. It's no fun to differentiate this in C++. I like how # is conceptually part of the name; I think it will be harder to confuse.

At a higher level, the objections to this proposal have been going in several different directions. For example, if we did a longer investigation into 1.1, we wouldn't get any closer to satisfying people who want more metaprogramming mechanisms. I think we have a strong reason for all the decisions in the current proposal.

hax commented 6 years ago

@littledan

My big concern about -> is that it's too annoying to confuse with .. It's no fun to differentiate this in C++.

There are alternatives of ->. Personally I prefer ::. Anyway, this is just a bikeshed issue.

And I even have some other idea which can eliminate any possible confusion between . and ->/:: or whatever, and solve some potential issues in shorthand syntax. But I don't want to discuss it before TC39 resolve the process failure problem I pointed out. The further discussion can be meaningful only after classes 1.1 or some similar proposal is revived in TC39 process. I guess this is why @allenwb , @zenparsing and Brendan Eich stopped the work on classes 1.1. Because in current situation, any possible improvement of alternative proposals are just ignored and can not get any "consensus in the room", so it's just waste time of both side.

I like how # is conceptually part of the name; I think it will be harder to confuse.

Unfortunately # being part of the name pay the cost that it share . with property access notation. I and many already discuss the problems of this.#foo vs this.foo many times which I think you should already read. I don't see any rational answer. So I assume you think such critique make sense. If that, it will be illogically you worry about confusion about . vs -> but think . and .# will be harder to confuse. I don't understand.

rdking commented 6 years ago

@hax You're not alone in thinking that dismissive opinion has been unevenly applied. This is a proposal I've been working on that includes the best features of the current proposal while removing many of the major issues like Proxy failure, this.#x !== this['#x'], and the future expansion issues. It does a lot more than that, but the extra stuff could easily be pulled apart in to separate proposals. I don't see any benefit in doing so, especially given that the current proposal is being delivered along with decorators and possibly static private. I just presented an all-in-one, decomposable solution.

@littledan Even if it will never see the light of day, I would still like to see the proposal I've offered get at least as much coverage as the 1.1 proposal. While it's true that the main visible difference is the syntax, as has been stated many times before, the syntax reflects what is actually going on under the hood. So changes to the underlying mechanism will necessitate changes to the syntax.

hax commented 6 years ago

@rdking Yeah, we share many opinions about current proposal 😄 , though we may still have different views on what is a better proposal should be.

For example, your #. notation in this#.x seems just a bad alternative of -> in classes 1.1 proposal. I understand it also allow this#['x'] to work, and make # like a postfix operator, but I will argue this#['x'] is not very useful and postfix operator is still stranger in JS (yes there are postfix ++/-- but programmers rarely combine them to other operators, so no luck for familiarity of #.), and, still unfortunate # 😂 . To be honest, I guess the syntax is as bad as current proposal in average programmers view.

Anyway, in this issue list, I feel your comments are always enlightening. So I hope I can hear your comments on classes 1.1 . What is your biggest concern on it? You can create issue on https://github.com/zenparsing/js-classes-1.1/issues and @ me.Thank you!

rdking commented 6 years ago

@hax I have never claimed that my proposal is better than the 1.1 proposal (that I can recall). My only claim is that my proposal is in keeping with almost everything that the TC39 has agreed upon with regards to the current proposal without suffering any of its unnecessary drawbacks. The 1.1 proposal, while not bad, would reset the discussions back to the beginning again. That's not a bad thing either, but I don't think the board is willing to do that.

-> notation is a non-starter in my book simply because of the ease of mistyping (=>) and still ending up with viable code that actually works but ends up doing something that's not even semantically parallel. With the 1.1 proposal, I'd rather use :: for the operator. Truth be told, I just want to use private x/this.x and be done with it. However, the board seems fairly tied to the opinion that private fields must be completely undetectable. If that wasn't the case, it might be possible to get that simple notation. However, that's probably like asking rainwater to fall on the sun.

With my proposal, this#['x'] is only marginally less useful than this['x'] because it cannot be used to dynamically create a new private. Beyond that, all the same use cases apply between them. I also understand that postfix is not an oft used operator style in ES. However, it does exist. So what I've suggested is in no way inconsistent with ES. It would be akin to this kind of logic: obj.getPrivate().field where .getPrivate() is reduced to just #.

Like I've stated before, the goal with my proposal is to make minimal changes to the current proposal so that we don't have to start all over from the beginning and lose the benefits of the many discussions that lead to the current proposal. Like the 1.1 proposal I want a complete implementation of class so that no future modifications need to be made to it. But at the same time, I want to preserve the existing functionality of the language and keep the general understanding that use of a particular keyword doesn't lock you into a particular usage pattern. This is why my proposal applies access levels to all objects and not just class.

I'll look through 1.1 and throw some comments on that. But for now, I'm of the impression that the reason we're all coming up with such disparate proposals is because we're trying to do some that doesn't yet make sense in the context of a class: add private property/field definitions. The problem here is that class doesn't even properly support public field definitions yet.