Repository navigation
Tracking Issue for Runtime Deprecation of Buffer constructor #19079
Description
Activity
One thing we'll need is a concise statement as to why we're doing this instead of leaving things where they are (with
new Buffer()now zero-filling and a documentation deprecation for it). I have a feeling we'll see a lot of comments like this: https://twitter.com/shelleypowers/status/969568430105006080. If we can't answer them quickly and concisely, it could be bad. I'll add an item to the checklist for this.Reacted by Marti Martz and Shelley PowersI think providing a good, concise statement is important because you're basically going back on an implicit promise outlined in https://medium.com/@jasnell/node-js-buffer-api-changes-3c21f1048f97, as well as the API docs for the last two years.
@Trott would you like CommComm's help in crafting the statement? I think we'd be generally happy to help on messaging around this / reviewing a blog post if y'all want to write one.
@bnb More than that, I'd like someone to lay out the technical reasons for doing this. I'd kind of like to collect bullet point reasons for doing this from @ChALkeR and @seishun on the one hand, and maybe from some of the used-to-be-no-never-ever-do-this-but-have-moved-a-bit-in-the-other-direction people (@addaleax and @evanlucas would be my choices there), and kind of try to come up with a statement from all of that. Then I can run it by @nodejs/community-committee and @nodejs/tsc.
you're basically going back on an implicit promise outlined in https://medium.com/@jasnell/node-js-buffer-api-changes-3c21f1048f97
@shelleyp Part of me wants to point out that that's not (as far as I know) an official communication from Node.js but the personal blog of Some Person Who Happens To Be On The Project Technical Steering Committee. You're not wrong to treat the statements in there with weight. At the same time, it's hard for the project to be bound by statements from its individuals. The members of the TSC disagree on things all the time, as do people in the larger project.
Again, though, that issue of communication is the project's problem to solve. You're not wrong.
(Maybe we can ask people to use a disclaimer similar to the ones you see that indicate someone's opinion is their own and not their employer. Seems like something that would be hard to enforce and probably ineffective to me, but I don't know what else to suggest.)Reacted by Daniel Faria Sampaio- added 2 commits that reference this issue
on Mar 2, 2018 It's less that James Snell was a person on the TSC, but was seen as a pretty definitive authority on Node at the time. And he spoke with a great deal of finality about the future of Buffer constructor.
But regardless, Buffer constructor was such a fundamental element of earlier Node releases. Technology support groups typically hesitate or do anything they can to avoid 'breaking' backward compatibility.
Having said that, security is one impetus for breaking backward compatibility.
Still, I can see some poor soul somewhere having created a large complex Node application, who played by the rules and didn't use Buffer constructor, and all of a sudden she's hit with this warning. And she'll look at the app's module dependencies and they're fine, but it's some slub of a module buried layers deep that is perfectly safe...but uses Buffer constructor.
All of a sudden, Node seems far less stable than it once was. Especially when she starts tweeting about this blankity-blank warning that popped up that she can't do anything about.
I'm just trying to give you an outsider perspective, having been through these types of events with other technologies.
So if you all decide to do this, OK. But warn folks ahead of time, strongly highlight this change for 10.0 for those migrating, and provide an explanation, and even provide steps a person can take--either to remove the warning or to reduce the dependency on the module that generated the warning.
Node has had some issues in the last few years (more organizational than technical). It needs to present rock solid stability to the world.
My 2 cents worth
Reacted by Rich Trott, Tierney Cyren, Anna Henningsen, Michael, clive boulton and Ali ArainReacted by Tierney Cyren, Rich Trott, Anna Henningsen, Mathspy and Ali ArainIt's less that James Snell was a person on the TSC, but was seen as a pretty definitive authority on Node at the time. And he spoke with a great deal of finality about the future of Buffer constructor.
Not so much "was". Still here.
So if you all decide to do this, OK. But warn folks ahead of time,
We have been actively doing so.
...strongly highlight this change for 10.0 for those migrating, and provide an explanation, and even provide steps a person can take--either to remove the warning or to reduce the dependency on the module that generated the warning.
As the person who is handling the 10.0.0 release, and the person who will be writing up the notable changes for that release, I intend full well to clearly highlight the new runtime deprecation for
new Buffer(), and communication via other channels such as Twitter have already been used to communicate the change.Other members of the TSC have been actively opening pull requests on ecosystem modules to update code.
Reacted by Tierney Cyren, Sarat Addepalli and Benjamin GruenbaumReacted by Tierney Cyren and Sarat Addepalli@shelleyp I can't commit to anything for the TSC, but I have little doubt that the project will do everything in its power to communicate this as effectively as possible and push for stability of the ecosystem.
Judging by what's happened with prior releases, I imagine that the project will publish a release blog post for v10 and that this specific change will be front and center.
As a member of the Node.js Collection Initiative, I am happy to make myself available as a resource to the TSC help create any/all communication about this change - and any other major changes - in v10 and future releases. If you specifically have any concerns on things that need to be addressed before the release, sharing those would be extremely helpful for us so we can communicate as effectively as possible, addressing the needs of the community.
@shelleyp You are describing an example of an affected person in details — I understand your concern, but to truly determine the best path forward, we need to understand how many people on average will be negatively affected by this (and how) and how many people are expected to be affected if we don't make this change (and how).
The sad thing is that the documentation-only deprecation doesn't help. The number of packages using the old unsafe API only increases over time (yes, we actually have done measurements), and overall impact on the ecosystem caused by modules using the old API also increases over time (yes, we have dome measurements of that too, in downloads/month).
We are trying hard to track down the old Buffer API usage in popular packages to minimize the negative effect that you are speaking about. I have a list of those sorted by downloads/month, a significant number from the top list are already fixed or have have PRs opened, others are still on the roadmap.
A relatively large number of those turned out to be actual or potential security problems (varying from API footguns to leaking secrets to remote unauthorized attacker in a highly popular package), and some turned out to be undefined behaviour problems, like moxystudio/node-cross-spawn#94.
If we don't deprecate — the problem would only intensify over time, given the current trend, and someday real people are going to actually loose their sensitive data.
Reacted by Tierney Cyren, Joyee Cheung and Michał WadasAs I mentioned, security is one good reason to deprecate established functionality. And you have deprecated it, but in the past, you didn't penalize the use—you documented the issues in the API and they were discussed in various articles, etc.
So I'm curious: why the decision to turn this from a documentation into a runtime deprecation? What's happening now that wasn't happening two years ago?
You note that you discovered the modules using the Buffer constructor and many uses are being corrected. But then you say the use is increasing over time.
Have you all thought that a better option might be a tool that people can run in their own system that highlights all incorrect uses of the Buffer constructor so that people can take steps to correct the implementation? Or swap out modules?
Or even identifying Node modules that are using the Buffer constructor, and maybe work with npm to note that in the documentation for the module?
Anyway, I've made my point and don't want to belabor the issue. I understand the concerns and have documented my own. Will be interesting to see what happens.
107 remaining items
- added a commit that references this issue
on Apr 9, 2018 - added a commit that references this issue
on Apr 19, 2018 - ghost added a commit that references this issue
on May 30, 2018 - added a commit that references this issue
on Jul 29, 2018 - added a commit that references this issue
on Jan 6, 2020 - added a commit that references this issue
on Jan 20, 2020
TSC approved runtime-deprecation of Buffer constructor for 10.0.0.
Here's the tracking list. @nodejs/tsc: Feel free to edit this to add more items to the checklist. I'm just getting it started, really.
new Buffer()now zero-filling and a documentation deprecation for it). We're likely to see a lot of comments like https://twitter.com/shelleypowers/status/969568430105006080.What have I missed?
@nodejs/community-committee @nodejs/buffer
FWIW, here's how the TSC voted: