buffer: runtime-deprecate Buffer constructor everywhere by default
Creating a new PR per the vote in https://github.com/nodejs/node/pull/15346#issuecomment-396076307.
Some backstory:
- In Node.js 6.0.0, new Buffer APIs (
Buffer.from()
,Buffer.allocUnsafe()
andBuffer.alloc()
) were introduced to address security concerns of the Buffer constructor, which was deprecated in the docs in the same version. (see https://github.com/nodejs/node/pull/4682, https://github.com/nodejs/node/issues/4660) - In Node.js 8.0.0, a "pending deprecation" for the Buffer constructor was introduced. When Node.js is launched with the
--pending-deprecation
flag, the use ofBuffer()
andnew Buffer()
causes aDeprecationWarning
to be emitted. (see https://github.com/nodejs/node/pull/11968) - It was proposed to revisit this and possibly move the warning from behind the flag in Node.js 9.0.0. (see https://github.com/nodejs/node/issues/9531#issuecomment-285772835, https://github.com/nodejs/node/issues/9531#issuecomment-291046546).
- In Node.js 10.0.0, unconditional runtime deprecation for the Buffer constructor was introduced for code outside of
node_modules
, with a plan to enable it for all code in a "later release". (see https://github.com/nodejs/node/pull/19524#issuecomment-375150929).
Several reasons for enabling runtime deprecation by default, in no particular order:
- Potentially prevents possible DoS vulnerabilities caused by accidentally calling
Buffer(num)
instead ofBuffer(string)
due to insufficient input validation. -
Buffer(num)
zero-fills by default since 8.0.0, preventing accidental data disclosure. (see https://github.com/nodejs/node/pull/12141) However, it was not backported to earlier Node.js versions, which could result in security issues if new code is written with the assumption of zero-filling and then run on Node.js versions that don't zero-fill. (this and above is described in more detail here) - Some developers might be unaware that
Buffer(num)
zero-fills and still be using it in performance-critical code, whereBuffer.allocUnsafe()
might be more appropriate. - Calling
Buffer()
withoutnew
would be runtime-deprecated as well, so this functionality could potentially be removed in a later Node.js version. This would make it possible to refactorBuffer
into a proper ES6 class that can be subclassed. - Following "pending deprecation" with actual runtime deprecation will create a precedent that will cause more people to use the
--pending-deprecation
flag to test their code in the future. This will in turn allow us to introduce new runtime deprecations more smoothly by going through the "behind-the-flag" stage first. - Eventually, most code will use the new Buffer API, which will make it less likely that people new to Node.js will ever need to learn about the deprecated API.
Reasons why runtime deprecation only outside of node_modules
is insufficient:
- Many modules are not actively tested on the latest Node.js versions, which means the problematic code in them will never be fixed, since no users will see and report the warning.
- Many applications and modules depend on old module versions, which means they'll continue running with problematic code even if all dependencies have been fixed long ago.
Checklist
-
make -j4 test
(UNIX), orvcbuild test
(Windows) passes -
tests and/or benchmarks are included -
documentation is changed or added -
commit message follows commit guidelines