Node: Extending Buffer constructor API

[Trott]

Opening a can of worms, I suspect, or else rehashing an idea that's already been considered and rejected, but hoping not....

Is your feature request related to a problem? Please describe.

Buffer constructor has been deprecated since v6.0.0. Tons of code in the ecosystem still uses it though and that's seemingly never going to change. In an attempt to support the new APIs in both old and new versions of Node.js, packages like https://www.npmjs.com/package/safe-buffer have been introduced. But there's still a ton of Buffer constructor stuff out there.

Describe the solution you'd like
Please describe the desired behavior.

In current versions of Node.js, new Buffer(num) is zero-filled. So, the issue with revealing leftover memory contents is mitigated. However, there are still issues around getting an unexpected input type (e.g., number when you expect a string). With the newer APIs, Buffer.from() cannot get a number and Buffer.alloc() cannot get a string. A TypeError is thrown if that happens.

Instead of trying to get older packages to use safe-buffer, which means adding a dependency and rewriting some code that may be using the Buffer constructor in a perfectly safe way, it may be more palatable to add an options argument to Buffer constructor allowing the programmer to identify the expected type. So, for example, new Buffer(50, { expect: 'number' }); would be fine, but new Buffer('abc', { expect: 'number' }); would throw. (Obviously, there's bike-shedding to be done on the property name and accepted values. I'm just trying to verify the basic concept with this issue.)

The main advantage this has to other approaches possible now is that it can be added to code that runs old versions of Node.js harmlessly. The option will be ignored in that case. No need to add a dependency or change any other lines of code. I'm (naively?) optimistic that some package maintainers who have been resistant to safe-buffer or simply moving to Buffer.from()/Buffer.alloc() might look more positively on changing their code to use the option on the grounds that it improves security for people running newer versions of Node.js without impacting older versions (the way a switch to Buffer.from()/Buffer.alloc() would), without adding dependencies they didn't need before, and so on. It's purely additive and a security win. Sure, people running Node.js 0.10.x won't get any additional protection but they'll be no worse off and it should be easier with this for module maintainers to use the new API feature while maintaining backward compatibility.

Biggest disadvantage I can think of now is more API surface-area to maintain and multiple ways to do the same thing. FWIW, I guess if I could go back in time, this is probably what I'd propose instead of Buffer.from()/Buffer.alloc(). Was this already considered and rejected at some point? Am I being too optimistic?

Would be interested in opinions from prolific module maintainers who were greatly inconvenienced when Node.js 7.0.0 went out with a runtime deprecation warning for Buffer constructor. I'm not proposing putting a deprecation like that back here, but I want to know if they'd be more likely to accept PRs with this API than they are for PRs that use safe-buffer or PRs that switch to Buffer.from()/Buffer.alloc(). Or not really because some folks have scores or even hundreds of modules that might be subject to these kinds of updates and merging+publishing would be time-consuming and people have other priorities? /ping @substack @mafintosh @feross (who, yes, I know writes/maintains safe-buffer)

Describe alternatives you've considered
Please describe alternative solutions or features you have considered.

Doing nothing is the primary alternative.

[Trott]

If we go this route, the idea is that the Buffer() constructor would not emit a deprecation warning if it is invoked with the type-specifier in the options object.

[Trott]

Also: Buffer.from() and Buffer.alloc() (and Buffer.allocUnsafe()) would still be the recommended/preferred way to do things. This is trying to create a better path for cases where migrating to those APIs is not going to happen (whatever the reason: support needs, maintainer choice, or something else).

[mscdex]

It seems like if people are going to have to make changes to their code to support this, they may as well just add a typeof value check instead.

[Trott]

It seems like if people are going to have to make changes to their code to support this, they may as well just add a typeof value check instead.

I wish that didn't make as much sense as it does but...yes, that's a valid point.

[Trott]

I'm going to close this, but if anyone thinks it's worth discussing, by all means, re-open or comment.

[Trott]

The one advantage of this over adding a typeof check is that it avoids a deprecated API (and thus a runtime deprecation warning when run outside of node_modules) while maintaining backward-compatibility with older versions of Node.js.

[Trott]

The one advantage of this over adding a typeof check is that it avoids a deprecated API (and thus a runtime deprecation warning when run outside of node_modules) while maintaining backward-compatibility with older versions of Node.js.

Actually, I"ll leave this open a little while longer in case that's deemed a compelling argument by anyone. I find it somewhat compelling, but of course, I would....

[targos]

Is there evidence that the reason for people still not updating to the recommended methods is backward-compatibility?

[GrosSacASac]

In my opinion we should not optimize for legacy versions, it is already hard to convince everyone to move forward as is.

[sam-github]

I have the impression that anyone who is willing to update their source is willing to use the new alloc/from APIs, and its packages that are not actively maintained that are not able to change, so having another target to change code _to_ does not help.

But maybe we'll hear from some folks here. Unfortunately, its unlikely that package authors that don't have time to update their packages are actively on the nodejs/node issue tracker, but we can try!

[feross]

I haven't encountered much resistance to PRs that update code to use Buffer.from and Buffer.alloc. Every one I've sent has been accepted. Furthermore, most of the actively maintained packages that I use have already dropped support for Node 6, and will likely drop Node 8 over the next year (considering it's EOL in less than 3 months). Buffer.from and Buffer.alloc were added in Node 5, right?

I personally don't think the additional API surface area is a cost worth paying to make it (debatably) slightly easier to stop using new Buffer. In fact, I now encourage folks to just use Buffer.from and Buffer.alloc, etc. directly rather than safe-buffer these days anyway.

[Trott]

(Aside: I should have really run this by a few people privately before posting here. Would have saved folks some time reading a proposal that wasn't going to go anywhere anyway.)

Closing. Thanks, everyone!