tc39 / proposal-function-implementation-hiding

JavaScript language proposal: function implementation hiding
https://ci.tc39.es/preview/tc39/ecma262/pull/1739
MIT License
98 stars 7 forks source link

Censoring code should only be possible for native code, not userland code #16

Closed getify closed 5 years ago

getify commented 5 years ago

[Update: The title and OP of this thread was misguided, based on my misunderstandings, and it's been significantly clarified below]

I understand why a JS engine wants to be able to censor any self-hosted JS code from view. I don't love that in the first place, but I at least understand its motivations.

But I am opposed to adding a feature to userland JS that allows censorship of userland code.

  1. This proposal does not make it clear whether devtools' inspection of such censored code will be prevented. I saw one mention in another thread that it won't "necessarily" affect it, but that's not a definitive position one way or the other.

    It is imperative, before this proposal should advance, that it be clarified whether any of the censoring would be contained to runtime-only, or whether it could affect devtools usage as well, and also how those two "modes" interact, if there are two modes.

    For example, what will be the behavior of step-debugging through censored code? If the runtime censoring would have prevented inspection, but now all of a sudden that devtools is open and engaged, the censoring is NOT happening, then you're actually changing the behavior of the application. For example, a feature may break when run on the page (because it's trying to parse censored code) and then mysteriously start working once devtools is open.

    I should note that any censorship of userland code from devtools inspection is highly objectionable, not the least of which reasons is, for security purposes. I absolutely must be able to see any of the userland code that may be running on a page. Otherwise, malicious code can trivially hide itself and make detection and removal hard or impossible. This is unacceptable.

  2. Even if there's some way to safely and sanely navigate the runtime/devtools mode divide, I still object to this proposal because it unnecessarily handcuffs certain userland utilities.

    For example, I am currently exploring building a utility/library that, for a variety of reasons, unfortunately will need to inspect, and even possibly lightly modify, the code from a userland function, returning back a new function that's been "re-compiled". I won't go in all the motivations behind this, but it's in the area of creating better management of cancelation states in application code, both from a convenience perspective and from a functionality perspective. Think of this as sort of the userland runtime equivalent of some of the same kinds controls and verifications that some frameworks apply to code during build step transformations and linting.

    Another example, I have a publicly shipped FP library (FPO that, for convenience of use of one of its utilities, uses a regex to make a best-effort parse of a function's parameter list to default a set of options for its behavior. Of course, it's not perfect or 100% reliable, so the default has to be overridden for more complicated function signatures. But in the 80% case, it works great.

    In both of these cases, plus many others out there, there's a hazard to a developer who uses such tools if they are able to rely on them working based on the ability to get at the .toString() output, and then all of a sudden one of the functions they pass in starts being censored, and now it breaks.

    How can a developer be expected to keep up with whether the library they're using decides at some point to start shipping with censorship turned on? And that hazard is even significantly higher if there are other out-of-band mechanisms for the censorship, such as HTTP headers returned from a CDN, or whatever.

Censorship of native code is less than ideal, but reasonable.

Censorship of userland code is unacceptable.

ljharb commented 5 years ago

Censorship of userland code is already possible in ES5 with .bind().

getify commented 5 years ago

AFAICT, calling bind() on a function produces a new function that actually came from the JS engine, which indeed censors itself. The "native code" I see there isn't a lie; the bound function is indeed native code. It's sitting in front of a non-censored userland function that I may, or may not, be able to separately access. So it may, or may not, have the effect of preventing me from seeing the original function's code.

NOTE: that trick doesn't come without a price. bind() on a function strips the function of being able to take advantage of dynamic this binding, by forcing it to be hard-bound to some specific this context. In other words, userland code can't "just" censor itself, it has to make the deliberate choice to cut off normal dynamic this behavior in doing so. That certainly limits its suitability, at least some.

Furthermore, the fact that you can hack a censorship of a userland function does not put that on equal footing with a feature literally designed specifically to pave the path for it. This feature would actively encourage this behavior, indeed endorse it direct from the language spec.


But I'll turn this around: if it's sufficiently "hiding" of code for a function by just only exposing the bind() result of it, then this pragma isn't actually needed and the engine can easily (and probably super efficiently) just do exactly the same thing as is done with a .bind() call, and have been able to do so since ES5, as you point out.

michaelficarra commented 5 years ago

You can censor with full fidelity using a closure instead of bind

const safeApply = hide.call.bind(hide.call, hide.apply);
function hide(fn) {
  return function() {
    return safeApply(fn, this, arguments);
  }
}

let hidden = hide(function hidden(param) {
  // you can't see me
  console.log(param);
});

hidden('test');
console.log(hidden.toString());

With a little extra effort, you can preserve the name and length as well.

peey commented 5 years ago

@getify If I understand this proposal correctly, then it won't affect operation of anything which has access to the source, it'll only affect programmatic access on a function made from within the source code.

So

  1. It won't affect devtools' operation. Devtools should be directly operating on the source code.
  2. Many tools which need to transform the syntax in some way (e.g. a tool which applies compile time optimizations and modifies a function definition) would be best suited at build-time (along with all your babel and flow / typescript).

Though I do think that it'll affect the library you linked, but they've directly addressed this as something libraries shouldn't be doing in the proposal by giving Angular's example.

P.S. The word "censorship" might give the impression that code is unavailable to users to inspect in any way before it's executed, which is why I also read the whole readme to try to figure out if that's the case.

getify commented 5 years ago

@peey

It won't affect devtools' operation. Devtools should be directly operating on the source code.

I read the proposal and I was not clear on this point, and I still am not. If there's something I missed, I would certainly appreciate clarity.

I'm skeptical that censorship of the code could happen only for run-time and not for devtools, given this sort of complexity as I mentioned:

For example, what will be the behavior of step-debugging through censored code? If the runtime censoring would have prevented inspection, but now all of a sudden that devtools is open and engaged, the censoring is NOT happening, then you're actually changing the behavior of the application. For example, a feature may break when run on the page (because it's trying to parse censored code) and then mysteriously start working once devtools is open.

IOW, how is it possible for the run-time code and the devtools inspector to be running through a piece of code simultaneously, where the devtools can step through (and thus display) that code, but the runtime is somehow prevented from seeing the code that's being stepped through at that moment? That really makes no sense to me.

ljharb commented 5 years ago

That’s already the case; devtools can see into closures, but runtimes can’t. It’s not censoring it from the runtime, it’s censoring it from user code.

getify commented 5 years ago

I still object to the "user code running context", or whatever we call it, being able to be censored from seeing other user code. It's going to break existing libraries like FPO. It's going to break any tool that has previously relied on any sort of inspection of code via toString. For example, I believe the Benchmark.js library relies on being able to take a function and re-compile it.

Many tools which need to transform ... would be best suited at build-time

I disagree with this assertion. But even if that were an acceptable assertion, this censoring would affect any of those tools if those tools themselves are written in JS and run in JS, because any JS environment that ran the tools would be subject to the censoring.


The proposal mentions a distaste for tools that automatically inspected parameter names and did dependency injection. I understand why that might be frustrating to some folks like the author(s) of this proposal, but for others it's vital and useful. And most importantly, it's been possible for decades.

That capability should only be taken away, breaking existing libraries/tools, for extraordinary reasons, such as serious security concerns. None such exist here, or at least haven't been put forward. This proposal basically claims it's mostly for implementer convenience.

This proposal is too broad in allowing the broad access to censorship for all of user-land code. Censoring should be restricted to something that only privileged environment code can do.

michaelficarra commented 5 years ago

If you want an API for reflecting on parameter lists, ask for that API. Function.prototype.toString is not that.

ljharb commented 5 years ago

@getify given that censorship is already achievable (since ES5, at least) with a combination of .bind and closures, what new capabilities do you think this proposal offers? (it certainly makes such censorship easier and more ergonomic)

getify commented 5 years ago

Function.prototype.toString is not that.

You claim that, but for 20 something years it's been a fine API for that. You shouldn't be so quick to be happy about breaking people's tools.

domenic commented 5 years ago

Nobody's tools are being broken; I would appreciate more good faith and less hyperbole. This is an opt-in feature, and any tools that would have trouble with code that uses this feature already have trouble with closures, built-in functions, rest arguments, etc. Tools of the type mentioned rely on a very specific contract to be upheld by the code they are consuming, and any code that upholds that contract will just choose not to use this feature.

getify commented 5 years ago

it certainly makes such censorship easier and more ergonomic

It not only makes it easier, it endorses and encourages it.

I imagine almost any JS API (like google maps, etc) that doesn't want you accessing or messing around with code will immediately add this one pragma to the top of their files just to shut down any attempts people have made to run-time monkey patching.

Moreover, the "breakage" I'm talking about is that the pragma in question (especially at a file level) is far easier to "accidentally" create unintended censorship problems. If I wanted to go out of my way to censor a specific function, I had to very intrusively change the design of that single function (via the closures or binds or whatever other clever approaches). I couldn't just put a single line at the top of a huge concatenated file.

Say someone is using FPO on their own code, and also on some methods provided by google maps API. Then all of a sudden google maps ships an update that starts censoring their own code. Now, that person's site is broken because FPO no longer can parse the parameter list (or whatever).

ljharb commented 5 years ago

This pragma doesn’t alter runtime monkeypatching whatsoever, just code introspection (ie, not reachable value introspection, which is the only thing you can monkeypatch).

getify commented 5 years ago

and any code that upholds that contract will just choose not to use this feature.

Google Maps certainly doesn't know about or care that FPO parses a parameter list of a function. They are not going to restrain themselves from censoring their methods just because a few users were using FPO to curry those functions and now all of a sudden that breaks.

They'll ship the censoring because it makes sense to them, and shift the burden to the end-developer to make sure that didn't break anything for them.

That end-developer probably had no idea that FPO was relying on being able to inspect a parameter list from toString(), so they couldn't even imagine that an update from google maps would have broken their code in that way.

getify commented 5 years ago

This pragma doesn’t alter runtime monkeypatching whatsoever, just code introspection (ie, not reachable value introspection, which is the only thing you can monkeypatch).

I think you are completely failing to imagine the kinds of things people have figured out how to do with toString() usage up to this point.

ljharb commented 5 years ago

Do you have more examples besides FPO? I’d be interested to learn about use cases that prefer runtime introspection over a build process.

getify commented 5 years ago

I would appreciate more good faith

I am acting in good faith. I have cited several examples (FPO, Benchmark, etc) of tools that:

  1. Rely on being able to access the toString() output of functions provided to those tools
  2. The end developers are likely not much aware of this fact, it's a nuanced implementation detail not a main documented feature
  3. These tools are certainly being used against "third party" code in addition to the end-developer's code
  4. That third-party code can ship a one-line change update with the censorship pragma turned on, and now all of a sudden the app is broken
getify commented 5 years ago

prefer runtime introspection over a build process.

Again, this distinction is not actually relevant here, since the "build process" could include tools written in JS, and thus running in a JS environment, and thus subject to the JS engine applying the censorship to the build tool.

Not everybody builds full fidelity parsing tools, sometimes you want to do one small little task and inspecting toString() output has, for 20ish years, been entirely fine for that.

Do you have more examples besides FPO

Other than FPO and Benchmark, and the Angular dependency injection cited in the original proposal, I wrote production (non-library) code on numerous occasions for my template engine (grips) which relied on being able to parse the parameters being passed to a function, and if a parameter had specific name, then the template engine made sure to pass along certain properties... sorta like dependency injection, but in a really specific, not general, way.

Another example I did once: I parsed a function's toString() and if it expected a certain argument in the first position, then I returned a wrapped version of that function which passed a specific value for that argument.


The point is, there's plenty of code that has relied on toString() over the years -- however advisable those actions were -- and it's worked just fine up to this point. Introducing a feature to userland code that can, in one line fell swoop, essentially affect hundreds or thousands functions that used to have a kind of behavior and now don't... that will certainly break some code, some where.

It's kind of surprising to me that some in this thread continue to insist that's not the case. It's a plain, logically reasonable conclusion of this proposal as written.

getify commented 5 years ago

From twitter, this library (along with others like it, apparently), accesses a function's toString() output and "recompiles" that function in a separate (worker) thread:

https://github.com/developit/greenlet


Another example cited on twitter, relying on this strange usage to embed various text inside a function in a block-comment and access that code later.

function bar() {/*
   <div>..</div>
   <div>..</div>
*/ }

bar.toString();  // "function bar(){ /* ...... */ }"

Another similar example: https://github.com/sindresorhus/multiline


I can't find the link right now, but I recall a library years ago that did something similar, where it parsed (at run-time) JSDOC-style comments from a function you passed it, and did certain things based on what was parsed out.


Again from twitter, React DevTools checks if code is minified by looking at toString() output:

https://github.com/facebook/react-devtools/blob/master/backend/installGlobalHook.js#L133


Another example: run-time instrumentation of code, like re-compiling functions with a console.log(..) inserted on the first line of the function.


From twitter:

@shvaikalesh Libraries like puppeteer or nightwatch use Function#toString to evaluate code in browser context.


https://github.com/dfkaye/metafunction uses toString() to instrument/modify functions during tests.

domenic commented 5 years ago

Flooding this thread with libraries that use toString is unhelpful; if you feel compelled to do so, please edit a single post to minimize the amount of notifications and emails you generate. Thank you.

Pauan commented 5 years ago

Introducing a feature to userland code that can, in one line fell swoop, essentially affect hundreds or thousands functions that used to have a kind of behavior and now don't... that will certainly break some code, some where.

There's nothing stopping existing code (e.g. Google Maps) from using censoring techniques (like .bind()) throughout their code:

var foo = function (x, y) {
    ...
}.bind();

This achieves the same censoring, using existing techniques.

Why don't they do that? Because it offers little benefit, and it has potential problems for downstream users (as you noted). I don't see how this proposal changes any of that.

Are you just concerned because it's "too easy" to censor? Do you think it is significantly easier than slapping .bind() on function definitions?

Do you think that the small extra ease of use (compared to bind) will lead to thousands of popular functions being censored (which aren't currently censored with existing techniques)?

And that developers will blindly apply the censoring without understanding the consequences of it?

And that the developers of said popular functions will then ignore users complaining about the censoring?

getify commented 5 years ago

.bind()... achieves the same censoring, using existing techniques.

As I've already addressed in this thread,.bind() isn't a direct equivalent because it locks the this context, meaning the function in question can no longer respond to dynamic this binding. You could only use bind() for censoring of functions that didn't care about a this (or rather, didn't care about having a dynamic this). That limits its effectiveness, and is one potentially one reason why more functions aren't censored.

Same goes for the intrusive closure wrapping pattern... it works in some circumstances, but would be limiting in others. And it's also a fair bit more effort, so maybe that effort is not seen as being worth it when repeated for each of a file's thousand functions.

A "use censored" (or whatever), especially at the top level of a file, would have none of those drawbacks, so would be more likely to be used.

Are you just concerned because it's "too easy" to censor

That's not my only concern, but it's a major component of my concern. It's not a minor shift in usability (of censoring), it's a major shift. It's also a neon sign that says, "Hey, look, we're officially endorsing the idea of userland code censoring itself!!!" That will draw attention to censoring, which combined with how easy it is, certainly increase how often it's seen in the wild.

mathiasbynens commented 5 years ago

I believe the Benchmark.js library relies on being able to take a function and re-compile it.

Only for the function you pass in, which is generally where any calls to library code happen. Please stop using Benchmark.js as an argument against the censorship proposal, as it is not.

getify commented 5 years ago

But I want to address a larger point: the reasoning put forward thus far to justify this feature (for userland code, specifically) is weak and circular.

I apologize for the length of this message. Contrary to other assertions in this thread, I am trying to act in good faith to share what I consider are reasonable concerns.


Let's postulate for a moment that the amount of userland code in the wild currently using censoring techniques (closure, bind, etc) is pretty low. Let's further assume that my above predictions are wrong, and that adding this feature won't lead to any appreciable increase in the usage of censorship; it will only help the few special cases where authors would like to censor their userland code.

In that scenario, pretty much all the userland code (still a very small amount) that needs censoring already has (or can have!) censoring with existing JS functionality.

If these assertions are true, then this feature doesn't justify itself, because it only provides a very minor convenience to a very small set of code, and that's the only benefit it will ever provide. At best, the justification thus far is pretty weak. Anyone that wants to censor their userland code can and should do it using existing functionalities; there's no compelling reason to give these corner cases additional affordances.

Features should only be added to JS if they have strong justifications. This one does not.


But let's now assume the opposite: "This feature will offer a significant improvement in the ergonomics of censoring code, and be used a lot more as a result! Strong justification!"

Those who were already censoring code are quite happy that they can now censor much easier. They throw "use censored"; at the top of every file and take out all those ugly bind()s or closure-wrappings strewn throughout. By removing any of the barriers or inconveniences that may have restrained their censoring before, they now are inclined to censor by default. Instead of only censoring a few functions in their code, effectively all of their functions are censored.

Because... why not? If you are inclined to do any censoring, and now there's a feature to allow it quite easily without any downsides, it's reasonable to assume you'll censor pretty much everything.

It can then reasonably be expected that a relatively larger chunk of other userland code out in the wild that previously wasn't being censored (impractical due to this issues, etc) will now be a much more likely to be censored. More authors are likely to learn about the idea of censoring by seeing another competing lib do it, or reading a blog post.

Anecdote: I've written JS for 20+ years and written 8 books on the topic, and I didn't realize until this discussion thread that .bind() could effectively "censor" code. A lot of people who may have wished they could censor code may have never realized there was a "simple" way to do it. Now there will be release notes for a new version of JS that calls out this new "censoring" feature.

We're also making it easier for bad actors to abuse censoring, like patching a global function but "hiding" that fact with a censored toString(). They can already censor, yes... but we're rolling out the red carpet for them. The potential for increased abuse weakens the justification for this feature.

It's obvious that censoring as a technique, with the removal of all barriers, will increase in popularity. In fact, the crazy slippery slope argument here is, eventually this technique becomes so common that it becomes a default in babel, and now basically most code is censored. Because... why not?

Unfortunately, the more popular censoring becomes, the more likely it is to break other code (as the cited examples up-thread indicate).


But, you say... "Censoring simply won't be used in places where it breaks code. It's just opt-in anyway."

The problem with thinking of this as "just opt-in" is that most of us use code in our apps that we don't own. Unlike "use strict", which essentially affects only the runtime behavior of code in that file, "use censored" can leak its effects out to other code.

Most of us rely on being able to include two third-party libs A and B, and have these two interact in predictable ways. If A decides to censor itself, and that happens to break B, whose shoulders does that breakage burden fall on?

A likely won't care (or even know, at first) that they've broken interoperability with B. Only if enough users care about this breakage, and complain enough to A, will they consider undoing the censoring. If it's an OSS project with good community stewards, these complaints may be heard. If it's a corporate owned library (like Google Maps), that's less likely. At best, they'll just add to their FAQ, "Not compatible with B."

B will be harmed by the decisions of A. In a similar way to how browser game theory works, if a dev uses A and B, and B is the one who breaks when used with A, which one are they likely to blame? Of course, B. They'll just stop using B.


"But, but... if this is really a problem, and not just hyperbolic overreaction, these problems will work themselves out."

OK, let's then postulate that the breakage of B becomes a big enough deal that A rolls back their decision and stops censoring. Great, problem solved, right?

If all or most of the As out there try out censoring, but many or most of them roll back to not censoring because of breakage (or even just fear of breakage) with other libs... sure, breakage was minimized.

But here's where the justification gets circular. What's also true is that we're back to the beginning scenario, which is that this feature would be of very limited practical usage in the wild. The justification is thus greatly weakened.


My position: this feature should be restricted to apply to code shipped "natively" by a JS environment (browser, node, etc). Userland censoring carries weak justification at best, along with enough risk of breakage to rule it out.

ljharb commented 5 years ago

@tannerlinsley can you elaborate? that comment is pretty unhelpful as it is.

peey commented 5 years ago

This proposal is asserting that you shouldn't be using .toString() to get information about a function's implementation, because it's not a nice way to do it. It's not saying that you shouldn't get this information at all.

As an example

A historical example of this in action is AngularJS's reliance on f.toString() to figure out the parameter names of a function. It would then use these as part of its dependency injection framework. This made what was previously a non-breaking change, modifying the name of a parameter, into a breaking one. In particular, it made it impossible to use minification tools in combination with this mode of AngularJS.

It's true that some of this information is needed for some purposes, like in the examples you cited. But many of these can do without runtime availability of a variable's source. During benchmarking, testing, optimizing code to run in different workers, or debugging, the tools already have access to the source code, they don't need to rely on a JS API call to get the source.

For the cases where there is a need for reflecting at runtime, as you say yourself

it's not perfect or 100% reliable, so the default has to be overridden for more complicated function signatures. But in the 80% case, it works great.

So a better solution would be to introduce APIs which reliably let you get information about implementation of a function.

"Censorship" here does not mean non-availability of code. The code is always available to the client to inspect before running. Your browser will always be able to instrument, optimize, and check what the code is doing before you execute, because your browser will have access to the source.

Pauan commented 5 years ago

As I've already addressed in this thread,.bind() isn't a direct equivalent because it locks the this context, meaning the function in question can no longer respond to dynamic this binding.

If dynamic this is needed, it's easy to create a custom censor function:

function censor(f) {
    return function () {
        return f.apply(this, arguments);
    };
}

This also achieves censoring in a quick and easy way (while supporting dynamic this). So my points still stand.

That limits its effectiveness, and is one potentially one reason why more functions aren't censored.

All of your arguments essentially hinge on the idea that developers really want to censor things, but it's so difficult to do so, thus preventing them from censoring. But this proposal makes it easier, and therefore suddenly the floodgates will open, and a lot of things will be censored!

I disagree with that perspective. I think most developers quite frankly have no interest in censoring. There are currently many easy ways to censor, yet they are not used. The reason is simple: there's no motivation to censor, because there's not really any benefit to censoring.

So the burden of proof is on you to demonstrate that developers want to censor but they don't because it's too difficult. Do you have any evidence for that?

If these assertions are true, then this feature doesn't justify itself, because it only provides a very minor convenience to a very small set of code, and that's the only benefit it will ever provide. At best, the justification thus far is pretty weak. Anyone that wants to censor their userland code can and should do it using existing functionalities; there's no compelling reason to give these corner cases additional affordances.

I think this is a good argument. Thus far it is the only good argument I've seen.

A lot of people who may have wished they could censor code may have never realized there was a "simple" way to do it. Now there will be release notes for a new version of JS that calls out this new "censoring" feature.

That might be true, but this proposal doesn't really change much. If devs want to censor, they'll eventually figure it out regardless of this proposal.

We're also making it easier for bad actors to abuse censoring, like patching a global function but "hiding" that fact with a censored toString(). They can already censor, yes... but we're rolling out the red carpet for them. The potential for increased abuse weakens the justification for this feature.

I think this is a poor argument. There is absolutely no condoning of abuse in this proposal. As you say, it is already possible to censor, and thus bad actors are not "enabled" by this proposal.

Unless you are arguing that the bad actors are so incompetent that they cannot figure out censoring on their own without an official censor feature.

And the flipside is that the ability to censor makes things better for polyfills (since they want to be indistinguishable from native builtins).

It's obvious that censoring as a technique, with the removal of all barriers, will increase in popularity.

I do not think that is obvious at all. Removing barriers does not automatically increase popularity. You can remove all the barriers from an undesired feature, and it will still be undesired.

In fact, the crazy slippery slope argument here is, eventually this technique becomes so common that it becomes a default in babel, and now basically most code is censored. Because... why not?

You have already pointed out many reasons "why not", and those reasons would be brought up to the Babel team.

Anybody can create crazy "what-if" doomsday scenarios, but in order to be taken seriously it needs to be backed up by something.

Unlike "use strict", which essentially affects only the runtime behavior of code in that file, "use censored" can leak its effects out to other code.

That isn't true. "use strict" affects the runtime behavior of other code:

function qux() {
    console.log(qux.caller);
}

function foo() {
    "use strict";
    qux();
}

function bar() {
    qux();
}

foo();
bar();

Niche? Sure, but so is .toString() parsing. And .caller used to be used a fair amount, in order to recreate stack traces on old browsers.

If A decides to censor itself, and that happens to break B, whose shoulders does that breakage burden fall on?

The burden is on B, because it was relying upon implementation details of A (which by definition you shouldn't do).

Because it relies upon implementation details, .toString() parsing requires a contract between different code units.

If there isn't a contract in place, you simply cannot rely upon .toString() of third-party code. Period. That is true today, and this proposal doesn't change that.

And if there is a contract, one of the requirements would be "you cannot use "use no Function.prototype.toString"".

To be clear, by "contract" I just mean "rules you need to follow in order for .toString() to work properly". I don't mean a legal contract. In many cases the contract will be implicit and informal.

There are plenty of ways to break that contract even without censoring:

So even if censoring was forbidden, there would still be plenty of ways for third party code to break .toString().

A contract is required, there is no way around that. You simply cannot expect third party code to reliably work without a contract. Most of the examples you listed earlier have a (implied) contract, so this proposal does not break them.

It is unfortunate that .toString() is often needed to achieve certain functionality, but the solution in that case is to add in proper runtime reflection APIs. And in any case this proposal does not make the situation any worse.

If all or most of the As out there try out censoring

Do you have any evidence that devs want to "try out censoring"?

My position: this feature should be restricted to apply to code shipped "natively" by a JS environment (browser, node, etc). Userland censoring carries weak justification at best, along with enough risk of breakage to rule it out.

Censoring already exists, it cannot be "limited to native code".

getify commented 5 years ago

This also achieves censoring in a quick and easy way (while supporting dynamic this). So my points still stand.

Having to wrap every single function from a file (maybe hundreds) in a run-time call (and losing some hoisting benefits), is not at all comparable in "easiness" to putting a single pragma at the top of the file. Your points don't stand.

All of your arguments essentially hinge on the idea that developers really want to censor things

Half of my arguments assume that, and frankly, to justify adding a feature to JS, it ought to be expected there's a decent amount of expected usage, not just a few niche users (like TC39 members who work for a browser).

The other half of my arguments, where my actual opinion lies, is that this feature is poorly justified because it's awfully niche, and not solving things which are substantial enough problems for the JS language to concern itself with. As you keep pointing out, there are so many ways already that those few who want to censor can, so just tell them to keep doing that.

Moreover, what I'm worried about is whether what's niche now becomes popular by paving a cowpath -- a social-effect progression that's plainly common in many areas of software -- because I don't think userland code should be censoring itself. I don't think polyfills should do it, I don't think corporate libs (Google Maps, etc) should do it, and I don't think OSS libs should do it.

I don't know how many want to do it, but I definitely don't want to encourage it.

The burden is on B, because it was relying upon implementation details of A (which by definition you shouldn't do).

I dispute your assertion that any such B was doing something "wrong". You may dislike that they do what they do, but they do exist and they've been relying on features that have, more or less, been reliable in JS for a long time.

B's rely on toString(), and have for decades, because there wasn't another option for whatever they were doing, not because that was their preference. Whatever benefits that B brings, even if on a best-effort basis, are preferable to (at least some) end-developers than the alternative, which is to not have the capability of that B at all.

You claim that toString() reliance should be replaced by something else. Great! My B's would be happy to switch to it. Once that lands in JS, let's come back and revisit userland censorship of toString().

And in any case this proposal does not make the situation any worse.

However niche you claim reliance on toString() is, a number of those B's exist, and they've existed in the tenuous but stable and workable situation that techniques which obscure/censor the toString() output are incompatible, yet thankfully those censorships are rare.

This isn't about contracts. This is about what observably works, what doesn't, and whether the observably working is likely to continue working or not. That's the gray area a lot of us play in most of the time.

It's disingenuous to claim that this proposal is only for a few authors wanting to save a few lines of code or have a slightly nicer developer convenience. If that's really the case, the Stage 1 "Make the case for the addition" requirement shouldn't have achieved consensus.

It strains credulity to propose adding a feature to a language, the world's most ubiquitous language at that, and not expect that more people will use that feature once added, than the number of people who, prior to the feature, hacked in work-arounds. Of course more will use it.

Features that make censorship easier and/or more common are an existential threat to any B. That situation is plainly worse with this feature, from the perspective of someone who's written many sorts of Bs over the years.

Censoring already exists, it cannot be "limited to native code".

This is a strawman. My usage of "censoring" in that summary is not about the broader set of practices, but specifically about this proposal. This proposal could be limited to environment-hosted code, and it should be.

Pauan commented 5 years ago

not just a few niche users (like TC39 members who work for a browser).

This proposal has absolutely nothing to do with browsers, they are already perfectly capable of censoring.

This proposal is specifically for users of JavaScript.

I don't think polyfills should do it

Polyfills absolutely should censor. It is necessary to make them as close to the native APIs as possible.

I dispute your assertion that any such B was doing something "wrong". You may dislike that they do what they do, but they do exist and they've been relying on features that have, more or less, been reliable in JS for a long time.

Whether they exist or not is irrelevant. Using .toString() requires a contract (whether implicit or not). This is a fundamental fact. Nothing changes that.

The reason they work is mostly by accident, it's not something that can be relied upon without a contract (I already listed many things which cause .toString() to break, and the situation will only get worse with time).

This isn't about contracts. This is about what observably works, what doesn't, and whether the observably working is likely to continue working or not. That's the gray area a lot of us play in most of the time.

You should go back and re-read my comment. I had edited it after posting, so you probably missed some things.

I was not referring to legal contracts, but simply to "the rules that must be followed in order to make .toString() work".

Those rules will vary from situation to situation. Those rules are often implicit (and not written down). But they do exist.

This is a strawman. My usage of "censoring" in that summary is not about the broader set of practices, but specifically about this proposal. This proposal could be limited to environment-hosted code, and it should be.

This proposal is only useful for users. Browsers are already fully capable of censoring (and they do censor already).

So saying that "this proposal should be restricted to native code" is the same as saying "this proposal shouldn't exist at all".

If you were suggesting that, you should have said so outright. Since you didn't, I assumed you meant something else (e.g. about the general practice of censoring).

Nowhere in your sentence did you mention "proposal", only "feature" or "userland censoring".

And since all censoring techniques carry the same risk of breakage, hopefully you can see why I might have interpreted your sentence as referring to censoring in general.

I really do not appreciate you assuming bad faith on my part, especially when your sentence was ambiguous (and thus open to interpretation).

Quite frankly, I am frustrated with talking to you, so I'm going to stop now.

littledan commented 5 years ago

To respond to the original concerns of this post:

  1. I'd personally recommend that DevTools show censored code, just like I'd recommend that DevTools show private fields and methods (all of this modulo any "blackbox" features that DevTools may have). TC39 doesn't make specific requirements for DevTools, though--I think this is a good thing, since DevTools are innovating really fast these days, and they shouldn't need our permission.
  2. One path forward for tools is transpiling away the censorship declarations. As others have explained, it's already opt-in, and there are already ways to do it, but there's still a practical issue about what to do if we encourage more censorship to happen than would otherwise. I would guess that this proposal makes things easier for tools, as it makes a clear pattern which is easier to recognize and remove by build tools than the various existing idioms to achieve the same pattern.

What do you think?

getify commented 5 years ago

This proposal has absolutely nothing to do with browsers, they are already perfectly capable of censoring.

If you were suggesting that, you should have said so outright.

Nowhere in your sentence did you mention "proposal", only "feature" or "userland censoring".

your sentence was ambiguous (and thus open to interpretation).

For the entirety of this this thread, I've been saying "censoring of self-hosted/native code is understandable and should be allowed, but censoring of userland code should not." I've been very consistent in that position across all my messages.

For one, my usage of "censoring" here has always been about the feature in this proposal. I wasn't opening this thread to say that TC39 should take away all the general censoring techniques that already exist, which clearly would be impossible... this whole thread was about me saying that this proposal's censoring is where my objection lies.

I thought my position-summary above was quite unambiguous: "My position: this feature should be restricted to apply to code shipped "natively" by a JS environment (browser, node, etc). Userland censoring carries weak justification at best, along with enough risk of breakage to rule it out."

I'm not sure how I could have been any clearer about what I was focused on in this thread, but in any case, I apologize for the miscommunication.


Secondly, I opened this thread because I thought I understood that at least part of the motivation for this proposal was to make it easier for self-hosted code to be censored.

The whole title of this issue was intended to make my case that this proposal should be narrowed to only applying to the self-hosted kind of code, not to open userland code. I also repeatedly implied that the (im)balance between these two kinds of usage was part of my argument for weakness.

I'm frankly unsure now, upon a complete re-read, why I thought that this proposal had anything to do with self-hosted code. I definitely was certain it was for both, and I feel like I must have gotten that impression somewhere. Maybe I was confused from another proposal? Shrugs.

If I had properly understood the scope, I would have instead titled the issue differently, I wouldn't have kept implying things like "restricting", and I would have argued more narrowly throughout this whole thread.

So, I apologize for misunderstanding the scope of this proposal, thinking that it was broader than intended.

I think I'm clear now. So let me clarify my position. This proposal should be rejected, for the following reasons:

  1. The justification for this proposal is weak, because:

    • userland censoring of any sort is a very narrow, niche approach
    • userland censoring of any sort has been vehemently argued in this thread as already having a variety of viable approaches, without needing this proposal's feature
    • userland censoring may have some good uses, but it also has some abusive uses
  2. This proposal cites multiple justifications, but which are upon inspection, at odds with each other in preferred outcome. That further weakens the proposal:

    • Some stated justifications in this proposal, such as libraries hiding their implementation details to protect future refactoring, have absolutely nothing to do with pretending to be native code. Painting over those as "native code" is completely misplaced.

      It's bad precedent to make it easier for userland code to claim that it is "native code" when that's not what is meant at all. That both increases confusion and also increases potential for abuse.

      These usages would only make sense (and be safer) with a label like "censored code" instead of "native code".

    • AFACIT, other stated justifications in this proposal, like polyfills, would gain absolutely no benefit (over what can currently be done before the proposal) from a label like "censored code". The only apparent reason a polyfill benefits from this proposal is in its ability to be able to claim to be native code.

    Whichever label this proposal ends up using, it can't do both. That means whichever one is picked, the other justifications in the proposal are moot, and thus the proposal justification is weaker.

  3. If this proposal were to be accepted, it would inevitably lead to an increase in censoring, to an unpredictable degree, and this increases the existing code breakage hazard.

    Any proposal which makes it more likely for existing code to break than before the proposal, should automatically be a cause for concern. Such a proposal should only get support if its justifications are strong enough to outweigh that risk, such as overriding user security concerns. This proposal does not have that level of justification.

ljharb commented 5 years ago

userland censoring may have some good uses, but it also has some abusive uses

What would be "abusive" uses?

Note that no code will be broken by this proposal - "breakage" means "old code interacting with old code will continue to interact the same". If there's any new code involved, all bets are off - this includes if someone changes a function to hide its code (through any mechanism).