Node: Tracking Issue for Runtime Deprecation of Buffer constructor

[Trott]

• Version: 10.0.0
• Platform: all
• Subsystem: buffer

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.

• [x] Draft 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). We're likely to see a lot of comments like https://twitter.com/shelleypowers/status/969568430105006080.
• [x] Land runtime deprecation on master (@seishun has a PR open: https://github.com/nodejs/node/pull/15346)
• [x] Write material for upgrade guide (I believe @ChALkeR and @addaleax have expressed willingness to do parts of this, and @targos proposed upgrade guides in the first place so maybe him as well) https://github.com/nodejs/nodejs.org/pull/1666
• [x] Identify important public modules that need patches and submit those patches (@ChALkeR has done a lot on this already)
• [x] Get the word out! Twitter, Node.js Collection post, whatever!

What have I missed?

@nodejs/community-committee @nodejs/buffer

FWIW, here's how the TSC voted:

• 9 in favor of runtime deprecation in 10.0.0: @jasnell @ofrobots @mhdawson @joyeecheung @cjihrig @mcollina @evanlucas @ChALkeR @targos
• 4 "soft" no votes: @rvagg @addaleax @MylesBorins @trevnorris
• 3 abstentions: @danbev @gibfahn @Trott
• 3 folks did not participate: @fhinkel @Fishrock123 @thefourtheye

[Trott]

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.

[bnb]

@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.

[shelleyp]

I 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]

@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.

[Trott]

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.)

[shelleyp]

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

[jasnell]

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.

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.

[bnb]

@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.

[ChALkeR]

@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.

[shelleyp]

As 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.

[Trott]

Adding to the TSC agenda so we can hopefully get some of these things moving.

[thefourtheye]

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?

I recently saw one of @joyeecheung's works, CodeMod for deprecated Node.js APIs. I haven't tried that myself, but I believe it works for Buffer constructor, as per this tweet.

[joyeecheung]

@thefourtheye The codemods are only useful to a certain extent - it does not detect runtime types, only static types, so if the user pass in a variable, we can at best look at the names and guess the types (e.g. size in new Buffer(size) should be a number so it may be transformed to Buffer.from(size), I have not implemented those though).

For code with tests, maybe a better solution is to write something that monkey-patches the Buffer constructor, collect all the traces to find the unique ones, and analyze the actual arguments passed during the tests to provide suggestions for migration. But that's probably much more work to do..

[targos]

We should probably advertise ESLint's no-buffer-constructor rule.

[addaleax]

By the way: I have been using Node with NODE_PENDING_DEPRECATION=1 in my .bashrc for over 3 weeks now, and I don’t think I have seen a single CLI application that doesn’t emit the warning with it.

So I would really appreciate if we at least excluded TTYs from a default runtime warning, as suggested in https://github.com/nodejs/node/pull/15346#issuecomment-368990001.

Also, I would like to strongly encourage all @nodejs/tsc members to also set that flag on their development machines. It should be helpful in converting code that uses deprecated mechanisms, but also just to get a feel for how overwhelming this change is for our ecosystem.

[ChALkeR]

@addaleax What do you think about printing those from the app itself always, and from node_modules — at n% chance, starting with 1% and increasing over time?

The message for the former could look something like this:

Buffer() constructor is deprecated for security reasons, and appears to be used directly.
See more info at https://example.com/, and see https://example.org/ for the porting guide.

The message for the latter could look something like this:

Buffer() constructor is deprecated for security reasons, but is used by a dependency.
See more info at https://example.com/

To always observe this warning, launch with `node --pending-deprecation` or with `NODE_PENDING_DEPRECATION=1` env variable.
To opt-out from seeing this and other warnings, launch with `node --no-deprecation` or with … env variable (not recommended on dev setups).

That way, developers of popular modules would be informed even at n=1 (p=1%) but end users would be affected 100 times less at first.

To inform less popular modules, n should depend either on release or on date, and I would argue that with the presence of opt-out depending on date is the less bad choice, as we don't want people to have additional reasons _not_ to upgrade to a new Node.js version inside a single major branch.

[mcollina]

I like that approach.

[seishun]

Deprecation warnings are already emitted only once per process. Non-deterministic warnings would just add confusion.

[mcollina]

Have we thought about deprecating only new Buffer(num)?

[jasnell]

@addaleax ... Curious... Do you know if the warnings are primarily being emitted by node child processes?

[jasnell]

If so.. we could always make --no-warnings a default for child processes and only emit from the parent

[ChALkeR]

@mcollina Yes, we have discussed that multiple times afaik, and no, that won't work — when arg is user-controlled and typically is a string, Buffer(arg) still poses a problem but won't print a warning that way.

We could specifically white-list Buffer(0) (as if only 0 is passed in it almost always means that it's hard-coded literal Buffer(0)), but that's only a small percentage of usage and won't affect things much. Also, could miss dangereous code.

Also we could white-list Buffer(string, encoding), but on older Node.js versions (4.x LTS) that's still dangereous code, so that should better to be eradicated too. Also, my analysis suggests that whitelisting that won't significnatly change the amount of observed warnings, as almost all popular modules that do that _also_ do Buffer(arg).

So, the Buffer constructor should better be deprecated entirely. The question is how to present the warning to still reach every package out there while trying to minify users disturbance.

For example, ignoring node_modules entirely (as was discussed some time earlier) won't help — many package authors don't run the tests themselves anymore, and the only way the issue in their package could be fixed is by someone noticing it from the outside. Also, developers (and users) should be notified of potentially dangereous code in their deps.

[addaleax]

@addaleax ... Curious... Do you know if the warnings are primarily being emitted by node child processes?

Not that I’d be aware of, and not that I’d expect. Would that make a difference? The code base is still the one of the tool, child process or not…

Also, I don’t find it too curious… we’re runtime-deprecating a very widely used API, so I’d say it’s what one would expect to happen.

[jasnell]

Sorry, my use of punctuation was confusing. I wasn't saying your results were curious, I was saying that I was curious if the deprecation warnings were being emitted from child processes (as in a parent launcher script launching child processes) ... in which case, if we pass --no-deprecation by default to child processes, we can potentially hide most of the noise. If that's not the typical case, however, nevermind :-)

[Trott]

By the way: I have been using Node with NODE_PENDING_DEPRECATION=1 in my .bashrc for over 3 weeks now, and I don’t think I have seen a single CLI application that doesn’t emit the warning with it.

That includes node-core-utils / git node land....

[ChALkeR]

A list of popular packages that still need PRs.

_Note: I will update this every now and then!_

@nodejs/collaborators

If you file (or find) a PR (or are in the process of doing so) for any of those — feel free to label that with a checkmark.
Make sure to link that PR here (by placing a link to this issue, https://github.com/nodejs/node/issues/19079, somewhere in that PR description or comment).

• [x] 25323991 iconv-lite
• [x] 19087569 asn1
• [x] 18218022 sshpk
• [x] 17307557 concat-stream
• [x] 16340003 ecc-jsbn
• [x] 12460913 deep-extend
• [x] 11843560 fstream
• [x] 10197105 bl - PR https://github.com/rvagg/bl/pull/51
• [x] 8384865 asn1.js
• [ ] 7774494 svgo
• [ ] 6958138 md5.js
• [ ] 6528474 winston
• [ ] 5747520 yauzl
• [x] 5680853 tar-stream - PR https://github.com/mafintosh/tar-stream/pull/80

_This list is not complete, I will add new entries after looking closer on those._

Preliminary upgrade guide is at Porting-Buffer.md (@addaleax has fixes to that that I didn't include yet, but the main part is there). Filing PRs to those packages would help greately.

[benjamingr]

I'll post issues on these so that the package maintainers get a notification - most of these are pretty easy fixes.

[ChALkeR]

@benjamingr Those packages don't strictly _break_.
They just print a deprecation warning. At max one common one per process.

---

_Upd: @addaleax (and anyone else seeing this comment and questioning why is it here): the issue titles filed to the repos just above were incorrect initially and mentioned «breaking» instead of a warning_.

[benjamingr]

@ChALkeR good point, I'll change the titles of these

[seishun]

@ChALkeR I would drop variants 2 and 3. Listing them means tacitly approving support for outdated Node.js versions, which implies tacitly approving their _use_. I think we should take a stronger stance against the usage of outdated and unsupported Node.js versions. Helping maintainers keep support for them might make their life easier in the short term, but it's bad for the ecosystem in the long term.

[mcollina]

@ChALkeR I would drop variants 2 and 3. Listing them means tacitly approving support for outdated Node.js versions, which implies tacitly approving their use. I think we should take a stronger stance against the usage of outdated and unsupported Node.js versions. Helping maintainers keep support for them might make their life easier in the short term, but it's bad for the ecosystem in the long term.

I disagree. Dropping support for old Node.js releases has a snowball effect of semver-majors along the dependency chain. This introduces less security in the ecosystem, because old versions are left behind and those massive download numbers are on old major versions. If the change is done in a new semver-major (dropping a Node line is semver-major), it would not be picked up automatically.

[YuryStrozhevsky]

Guys, please type a correct link to a package you are specifying and exclude any links to my ASN1.js package - its has no dependencies from Buffer at all.

[benjamingr]

That was my fault @YuryStrozhevsky - sorry!

(Also as a side note, that in general we prefer to not use gendered pronouns in Node - we're not all guys here)

[ChALkeR]

@seishun Thought about that, that is a valid point. But there is a catch — for libraries, dropping Node.js version support is a semver-major change. And semver-major changes are propagating over the dependency chains significantly slower than minor or patch ones.

So, for libraries that are willing to land this change only in a semver-major version, I would recommend to drop support for old Node.js versions in a new version, _but_ to land a polyfill on older branches so that users don't have to perform semver-major upgrades to receive the fix.

I tried to emphase that in my text, but perhaps that needs further refinement. Care to file a PR?

---

_Side note: GitHub reactions are poorly designed._

_@seishun, I disagree with your opinion there (so I would label that as :heavy_minus_sign: if that was available), but I appreciate the concern and I would like to see more engagement like that (so I would label that as :+1: if that was distinguishable from :heavy_plus_sign: here on GitHub)._

_Sorry, that was me whining again about those._

[ChALkeR]

@benjamingr @nodejs/collaborators Also, to my experience, PRs deal with this kind of issues a lot faster, so if anyone is willing to fix things in any of those packages — please do that. I filed a bunch of those already, you can see examples linked above to this issue. Will continue doing that, but any help would be appreciated.

[mafintosh]

Tiny rant: I understand where this is coming from but this is causing me a lot of work to be honest. I have lots of modules that uses the Buffer constructor with significant usage not on the list (https://www.npmjs.com/package/csv-parser for example). I would much prefer to be spending my time doing something else, instead of having to clone 600+ repos (and those are just my own) and checking the source code of all of them.

[mafintosh]

More constructive feedback from a highly invested npm author:

1. Where is the tools I can run to find which modules I have that needs updating?
2. Reading the guide it recommends dropping support (major bump). That'll get loads of modules stuck in forever deprecation mode (including many of my own where I'm still on an old major) since userland uses ^ a bunch. More work for me.
3. Node 10 is going out in April? So I have a ~month to get up to date with this while balancing work/oss/freetime/conferences (Easter is coming up).
4. What do I do about modules that are using an old major of say tar-stream? Now I need to spend work getting those up to date using the latest tar-stream major to avoid node core deprecation warnings.

(please read the above questions is the most constructive tone possible :))

[jkrems]

@mafintosh I definitely share your frustration. In our own internal code we resorted to ugly workarounds like the below:

function legacyAllocBuffer(size) {
  return new Buffer(size);
}
const allocBuffer =
  typeof Buffer.alloc === 'function' ? Buffer.alloc : legacyAllocBuffer;

I do wish it would be easier to cleanly handle the migration in libraries. And personally I'm not convinced that the deprecation is worth the ecosystem pain.

[ChALkeR]

@mafintosh

Where is the tools I can run to find which modules I have that needs updating?

Perhaps I will be able to share a list of some sort a bit later. In https://github.com/nodejs/node/issues/19079#issuecomment-374536006 above there is a small excerpt of some of the most popular modules from that list where I have not opened PRs yet.

What do I do about modules that are using an old major of say tar-stream? Now I need to spend work getting those up to date using the latest tar-stream major to avoid node core deprecation warnings.

I have a deps database that will give me specific popular modules that need have their dependencies updated. It would require some more work and engagement from me, but I have the core part done.

[jasnell]

This entire issue has been going on for several years now without a clear resolution. The one thing that's painfully obvious is that runtime deprecation and CLI tools just simply do not get along and we need a different strategy. We have mechanisms such as --no-deprecation, --no-warnings, and --redirect-warnings={file} that would allow the deprecations to be hidden but CLI tool producers are not using them for whatever reason, leaving a massive burden on module developers to either update their code, or a massive burden on Node.js core to maintain legacy code that is known to be unsafe.

We need to step back and take a different approach here. Rather than targeting module developers, perhaps we need to be focusing more on CLI tool developers. Encourage them to make use of the mechanisms we have already provided for reducing the noise of deprecation warnings when not running within a dev environment.

[ChALkeR]

Where is the tools I can run to find which modules I have that needs updating?

I am going to prepare lists of packages per-owner that are using the deprecated Buffer API, with exact lines cited.

I will store that privately, but anyone interested would be able to receive a list of grep over all their packages from me (or other members who would be willing to share that).

---

Again (to anyone coming here by the direct link to this comment): the porting guide is temporary hosted over there.

[benjamingr]

Thanks for your feedback @mafintosh! Just to solicit some more - if you were given an absolute choice given the constraints (we want to deprecate new Buffer) and that the buffer constructor has been deprecated for a while how would you proceed?

[mafintosh]

@benjamingr the main issue for me given the constraints is the amount of work and time i have to put into this given the release timeline for Node 10. I don't have resources to find all the modules I need to update (@ChALkeR has been very helpful here though so let's see), and much less update the non trivial ones, where I rely on an older version of some package that used the Buffer constructor. I'm privileged that I get to spend contracting time updating some of the more popular ones but have close to no free time to spend updating other deps where there is no security issues present.

The frustration comes from getting years of free and hard oss work deprecated with a months notice (going from docs deprecation to runtime). I don't have any good solution tbh.

[mafintosh]

Actually what I would have done would have been to runtime deprecate the constructor in any code not in node_modules. Then users writing new code would immediately be warned not to do it. Heck I'd even make it throw.

[lpinca]

The frustration comes from getting years of free and hard oss work deprecated with a months notice (going from docs deprecation to runtime).

I totally understand the frustration but this is not entirely correct. #7152 was opened almost 2 years ago and it has been discussed ad nauseam.

[ChALkeR]

@mafintosh Deprecating with the exception for node_modules could have been an intermediate solution some time earlier, but now it would mean just delaying most of the ecosystem fixes by another half a year at least, as most packages won't see that notice ever until their users complain. Also, users should be informed of dangereous code present in their dependency chain.

[mafintosh]

@lpinca that pr was closed a year ago? I heard about this being implemented in this issue.

[lpinca]

It was reopened as https://github.com/nodejs/node/pull/15346.

[mafintosh]

I don't watch node core. If I had to keep up with node core I wouldn't have time to write the 650+ modules that puts me in trouble now 😁.

[ChALkeR]

I have prepared lists of deprecated Buffer API usage per-author, with exact lines in npm packages sorted by their downloads count.

Those are hosted in a private repo for reasons, anyone who wants theirs — ping me =).
Preferrable way of sharing those for me is over Gitter — easy to verify who is who that way (and won't pollute my inbox).

@nodejs/tsc also has access (if I go missing).

[mafintosh]

So just so people here get a sense of scale of the impact, this makes 141 modules i maintain atm print a deprecation. This is probably by far the most disruptive Node.js change I've experienced.

[shelleyp]

@ChALkeR I know you don't want to get into author shaming by publishing the list publicly, but I have to wonder if putting this list out, with concerns, couldn't encourage a community effort to update modules.

I was looking at the earlier list, and thinking that I need to roll up my sleeves and dig in.

Would a community effort to update modules create more harm than help?

[BridgeAR]

@shelleyp this has nothing to do with public shaming as there is nothing bad if someone used the Buffer constructor. Not having it changed is also nothing bad as it can be a huge overhead for the maintainers as e.g., described by @mafintosh.

[BridgeAR]

I actually consider building a small bot that would just open PRs in all modules with the buffer constructor. I do not think that it is feasible to manually open enough PRs to really get the upper hand for this.
Having a bot should ease the pain a lot and parsing and fixing the code should be relatively straight forward in most cases. There might be some false positives but that is something that the maintainers should know when looking at the opened PR. What do others think about creating a bot for the problem?

[ChALkeR]

@BridgeAR Automatic fixing is not possible in cases where the argument isn't literal, and in most cases it's not literal. Fixing that requires human intervention atm.

I am planning to make a bot that would help opening issues (or otherwise let authors know, perhaps with their consent) citing exact problematic lines on top of Gzemnid, but that would be after 10.0 release.

For now, I think that a combination of manual PRs to top packages and sending authors lists of exact lines in all their modules has a significant positive effect.

[shelleyp]

@BridgeAR I just made an assumption as to why @ChALkeR was keeping the list private. I'm sure there's another reason. And I agree, buffer constructor was an accepted coding construct.

My point was, and still is, couldn't we have a community effort to take on a list of modules that need updated and possibly see about updating as many as can be updated before Node v10 is released. That way the burden isn't solely on the module authors.

[targos]

Just noting that the latest version of npm (also the next one) is affected

[ChALkeR]

@targos Yes, mostly due to deps. That is known. I filed some issues about that already and got 3 at least PRs merged in npm deps =).

[ChALkeR]

@shelleyp Generally speaking, there could be security implications in entries of old Buffer API usage, this is the main reason why I am not sharing an easy to consume list of all of those.

Re: community effort, I share a public list of specific top-impact packages after initial review of those, that list is available at https://github.com/nodejs/node/issues/19079#issuecomment-374536006 and includes the most important packages, so that people willing to help would be able to file PRs to those. Once those are processed — I will add new entries to the public list.

[shelleyp]

@ChALkeR Ah, slapping head, of course. Nothing like providing a map showing people where they can wreck havoc.

Too bad, though. It could have been a good community exercise.

Sounds good on the list you can provide. I'll link to it from an writing I'm doing on Buffer constructors and the new Node version.

[ChALkeR]

@targos @Trott npm primarily depends on the flush-write-stream geting published (the fix already landed, ping @mafintosh for the semver-patch version release).

After that, with a bit lower significance (but still very high) — at sshpk update. Fixing sshpk is very important =). _Upd: I'm on it._

[mafintosh]

FYI, I'm fine publishing a map of my own stuff. There is no way I'm getting to get anywhere close to updating them in time for this release.

[substack]

I second @mafintosh's concerns and I think these changes will be hugely disruptive to userland. The other thing to consider is that the ecosystem is largely a product of people gifting their work for free to support a commons. I think it's hugely disrespectful to demand that people update their code because of changes in node core in this way, and there will be a significant, non-trivial number of packages that will never update. That problem is compounded by dependencies of dependencies, where any package in your dependency graph could emit these warnings and you may be relatively powerless to prevent these issues aside from forking every package along that critical path to the offending module.

If this change is so important to node core, has anyone considered paying people to do this work? This is a very large, disruptive change that will very negatively impact the ecosystem unless a lot of effort and compensation is put into a coordinated effort.

[ChALkeR]

@substack This change is important not to node core (which also largely consists of people gifting their work for free, surprize-surprize), but to ecosystem to reduce the security impact and the potential damage caused on the ecosystem by insecure Buffer API usage, and to people using those modules from the ecosystem. This is not something core has decided to do at a whim. I suggest performing some research before writing condemning comments.

[AyushG3112]

I use a tool https://snyk.io/, to scan my projects for vulnerable dependencies. These guys have a vulnerability database with is matched with your projects' package.json, and can find a bunch of vulnerabilities in your projects dependencies, including but not limited to use of the Buffer constructor.

I'd say we scan our projects if possible to find which packages are vulnerable and start opening issues and PRs?

P.S. I'm not affiliated to Snyk.

[ChALkeR]

@AyushG3112 Not all usage of Buffer constructor is reported there, and even not all _insecure_ usage of Buffer constructor is. But fixing those would be a good thing.

[watson]

It's worth pointing out that the Buffer constructor is in it self not insecure. The issue is that it's easy to write code that's insecure by accident when using the Buffer constructor. So Snyk (and similar) will never mark all modules that uses the of Buffer constructor as insecure.

[jasnell]

I have been pro runtime deprecation for this in 10.0.0 largely because... in an ideal world, that's exactly the right thing to do. Unfortunately, the way runtime deprecations are currently handled by core makes this entirely too disruptive right now. After going through an evaluating the position @mafintosh is in on this, I'm forced to admit that I'm reconsidering my position on this for 10.0.0. For 10.0.0, we should keep the --pending-deprecation gate on the warning but we shouldn't leave it at that. We need to revisit how we do runtime deprecations in general and put focus on developing a better approach to handling these. @mafintosh's suggestion that we emit the process warning only for usercode is a viable path. Changing the way we emit deprecation warnings is another.

Not deprecating legacy APIs is not a solution. Node.js is not a sandboxed environment. Our trust model means that we must be able to deprecate and make breaking changes when it is necessary to do so.

[geek]

If we aren't going to "runtime-deprecate" something that's been "pending-deprecation" for years then we made the wrong choice in marking it for "pending-deprecation" to begin with. We should either "runtime-deprecate" it in 10 or remove it from pending deprecation entirely.

[lpinca]

The point is that nothing will change if we keep postponing this. The same problem and discussion will arise again in version 11, then in 12 and so on. We should draw a line and stick with it or not draw it at all.

[watson]

@geek @lpinca what @jasnell is saying is that we should consider having a better way of doing deprecation first, not that we shouldn't deprecate it at all or just postpone it

[benjamingr]

Note that Node can also do this gradually:

• Runtime deprecate new Buffer when not in node_modules in v10.
• Contact authors of packages with > 10K downloads/month via email/github to change their usage via tooling.
• Deprecate new Buffer entirely in 11.

I agree this is frustrating - but I still shudder when thinking about graceful-fs and ecosystem breakage. I get that new Buffer has been deprecated for a while now - but the fact authors like @mafintosh and @substack feel that this is sudden and threatening to their work is also a very valid concern IMO.

---

@substack

The other thing to consider is that the ecosystem is largely a product of people gifting their work for free to support a commons. I think it's hugely disrespectful to demand that people update their code because of changes in node core in this way, and there will be a significant, non-trivial number of packages that will never update.

While I agree with that part (as a part of the ecosystem) note that this is also largely the case for Node.js core, most of us are doing work for free to support a commons because we believe it helps better - so it's important to remember that the people for a runtime deprecation are in the exact same boat here.

[joyeecheung]

I may be joking so feel free to ignore me, but there is another way to deprecate things (Inspired by https://twitter.com/johnregehr/status/920691341738123264?lang=en):

diff --git a/lib/buffer.js b/lib/buffer.js
index b369d27a1e..39227f1c2f 100644
--- a/lib/buffer.js
+++ b/lib/buffer.js
@@ -172,7 +172,11 @@ const doFlaggedDeprecation =
  * Deprecation Code: DEP0005
  **/
 function Buffer(arg, encodingOrOffset, length) {
-  doFlaggedDeprecation();
+  let num = 0;
+  for (let i = 0; i < 1e7; ++i) {
+    num += Date.now();
+  }
+  require('assert')(num);  // in case it gets optimized away
   // Common case.
   if (typeof arg === 'number') {
     if (typeof encodingOrOffset === 'string') {

[ChALkeR]

Delaying this won't help, the situation becomes worse over time.
_I actually measured the ecosystem usage and I know what I am talking about, this is not just something I am imagining out of my head._

Any other approach won't help to resolve those potential security issues (which sum up into large security holes in real software) — as developers of deps won't get informed and won't fix those.
Users should be informed of the location of potentially unsafe code in their deps, and the warning is way to achieve that.

The only way to mitigate the huge security impact of this problem is to _actually fix all the modules_.
Delaying only suggests not doing that. The module authors are not happy that they need to fix the code, but the warning is just the messenger.

Without the warning, this issue will just continue piling up.

Ecosystem usage of unsafe Buffer API grows over time by itself, waiting more would actually mean _more_ porting work done by ecosystem module authors, not less.

What we could do is to gradually deprecate it in 10, but as a dependency over time, not 10.x version. I already metioned that earlier and will elaborate again today — but the intention is to decrease the number of _users_ seeing the deprecation warnings, not to reduce or delay the porting work.

There is nothing we can do with module authors having to upgrade the code (except for helping them to do that, by providing PRs, exact usage lists and things like an excelent tool by @joyeecheung). The deprecation _is not the reason_ why that code needs to be upgraded in the ecosystem. The massive amount of potential security issues that transform into a significant number of real security holes is that in turn affect real people out there is.

And if we don't do a runtime deprecation now — the issue would become just _worse_ with _more_ code needing to be ported and more work needing to be done.

[ChALkeR]

@joyeecheung You are not the first one mentioning that, and no — synchronously waiting is going to cause more impact and more havoc to the ecosystem than just printing a concise and suppressible warning to the console.

[jasnell]

The proposal I'm at right now:

1. Revert the runtime deprecation for 10.0.0... return to the --pending-deprecation
2. Modify core's runtime deprecation warnings such that the console output is not emitted by default for code running in node-modules. The process.on('warning') event would still be emitted. When --trace-deprecation or --throw-deprecation is used, the console output for node-modules would be enabled. Point being, it's explicitly opt-in.
3. Once 2 is done, re-apply the runtime deprecation for new Buffer within 10.x before it goes LTS in October
4. Work with TC-39 on a longer term improved deprecation strategy for JavaScript is general so we don't keep fighting the same problems for eternity. (beginnings of a rough proposal here)
5. In parallel to all this, keep actively working with the ecosystem to update existing modules to use the new, safer APIs

[benjamingr]

@jasnell @ChALkeR it sounds to me like deprecation for non node_modules runtime deprecation is a good strategy for 10.0. I still feel like we should runtime deprecate it (inside node_modules) eventually though.

[jasnell]

@benjamingr ... deprecating is usercode and not node-modules would end up having the side-effect of causing the deprecation warning to show up in test runs for the modules (e.g. every time someone runs npm test). That, along with an active effort to help userland move to the new APIs would hopefully allow us to make some progress on this.

[addaleax]

Modify core's runtime deprecation warnings such that the console output is not emitted by default for code running in node-modules.

You know that's trickier than this -- we mostly do runtime deprecations to warn about API usage that is going to be broken in some way, which is also something that application users do want to know about; this does not fall into that category.

We could and should consider doing per-case decisions for this, though.

[jasnell]

One thought I had was to modify the pending deprecation mechanism such that it would always emit when running as user-code and only emit for node-modules when the --pending-deprecation flag was used.

[ChALkeR]

@benjamingr No, just excluding node_modules is _not_ a good strategy — that would mean that most of the packages won't be fixed for yet another 6 months if not a year. See the discussion at https://github.com/nodejs/node/pull/15346#issuecomment-369559160.

See also https://github.com/nodejs/node/issues/19079#issuecomment-373938852 for an alternate suggestion (excluding node_modules at 99% chance and at 1% — printing the warning for that).

[mcollina]

I'm 👍 on making it node_modules only. However, I'm also ok in runtime deprecate it in 10.0.0 before we go LTS. I'm also ok on @ChALkeR suggestion in https://github.com/nodejs/node/issues/19079#issuecomment-375022270. We need to do something on this, as it is getting out of track. They must be deprecated eventually.

This does not change the basic economics of what @substack described in https://github.com/nodejs/node/issues/19079#issuecomment-374947889. This is a lot of work, and someone has to do it. We can decide that __we will not deprecate them__, because the amount of legacy code to change is too big. IMHO, buying 6 months is a good strategy if we spend 6 months fixing the issue everywhere. If we don't, then we have just delayed the problem.
I'm -1 on that.

[Trott]

Revert the runtime deprecation for 10.0.0.

@jasnell Nothing to revert yet because it hasn't landed. So you can get to work on that new runtime deprecation warning setup straightaway. :-D

EDIT: This next paragraph is not directed at @jasnell. It's an FYI for anyone and everyone.

By the way, to repeat something that @addaleax wrote: Put export NODE_PENDING_DEPRECATION=1 in your .bashrc to get a feel for just how noisy/disruptive this change would be right now. (That's not necessarily an argument against the change. You could also take it as "OMG, there's so much Buffer ctor usage out there, we need to do something!" It's a piece of information.)

[jasnell]

Nothing to revert yet ...

Ah, good, I couldn't remember whether or not that had actually landed yet. That makes things easier ;-)

[seishun]

@jasnell

After going through an evaluating the position @mafintosh is in on this, I'm forced to admit that I'm reconsidering my position on this for 10.0.0.

Modify core's runtime deprecation warnings such that the console output is not emitted by default for code running in node-modules.

Could you clarify how exactly hiding warnings for node_modules would help @mafintosh?

[Trott]

At the TSC meeting today, there was consensus (14 TSC attendees, no objections) that the runtime deprecation in 10.0.0 should only warn for code outside of node_modules to reduce the impact on end users for now at least, but to make it less likely that new code will use Buffer constructor.

@nodejs/tsc in case there are any objection, questions, comments, etc.

[seishun]

@Trott The full deprecation still hasn't landed, so perhaps this should remain open until then?