Revisiting the meaning of "hard private". Is undetectability really necessary? #136
Comments
|
Yes, it is necessary, it has committed consensus at multiple proposal stages, it will not be changing, and it’s answered in the FAQ. Being detectable means that someone could write code that branches on the presence or absence of private fields, which makes adding or removing one a breaking change, which violates encapsulation. |
|
As I've stated many times before, I ask what I ask because for me, the FAQ provides insufficient justification for the direction being taken. What I'm looking for here is the clear and logical justification for taking on what seems to be an unnecessary requirement.
From Wikipedia:
Neither of the meanings for encapsulation say anything about detectability, only inaccessibility. Likewise neither of the meanings for encapsulation requires the encapsulating object to be sensitive to the needs of anything external to itself. So in what way does the detectability violate encapsulation? ...Maybe that's the wrong question. Would it be fair to say that the TC39 board is trying to reduce the risk that foolishly/desperately/dangerously creative developers will have their code broken by trying to depend on the existence of an implementation detail? If so, then I must ask why I as a library developer should care if you as a library user try to key off of an implementation detail that you can only detect through trial and error? |
|
Citing a definition of "encapsulation" isn't helpful; it would make adding or removing a private field a breaking change, which is a nonstarter for the entire feature, regardless of what term you want applied. As a library developer, if your library is broken, I claim it's your fault - anything that's possible for users to do, they will do, and if your library doesn't prevent it, then your library doesn't work properly. With this feature, library authors who care about robustness will have a language facility to do so; those who don't care about this concern can continue to use an underscore or Symbols or whatever mechanism they wish to ask users nicely to pretend they're private. |
It is helpful when you make claims like
My point was to show you that this argument is flawed and not in keeping with the meaning of encapsulation with respect to a programming language using objects. If this had been the actual reason the board decided it wants undetectability, then I would have requested profusely that the board re-visit this decision. I am at least requesting that you stop making this misleading and false claim if another asks for justification for undetectability.
Maybe I run in the wrong circles, but every developer I've ever come across knows that they're taking a risk when they use hidden implementation details. Also, it's not a breaking change if the provided API (the contract) didn't change. If a property can be detected, but isn't visible (cannot directly acquire the name) and isn't accessible (cannot acquire the value), then it is not part of the API(hence forth "undocumented API"). This by itself is already "robust", certainly far more so than what we currently have. Undetectability is "overkill". I would be willing to wager that even if undetectability were dropped, the percentage of developers or codebases affected due to undocumented API changes (after the dust settles from everything being updated to best use private fields) would be insignificant. Plus, there's an advantage to having developer's complain when library developers break them by taking away some undocumented API. Events like this give the development community a chance to coax the library developer into a new feature update. I know that you'll probably find what I've said so far unconvincing, but even still, I thank you. You don't shy away from questions, even when you think they've already been asked and answered adequately. Even wherein it doesn't make logical sense, you've done an excellent job of toeing the line on this proposal. |
|
I'm trying to re-think the issue, trying to see if I can become any more appalled by the possibility since you've more or less stated that this is a dire issue that must never happen from the point of view of the TC39. What you've described as the problem resembles this: /* file_1.js: private fields are detectable */
export default class Example {
#someField = "Can you see me?";
}
/* file_2.js */
import Example from "./file_1";
var ex = new Example();
//console.log(`someField = ${ex.#someField}`); //SyntaxError
console.log(`someField = ${ex.someField}`); //returns "someField = undefined"
Object.getOwnPropertyNames(ex).contains("someField"); //returns false
Object.getOwnPropertyNames(ex).contains("#someField"); //returns false
//Now the test
ex.someField = "Not the same field!"; //fails silently due to collision with #someField
if (!("someField" in ex) && Object.isExtensible(ex)) {
console.log("You don't think it's valid to ever be able to get here.");
)Does this example resemble the issue adequately? If not, can you provide me with an example that fully exhibits the problem? If so, isn't this just external brand checking? I can't think of a single scenario where this would be useful beyond deciding whether or not to use the ES is considered to be a "duck typed" language. Isn't it more reasonable to believe that most developers will test the accessible API rather than probe for useless knowledge of the inaccessible?The big question I would like to have answered is "Why does the TC39 believe this to be an issue worth preventing?" |
|
Nonzero developers probe, and anyone observing it is a problem. This happens all the time - npm broke node once because it removed an allegedly "private" property prefixed with an underscore. Anything that can be depended on, will be, and anything that can be done, will be. It's simply not reasonable or acceptable to me to say "meh, good enough". |
|
I'm just curious from this conversation. Are pivate properties present in MyClass.toString()? |
|
@shannon yes, and any discussion of observability or encapsulation has to do so by setting aside the existence of Function.prototype.toString - however, you can |
|
@ljharb I don't follow how you would obscure this. Couldn't you always just do something like this? Function.prototype.toString.call(MyClass)
//or
(class {}).toString.call(MyClass)This seems to me like private properties would always be observable in some sense or another. Is there something I am missing? *Edited class name for clarity |
hidden since this is mildly off-topic@shannon something like this: let boundFoo;
class Foo {
#something;
foo() {}
constructor() {
this.constructor = boundFoo;
}
};
boundFoo = Object.defineProperty(
Object.assign(Foo.bind(), { prototype: Foo.prototype }),
'name',
{ value: Foo.name },
);
Foo.toString(); // ''class Foo {\n #something;\n foo() {}\n constructor() {\n this.constructor = boundFoo;\n }\n}''
boundFoo.toString(); // 'function () { [native code] }'
new Foo() instanceof Foo; // true
new boundFoo() instanceof boundFoo; // true
Foo.prototype.foo === boundFoo.prototype.foo; // true
new boundFoo().constructor === boundFoo; // trueand now |
|
@ljharb I'm not sure this is off topic at all. If the question is, should undetectibilty be a requirement, it's important see that in order to truly hide private fields it requires a wrapper. So private fields are not really hidden from detection by themselves. Which as you stated was a requirement, and anything less would be unacceptable. *Edit "un-acceptable" |
|
@shannon the requirement excludes Function.prototype.toString. |
|
@ljharb I understand this may be frustrating but you have to see this from the other side. You state something as a requirement then state it has an exclusion. Why does one exclusion have more merit than the other? So why have this requirement? What exactly are you preventing? |
|
@shannon it's not actually trivial to programmatically pivot based on that behavior - a regex can't be used to parse a non-regular language like JavaScript. If you pivot based on any change in the toString, then any change in the function - including inside a comment - could be a "breaking change", so this particular facet is just not particularly useful to consider. |
|
@rdking if I'm understanding your code sample correctly, it's missing something extremely important: ex.someField = "Not the same field!"; //fails silently due to collision with #someFieldThis must not fail. That's not just because it would let someone detect the private field. It's because it means that adding a private field breaks existing consumers of your class who aren't even intending to rely on implementations details. For example, adding class LibClass {
stuff() {}
}
// elsewhere
let x = new LibClass();
x.foo = 0;
assert(this.foo === 0);We can quibble about the importance of not breaking people who are deliberately relying on implementation details (though, to be clear, the committee considers it important for the reasons given in the FAQ and we are extremely unlikely to revisit this unless something comes up which we had not previously discussed), but if private fields don't even prevent name collisions, there's barely any point in having them at all.
If you don't mask F.p.toString, then any change to your code is in principle a breaking change, including stuff like rewording a comment. Because that makes the term "breaking" effectively useless, we generally consider "breaking changes" to be restricted to the observable behavior of code apart from F.p.toString. Sidenote: the FAQ spells out exactly what it means by "encapsulation". |
|
@ljharb, @littledan, @shannon, @rdking I suddenly realized that, this proposal doesn't fit class A {
#somePrivate;
partOfPublicApi() {
this.#somePrivate; // brand-checking happens here
}
// all other parts of public API are dropped for brevity
}Usage: const realInstanceofA = new A();
const detector = Object.assign({}, realInstanceofA);
Object.setPrototypeOf(detector, Object.getPrototypeOf(realInstanceofA));
try {
detector.partOfPublicApi();
} catch(err) {
// we do know that partOfPublicApi is using something private
}So, even though committee tries to convince community that undetectability is a hard requirement (which IMO it shouldn't be) and forces that So since current proposal doesn't met requirements for which it was built, it should be at least revisited. P.S. |
|
@Igmat what presentation? i haven't presented anything about class fields. A thrown exception, or the lack of one, does not indicate the presence or absence of a private field - I could be using a WeakMap, or throwing based on any criteria I deem fit. In other words, you're incorrect, and the requirements continue to be met by this stage 3 proposal. |
|
@ljharb or use the number of AST tree walkers then. It's not very difficult no matter how you look at it. If I need to know if a private field is used, there is nothing that anyone can do to stop me. I'm not sure why you would pivot though. What would you do differently with this information? @bakkot I guess that's sort of the point of my line of comments here. While it sounds like a noble effort, I've come to believe that private fields aren't really going to be that effective in their goal to prevent inadvertent breaking changes. With it's declarative and lexical constraints, it's really not that hard to parse and manipulate like enclosed variables are. It just makes it more annoying for everyone. |
Sure there is, as @ljharb pointed out. |
|
@shannon not nothing; i detailed above how you would use .bind to stop you. What someone would do differently is irrelevant; the mere fact that they could makes it a problem. The privacy of variables in functions is precisely what’s needed here; this feature provides it for class instances. |
|
@ljharb, sorry it was a typo - I meant @littledan. Since in terms of brand-checking using BTW, using closure's/Symbols won't lead to same problem, so it definitely This proposal doesn't met its own requirement. P.S. |
|
@ljharb @bakkot except you ignored the other methods I described. I'm sure there are countless others and all it takes is one person to write a library that parses modules on the fly and all the work here is undone. I don't know, I understand what you are going for I just don't see it happening. JS has thrived as a hackable language and will continue to do so despite the efforts here. Why fight it? (rhetorical) Anyways, I'm going to step out of this conversation and continue lurking. |
|
@Igmat It does mean that these things won’t be revisited without new information, of which none has been presented. The changes expected during stage 3 are very limited, per our process document |
|
@shannon parsing source text is outside the scope of whats possible in the language, and parsing toString can be prevented with the method detailed earlier. |
|
@ljharb are you trying to say that breaking EXISTING capabilities of metaprogramming with proxies isn't critical and that prove of breaking of particular initial requirement (undetectability) provided above isn't enough for revisiting the proposal? What kind of feedback should we provide? How many users should show their concerns about this proposal in order you to think about revisiting it? But it also seems that amount of developers you asked about this proposal wasn't too big. And I won't be surprised if your syntax presentation was kinda biased. There are NO other stage 3 proposal that has such a big amount of issues and comments on them. Don't you think that it's a sign that something is wrong here? |
|
@Igmat, the class can of course catch the error and return a default value, thus preventing even that channel. But your claim that this proposal does not meet its own requirements is false: you can detect that a method throws when invoked on a non-instance, if that is indeed the behavior of some method, but because this behavior can happen for reasons other than the presence of a private field and because it does not necessarily happen even if a private field is present, this ability does not mean you have revealed the existence of a private field. The requirement is satisfied. @shannon, the other methods you described are not part of ECMAScript. That's not just a theoretical matter - code on the web, which remains by far the largest platform for ECMAScript, generally is not in a position to read other code with which it shares a realm except via Function.prototype.toString.
JS has thrived as a partly hackable language with privacy available through closures. We're not fighting that; we're just removing the weird edge case where this privacy was awkward to use in classes, hence making classes suitable to replace long-standing patterns which previously required function-based constructors and per-instance methods. In any case, let's step back a moment. Why does this particular question matter? Let's say we decided to drop the requirement that private fields be undetectable without inspecting the source of the class. What then? This would not particularly enable any other design - it would still have to be the case that a base class must be able to introduce a private field without running the risk of it conflicting with a public or private field on a derived class, or a field added to the object by non-class code after instantiation; it would still have to be the case that code outside the class could not read values held by private fields; etc. So what would removing this requirement enable? |
|
@bakkot concern here is that it's VERY rare situation, and this proposal will make it common! I really don't understand how you could evaluate trade-off |
|
@bakkot, sorry forgot to answer to your original idea of comment. |
Revisiting this requirement, leads to revisiting need of |
Do you see how much happened while I was at the store and out for dinner? The sheer amount of concern others are showing towards a fundamentally and logically flawed proposal where the proponents can only ultimately reply by saying something like "well I, value X more than Y" with no evidence or even logic to justify such an evaluation is daunting. Yet you still ask this? Ok. Here's why it matters. It's the linch pin for all the decisions in the FAQ! If this pin is removed, then all the justifications in the FAQ fall apart.
With this pin removed, there's no reason why it can't be. In this post, you said that public and private fields names cannot be mutually exclusive
That's only a problem for consumers who are mutating instances of the class. There's a simple work around for it that everyone already knows: stop doing that! Isn't this actually the proper use case for WeakMap and Map? Weren't these 2 object types added so that 3rd party implementations could store information relative to an object without mutating the object? So, easy fix without even doing anything awkward or changing the language.
With this pin remove, there's no reason why it can't be. Since the collision issue between public and private field names is easily resolved with proper use of maps, ... and just that quick, the balance of the FAQ becomes irrelevant. People gain a syntax they can really get behind. The path to shared private members opens up. The possibility of using private symbols as the underlying mechanism to obscure private names from the public interface becomes reasonable. etc... |
|
Here's that "must not fail" case re-written properly: class LibClass {
stuff() {}
}
// elsewhere
let myInfo = new WeakMap();
let x = new LibClass();
myInfo.set(x, {foo: 0});
assert(myInfo.get(x).foo === 0);Tada! Problem solved. LibClass is free to be mutated as much as the developer wants. It is never the library developer's fault if a user blatantly abuses the library. Using this as a justification for introducing a bad design is just wrong. |
|
@ljharb That's a neat trick. I don't think I would've ever considered using Function.prototype.bind() to hide the source code of a function behind a native code wrapper. Now I'm going to have to find some reason to use that. :) Here's the generic version. With this, the original class doesn't even have to cooperate! var obfuscate = function(obj) {
var retval = Object.defineProperty(
Object.assign(obj.bind(), { prototype: obj.prototype }),
'name',
{ value: obj.name },
);
retval.prototype.constructor = retval;
return retval;
} |
|
@ljharb Something's wrong with your logic...
That nifty little obfuscation trick to hide the source code of a function is just that, a trick. It is not the feature being provided by this proposal. Offering it when someone says something like
is just a fallacy by misdirection. First of all, @shannon is correct. There's nothing stopping him from just looking at the source code. So there's nothing stopping him from knowing whether or not a private field is used. But that's irrelevant since the argument is supposed to be about whether or not the provisions of this proposal effectively stop private field usage from being detected.The truth is, IT DOESN'T. Something akin to this obfuscation trick would need to be used on the class constructor and every public method that uses a private field immediately after parsing the class definition in order to claim that the proposal effectively denies programmatic detection of private fields. Since that's not part of the proposal, this proposal fails to meet one of its goals.
Did you forget that the web is just a file system? All one needs to do is fire off an AJAX request to retrieve the source file and voila! A web browser is ridiculously well positioned to deliver the source code of a particular library to some code searching for private fields. But I think this one isn't a problem because, if my memory isn't failing me, direct viewing of the source file was already considered an acceptable unavoidable weakness. |
|
as is Function.prototype.toString - the goal of undetectability does not include it. |
Your reply was that
To this, I can only say that it's not actually 'trivial' to programmatically pivot based on the presence of an inaccessible field of unknown name either. If the original source code is not accessible to the developer, and your obfuscation method was used to hide the output of You see, this is yet another one of those places where this proposal is trying to have its cake and eat it too. You want to claim that anything can reveal even a fragment of the original source is excluded from consideration from the "undetectability" requirement, all the while claiming that it's trivial to set up external brand checking. The problem with this is that external brand checking is only trivial if one can see at least a fragment of the original source, but you've already excluded that from consideration! So this proposal either:
You must pick one, but no matter which one you pick, the result is the same. The "undetectability" requirement has no merit. |
|
@ljharb Reaching back a bit...
I don't think this qualifies as the same scenario. The field npm removed was a public field marked private by the underscore convention. The code in node that broke wasn't simply a "presence check" but an "access". Now, suppose that at the time the broken code was written, private fields were already implemented without undetectability, and npm had used that code for the offending field. Would the offending code in node have even been possible? If the answer is "no", then you do not have a valid example of the problem you're so concerned about. Please try again. If your answer is "yes", care to explain how? |
|
@bakkot Just for fun, I want to further pull this argument apart. On first glance, it's seems reasonable, but:
Now for the breakdown:
|
|
@rdking Sorry if I'm missing something basic here, but without |
|
@mbrowne I get and agree with your concern, which is why I'm glad to say that the cost of implementing private fields without a sigil will not affect the cost of public own property access at all. It will, however, impose a small delay for access to properties in the prototype chain. From the ES2018 Spec:
Since a private property will only be accessible from functions declared in the scope of the class declaration, private field names need only be checked for initially. Adding support for protected means allowing the check to descend into prototype layers. So a rewrite might look something like this: Edit note: Fixed an oversight below. Forgot to reference CheckPrivate in OrdinaryGet...
So, as far as I can see, there is no penalty for own property accesses, and only a small penalty for prototype chain access. I think this is an acceptable cost for reasonable syntax and future extensibility. Even if the cost is deemed unreasonable, use of the sigil as a private access operator (in lieu of whatever undefined thing it is in this proposal) removes the need for the extra check and still gives us a reasonable syntax. But I still don't see anything in the language or it's definition that would make it costly or difficult to optimize without the sigil. Forgot to mention, given the modifications above, you get |
|
@rdking Thanks for the detailed reply. What about dynamic property names, e.g. |
|
@mbrowne Unless there's something I'm missing (which is entirely possible), there would be no penalty for dynamic lookups that isn't already being paid today. |
|
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. |
|
@littledan Instead of just leaving it at
can you please enlighten us as to why and what use cases undetectability actually solves? |
|
Maybe @erights could fill in here; I'm not sure if there's any particular document that explains all the reasons together. |
|
I would be happy to hear from anyone who's got a better argument than the one presented in this thread. Sadly, the one presented in this thread is logically flawed, supporting its own argument with a premise that it first dismisses. |
|
I too would like to better understand the committee's rationale. Here's my two cents (echoing a point I think I first heard from @ljharb)...JS already has a way to achieve total encapsulation: closures. They can be easily used to achieve class-level private state, but not instance-level. The semantics of this proposal are very similar to the encapsulation provided by closures, so you can think of it as new syntax feature to extend what's already provided by closures and WeakMaps without the inconvenience and awkwardness of doing this manually with WeakMaps. The "total undetectability" is no more and no less than what is already possible for class-level (or function-level or module-level) state via closures. I don't understand why we need to elevate private fields to the level of a security feature (in other languages, that's never been the goal of access modifiers), but perhaps there's something to be said for consistency with mechanisms already in the language. |
|
Security's been discussed frequently in TC39 as a motivation, but for me, I see more the practical value in making private state totally encapsulated. It's a nice property if nothing outside of the class can read the private elements, or manipulate the instance so the private element is no longer usable (as is possible with private methods in the private symbols proposal), etc. If you achieve WeakMap-analogous semantics, you have a pretty strong guarantee that you're meeting these properties. |
|
I still have an unaddressed issue. Can someone please clear this logical contradiction? Or is it the case that the board has decided to move forward with the undetectability requirement despite the contradiction? |
|
Well, I personally think this requirement is reasonable, but use a wrong name "undetectability". As @mbrowne said, such "undetectability" is just same as closure "private" var achieved from the first day of ES1. This is another sign why Classes 1.1 proposal is much better for understanding the constraint. |
|
Well, yesterday, after a good round of conversation with @ljharb, he pointed out a reasonably good reason bad enough to warrant the desire for undetectability: shared code updates for which the developer fails to provide sufficient programmatic distinguishability while making breaking changes. While I still cannot agree that it's the language's responsibility to protect developers from other developers doing things poorly, I also can no longer claim this requirement to be unreasonable. |
No matter how many times I consider the issue, one of the major pain points of this proposal is the definition of "hard private". As I understand it, "hard private" is being defined as "externally both inaccessible and undetectable". Inaccessible is exactly what nearly everyone(except those who think reflection should still work) is expecting from private fields, so there's no argument there. However, the undetectable requirement may be pushing things too far. I want to challenge the "undetectable" requirement as being "security by obscurity" and therefore meaningless in a dynamic language where even the source code is visible to the program.
Here a visual for my argument. Suppose there was a lockbox made out of unbreakable glass and equipped with a lock that was impossible to pick. Basically, anything inside this otherwise impermeable box is visible to anyone who can see the box, but no one has access to the contents except for the one who placed something in the box. Let's also assume that the box is also immoveable except by the person who locked it.
If I were to place an ordinary box with some content of mine inside this otherwise impermeable, immoveable lockbox and locked it, would anyone other than myself be able to access the contents of my box, or indeed, my box itself? This is just an analogy for my understanding of what a private field should be in ES.
If a trailer is loaded with ordinary boxes and a lockboxes like I've described, that trailer is my view of an object. The ordinary boxes are public properties. Anyone can walk up and open those boxes. The lock boxes, however, can only be opened by the one that loaded them onto the truck. Others may see the boxes inside the lockboxes, but they'll never know what's actually contained inside unless the loader allows them to see by opening the lockbox.
This analogy is completely in keeping with the expectation of privacy. What I want to know is what difference it makes whether or not the lockboxes are opaque?
The text was updated successfully, but these errors were encountered: