JS Class fields potentially harmful

I have questions Spec Reading
41

Whether it’s good or not isn’t particularly relevant; it’s a largely unavoidable consequence of the ES3 (probably earlier) ability to return arbitrary objects from constructors, and the ES6 decision to preserve these semantics in class syntax.

42

let me try again, demoing leaks of private fields to a foreign object:

class Parent {
  constructor() {
    return {}; // foreign object
  }
}

class Sub extends Parent {
  #leak = 'secret';
  constructor() {
    // developer intent to bind the click method
    const {onclick} = Sub.prototype;
    // oooops, super() is not an instance of this class
    super().onclick = onclick.bind(this);
  }
  onclick() {
    // expose the leak in any undesired way
    console.log(this.#leak);
  }
}

(new Sub) instanceof Sub; // false
(new Sub) instanceof Parent; // false
(new Sub).onclick();
// logs the leaked private from Sub

does anyone believe this is a good part of the fields instantiated no-matter-what after the super call is acceptable by any OOP meaning?

we are in stage 4 ... nothing is written in stones ... can we see the current approach that mimic C++ and Java but not Python or PHP and the fact classes fields are defined after because of ES3 is problematic?

So far every argument against my proposal brought more issues to the plate to me ... C++ and Java developers likely won't even ever use classes in JS (or even think about returning something else is even allowed) while people coming from other scripting PLs will have to learn the hard way what they write in classes as fields is not always what they get/expect ... there's no control by any mean to how these fields behave (decorators can't prevent issues from happening) and there are leaks ... the resolution looks like: "meh"?

43

There’s no leak in that example - Sub is either intentionally exposing a private field’s value/presence, or isn’t, and Parent is merely deciding the this object for the Sub constructor (and choosing to extend something is ceding a certainty amount of control/trust to it implicitly).

44

We can ignore the issue as much as we like but the returned foreign object has nothing to do with the class and its prototype:

  • any method would fail
  • any accessor would fail
  • any brand check would fail
  • it gets potentially accidentally fields attached and from that point on, it can accidentally get methods that expose fields too (please don't focus on my explicit example, think about any smart library helper or even decorator out there that accidentally can leak everything to foreign objects!)

There's no equivalent in either C++ or Java or Python or PHP of this potentially accidental situation, which represents all issues that in the past needed to get fixed with way too much time in between releases as specification and fixes at distance, namely:

  • the undefined potential leak via window[shenaniganNobodyKnows] = value
  • the __proto__ field that cursed all platforms, all runtime, and there's a recent stage something proposal to fix that

In all that discussions (especially the latter one where I was the first opponent to __proto__) I've lived already what's happening here: denial, "too late", mentions around the fact code would break at this stage 4 time without any evidence, and so on ... so I am concerned: are we learning from the past that when an issue is raised at any stage version should be considered, instead of just diminishing it?

I am just trying to avoid amends and fixes with "use intended classes fields" directive in 5+ years (and all discussions, effort, and really too late work, or fixes in production, that would require) :person_shrugging:

45

maybe I should mention in my last 5 years I worked on JS that catches all these potential exploits that evil code will use in the wild as soon as a not-so-tested conditional return in any important constructor can get exploited ... and the "who cares" here is overly-concerning to me.

46

You have to go out of your way to make return override work at all, and at that point you're far from OOP anyway. Thus I don't consider stamping a private fields onto a foreign object a class encapsulation problem.

That said, I very much dislike the ability to stamp private fields onto random objects as it effectively creates a syntax-based undeniable WeakMap. For the same reason the DOM wants to exempt some of its objects from this, I want the ability to exempt some objects similarly. That said it's out of topic for this thread.

47

it's the only way to extend functions without needing CSP privileges due evaluation, details here, it's also the only way to extend DOM builtins without the registry, but it's also the easiest way to observe and provide observability by overriding a return constructor with a Proxy ... and I feel like I've just scratched the surface of reasons people might want to do that, which is more common than people in here might think (apparently).

not really, see any of my linked examples and think about returned Proxy for "magic instances behavior" inherited from super classes ... JS allows that, and it's demonstrated to work, faster than usual alternative to augment the instance too!

The evil is once again in the details: found an exploit to a conditional return that can be compromised via some Object.prototype overload in the evil wild code, or replacing Proxy from the globalThis or window many care to reach for globals, found a security breach for either front-end or back-end related cases ... I am specially thinking about databases abstractions and possible conditional intended overloads that might leak due intended implicit bound methods attached after, as well as any Custom Element that due polyfills or indirections might as well result compromised ... the fact any method accessing a private field can be bound to anything and expose such field if somehow hijacked through a parent constructor horrifies me ... which is why I am insisting the issue as a whole I've raised here spans from developers expectations to security concerns and "it's OK" or "it's too late" (on a stage proposal) are in my opinion not answers at all.

48

Class fields are not a proposal they have been standardised since ES2022.

1 Like
49

OK, if nothing can be done then, I rest my case ... I still wonder what's the point of having a stage 4 if nothing that raises, when people use stuff landed, can be ever changed :person_shrugging:

I read specs from ECMAScript standard, not out of stage 4 list, it'd be great to have this stage step more clarified ... landed on browsers can be experimental too, landed definitively and immutable makes it a standard, not a stage 4 to me.

50

Others who have been involved with the committee for longer may have a better description of stage 4. But I see it as the period of time when a proposal is "finished" and will be accepted into the latest draft spec but as EcmaScript gets a new version each year there is a delay before it is in an actual "version" of the language. The current stage 4 proposals for example are currently waiting for the ES2023 "cut".

51

so ... is this ES2023 already or can it be anyhow improved, after rising concerns? if not, I'll keep advocating around the topic helping developers avoid footguns ... if there's a tiny chance, I'll be happy to keep answering questions and provide more examples around issues.

52

The important part of whether something can change is less about what stage it is and more about whether people have started shipping code which depends on it. And people have absolutely started shipping code which depends on class fields, including details like this. It is extremely unlikely to change at this point even if everyone was convinced that it would be better if it had been otherwise (and at least some people, including myself, are not convinced).

53

Stage 3 is when implementation starts, and no major changes to the semantics are expected, but the proposal can and is updated based on implementer and user feedback. Arguably, stamping private fields on foreign objects could have been a change considered at stage 3, but the overall class fields proposal reached stage 4 almost 2 years ago, so it's definitely too late for such a change.

FWIW, that proposal was at stage 3 for a whole 10 years, and the specific return override behavior was definitively discussed plenty of times over the years. I wasn't around then, and I don't know if creating an undeniable WeakMap through pure syntax was something that was fully considered, but I've accepted that this behavior won't change wholesale, and I'll need to find and champion a more specific carveout for my concern.

I still do not sufficiently understand your concerns with this behavior to know if they were fully considered. Instead of requesting a full change of the return override and private fields behavior, I would suggest you try to identify a subset of cases where the behavior is problematic, and try to come up with a potential idea of how it could be mitigated.

One solution I've considered is some kind of API to "private seal" an object and prevent new private fields from being added. However, I do not want this to look like Object.seal where this ability can be applied on objects I did not create. One possibility may be to add a new isPrivateExtensible() trap on proxies, and thus restrict the stamping opt-out to proxy or other host exotic objects.

1 Like
54

Well there was no reason to preserve those semantics for private fields. I had always expected private fields to work exactly like ECMAScript internal slots, that are being implicitly passed in the super() call to allow allocation when the object is created.
Also putting private methods on each instance, not on the prototype, breaks established semantics as well.

1 Like
55

They do work exactly like internal slots - the object is created (by the super call) and then the slots are installed.

56

Uh, really? From my understanding, the slot list is compiled before, and passed as an argument to MakeBasicObject (often through OrdinaryObjectCreate).

ECMAScript® 2022 Language Specification says

[…] internal slots are allocated as part of the process of creating an object and may not be dynamically added to an object. […] the ECMAScript language provides no direct way to associate internal slots with an object.

Admittedly, in the omitted part of the quote it says "Unless explicitly specified otherwise", but I have not found any example of that. So my mental model always was that the set of internal slots of an object is immutable and known when the object is created. This is different from private fields.

1 Like
57

This is indeed what this thread is about, both the fact the created object behind the scene could have already attached fields when super happens and the fact semantics at class definitions are different only for fields and not retrieved from the class ... it's like an utility a part that is invisible and called without any way to prevent it around the super() ... as in __polluteRegardless__(super(...args)) ... if decorators could allow a way to prevent this or a way to handle fields from the class the issue and mental model could be fixed (on user-land, at least).

58

TypeScript 5 is beta and it's the first time that it adopts decorators as meant by specs (just to argue that having stages for years is not always meaningful for users) + I've pinged Nicolo' around Babel transformer keeping latest decorators proposal at Stage 2 level, and it was breaking at 3 or 4 so while I am sure people already use classes fields, I wonder if they are exploiting the current result on purpose or they just used fields for properties that never require an overload.

I find it extremely difficult to justify foreign objects being polluted with stuff that doesn't belong to them so I'd be very interested in learning when this is meant, needed, or desired, 'cause I cannot think about a single use case where that's the case ... I rather would expect surprises for non tested code.

Few developers (linked at the beginning of this thread) already got bitten and decided to move away from fields ... they use fields only for primitives and not for primitives that need overrides.

Can anyone provide a single use case where the current spec-behavior is intended, useful, or desired?

59

The talks on the return override installing private fields was also before my time, but I feel like someone once told me one reason was web CustomElements when the element gets upgraded. Maybe others could expand/correct me.

60

Searching in the notes archive this looks like it might be of interest: