buffer: add Buffer.from(), Buffer.alloc() and Buffer.allocUnsafe(), soft-deprecate Buffer(num) by jasnell · Pull Request #4682 · nodejs/node · GitHub

It soft deprecates Buffer()
It adds Buffer.allocUnsafe(size) as a direct replacement for Buffer(number)
It adds Buffer.alloc(size[, fill[, encoding]]) to create a new initialized Buffer.
It adds Buffer.from(data[, encoding) as a direct replacement for Buffer(data[, encoding]) (for every variant)
It adds a --zero-fill-buffers command line flag that forces Buffer(number), Buffer.alloc(), and SlowBuffer(number) to zero fill the initialized Buffers.
It adds documentation, test cases and new benchmarks for each of these.
It modifies every existing call to Buffer() currently in core in order to demonstrate the extent of the changes required by these edits. It's not a small change by any measure.

Note: PR description updated to reflect new naming and implementation details

+1 on the current approach, but can we please find another name? safe and unsafe might be immediately clear in the context of the current issue, but imagine someone who is unfamiliar reading the docs. How about something like

Buffer.allocate(num[, unsafe])

where unsafe is a boolean. Maybe even find a better name for the argument.

-1 on Buffer.allocate(num[, unsafe])

Part of the motivation for this change is to make the choice between using
a prefilled buffer or not more explicit and obvious. We don't achieve that
using a single method with an optional Boolean.

We can bikeshed the names but safe and unsafe are concise and to the
point.
On Jan 13, 2016 9:57 PM, "silverwind" notifications@github.com wrote:

+1 on the approach, but can we please find another name? safe and unsafe
might be immediately clear in the context of the current issue, but imagine
someone fresh reading the docs. How about something like

Buffer.allocate(num[, unsafe])

where unsafe is a boolean.

—
Reply to this email directly or view it on GitHub
#4682 (comment).

@trevnorris @ChALkeR @Fishrock123

@jasnell I might miss the point of this a bit, but just to be sure:

This commit features hard deprecation and does not touch the docs at all. The proposed solution was soft (doc only) deprecation at first.
The deprecation message tells people to use «Buffer.safe or Buffer.unsafe» with no warnings against the latter. The latter should be either removed from this notice or reworded, see Buffer(number) is unsafe #4660 (comment) and Buffer(number) is unsafe #4660 (comment).
Buffer.safe(number) probably needs new tests, including those that make sure that it's zero-filled.

Just a quick note/question: with this approach a safe Buffer would be basically built with malloc + memset, right? (I'm looking here)

In order to make safe buffers faster it might be a good idea to use calloc, since the zeroing could essentially be free in some cases. Repeating the benchmarks @mscdex did afterwards would be nice.

Related question: is there a reason why the ArrayBuffer here gets the data allocated right above instead of just passing the size and letting it use its constructor which would already use calloc if the zero-fill flag was set? (see here).

PS: I just glanced throught the code out of curiosity, I might be totally off-base here. Sorry for the noise if that is the case.

@silverwind
-1 on Buffer.allocate(num[, unsafe]).

Several reasons:

That is a bit harder to grep and to automatically find all the places where you (or someone else) is using unitialized Buffers.
That flag changes the behaviour of a function in an unobvious way (I have seen people failing to understand all impications of using unitialized buffers).
The names for these functions should better be self-describing.
Merging two different methods with different behaviour into a one that changes it's behaviour depending on a flag isn't a good design move.
(Pretty important). The documentation on those methods should be separate, and it's very important. While Buffer.safe could be documented as usually, Buffer.unsafe documentation should describe all the dangers of using unitialized Buffers.

Once more notice: as it looks now, Buffer.safe completely ignores the pooled behaviour. What impications would that have on performance of Buffer.safe(number) vs Buffer.unsafe(number).fill(0)?

/cc @trevnorris

if someone wants to quickly do some benchmarks for us, you can grab N|Solid and run the buffer benchmarks. It's based on the latest v4.x and comes with a "policies" feature that you can use to zero-fill by default—it switches out the malloc with a calloc when enabled, see: https://docs.nodesource.com/nsolid/1.2/docs/policies. The performance profile for this would be very close to what we'd end up with by doing this by default in core because it's basically the same thing.

@ChALkeR ... This PR is still a work in process so some pieces are still missing or not quite complete. It's intended to facilitate more constructive conversation about what changes we need to make. I fully intend to iterate on it more and add the necessary docs.

@saghul ... with the approach currently implemented by this PR, calling Buffer.safe() allocates a new Buffer using calloc (see https://github.com/nodejs/node/pull/4682/files#diff-196d056a936b6d2649721eb639e0442bR90). Calling createBuffer() with flags[kNoZeroFill] set to 0 will cause the ArrayBufferAllocator::Allocate() function in node.cc to use calloc(size,1) as opposed to malloc.

Currently, when Node starts up, a large buffer pool is created. New Buffer instances created using Buffer(num) are allocated off that pool and the memory is NOT zeroed out each time. Calling Buffer.safe() bypasses this pool and ensures that a new zero-filled allocation is created each time. You don't get the performance benefits of the pool but it's safer overall. And calling calloc should be a bit faster than calling fill(0) after the memory is already allocated, but that needs to be benchmarked (I'm sure @trevnorris could say for sure).

I'll be looking for ways of making this more efficient but given that the intent is to provide a more obvious way of creating "safe" buffers, going straight to calloc each time likely makes the most sense. We'll see tho.

@jasnell ah, I see now, thanks!

Looking at the implementation for the --safe-buffers command line flag now... there are a couple of design questions. The intent of --safe-buffers is to force all newly created Buffers to be zero filled automatically. Given that, there are three approaches I can take with this:

Still use the slab buffer pool and fill(0) all Buffers after they are created. This works ok for smallish Buffers but could have significant performance impact on large buffers.
Skip the slab buffer pool and simply calloc all new Buffers. This is more efficient than calling fill(0) but means we don't enjoy the benefits of the slab allocated pool.
Use a hybrid approach... for Buffers under a certain size, we allocate off the pool and fill(0), but for Buffers over that size we fall back to calloc.

Regardless of the option chosen, --safe-buffers is going to cause a significant performance degradation, it's just a matter of figuring out how to minimize it.

@jasnell Btw, about --safe-buffers — I don't like that name. Buffers are generally safe, careless (or accidential) usage of Buffer(number) isn't. Also, that flag probably should be there only until Buffer(number) is hard deprecated (or removed). Perhaps giving it a more explicit name, like --safe-buffer-from-number or --safe-buffer-number would be better.

@jasnell About the Buffer(number) speed under the flag:

Buffer.safe(number) should try best to be fast with various number values. That could involve calloc-ing for all number values, or callocing for large number values and using a pooled approach (like Buffer.unsafe(number).fill(0)) for smaller number values, whichever is faster.
Buffer(number) under the --safe-buffer-number flag (or however it would be called) should behave exactly like Buffer.safe(number) and reuse that code. That will guarantee that it's as fast as Buffer(number) is and minimize the codebase, lowering the probability of introducing bugs.

@trevnorris @silverwind @saghul @ChALkeR ... ok, pushed an update:

adds documentation
adds SlowBuffer.safe() and SlowBuffer.unsafe() also, deprecates SlowBuffer(size)
changes the impl of Buffer.safe(len) to simply call Buffer.unsafe(len).fill(0), benchmarks are showing that it's still faster than doing the calloc directly every time.
expands benchmark/buffers/buffer-creation.js to include the safe and unsafe options and larger buffer sizes
updates test cases and internal code bits to use SlowBuffer.unsafe.

Still todo: add the --safe-buffer command line flag... per @ChALkeR's comments I'll likely rename it to --zero-fill-buffers

This is moving a bit quick, but some general things:

If we are going to do this, Buffer(number) should be deprecated, probably first only in docs. I prefer runtime deprecations, since you can ignore them --no-deprecation, but I understand some users don't.
safe() and unsafe() are not actually that descriptive. These sound like you are turning something on or off. I suggest going for allocSafe() and allocUnsafe().
--zero-fill-buffers is a better naming, yes.

You mentioned the pooling aspect. Would it be worthwhile to have a safe pool, or does that defeat the purpose?

@jasnell Could you make sure to split up these commits, one replacing usage in core should be separate as it is quite large and makes this difficult to review.

@Fishrock123 ... I have absolutely no intention of landing this any time soon so there shouldn't be a worry about it moving too quickly :-) ... the intent right now is to get concrete options in the form of working code on the table for discussion as opposed to going around and around discussing whether or not it's a bug or not.

oh, another point, having a safe pool likely isn't worthwhile because you'd end up having to zero fill every time anyway to reset from the last time :-) An argument could be made that if --zero-fill-buffers is set, the pool should be zero filled on creation but given that we end up having to zero fill later anyway, it's a bit redundant and wouldn't buy us anything.

Also, the current implementation of Buffer.safe() simply does a Buffer.unsafe().fill(), which shouldn't be more efficient that simply doing a calloc but benchmarks are showing that it is... which baffles me a bit. I'd like to get that figured out.

@Fishrock123 ... I reswizzled the commits to make it easier to review.

citgm: https://ci.nodejs.org/job/thealphanerd-smoker/41/

Pushed a new commit to use the names zalloc() and alloc() instead

(What is zalloc?)

zalloc == 'zeroed-allocation'

The following significant (semver-major) changes have been made since the previous Node v5.0.0 release. * Buffer * New Buffer constructors have been added [#4682](#4682) * Previously deprecated Buffer APIs are removed [#5048](#5048), [#4594](#4594) * Improved error handling [#4514](#4514) * Cluster * Worker emitted as first argument in 'message' event [#5361](#5361). * Crypto * Improved error handling [#3100](#3100), [#5611](#5611) * Simplified Certificate class bindings [#5382](#5382) * Improved control over FIPS mode [#5181](#5181) * pbkdf2 digest overloading is deprecated [#4047](#4047) * Dependencies * Reintroduce shared c-ares build support [#5775](#5775). * V8 updated to 5.0.71.31 [#6111](#6111). * DNS * Add resolvePtr API to query plain DNS PTR records [#4921](#4921). * Domains * Clear stack when no error handler [#4659](#4659). * File System * The `fs.realpath()` and `fs.realpathSync()` methods have been updated to use a more efficient libuv implementation. This change includes the removal of the `cache` argument and the method can throw new errors [#3594](#3594) * FS apis can now accept and return paths as Buffers [#5616](#5616). * Error handling and type checking improvements [#5616](#5616), [#5590](#5590), [#4518](#4518), [#3917](#3917). * fs.read's string interface is deprecated [#4525](#4525) * HTTP * 'clientError' can now be used to return custom errors from an HTTP server [#4557](#4557). * Modules * Current directory is now prioritized for local lookups [#5689](#5689) * Symbolic links are preserved when requiring modules [#5950](#5950) * Net * DNS hints no longer implicitly set [#6021](#6021). * Improved error handling and type checking [#5981](#5981), [#5733](#5733), [#2904](#2904) * OS X * MACOSX_DEPLOYMENT_TARGET has been bumped up to 10.7 [#6402](#6402). * Path * Improved type checking [#5348](#5348). * Process * Introduce process warnings API [#4782](#4782). * Throw exception when non-function passed to nextTick [#3860](#3860). * Readline * Emit key info unconditionally [#6024](#6024) * REPL * Assignment to `_` will emit a warning. [#5535](#5535) * Timers * Fail early when callback is not a function [#4362](#4362) * TLS * Rename 'clientError' to 'tlsClientError' [#4557](#4557) * SHA1 used for sessionIdContext [#3866](#3866) * TTY * Previously deprecated setRawMode wrapper is removed [#2528](#2528). * Util * Changes to Error object formatting [#4582](#4582). * Windows * Windows XP and Vista are no longer supported [#5167](#5167), [#5167](#5167).

This backports the new `Buffer.alloc()`, `Buffer.allocUnsafe()`, `Buffer.from()`, and `Buffer.allocUnsafeSlow()` APIs for v4. Some backported tests are disabled, but those are not related to the new API. Note that `Buffer.from(arrayBuffer[, byteOffset [, length]])` is not supported in v4.x, only `Buffer.from(arrayBuffer)` is. Refs: nodejs#4682 Refs: nodejs#5833 Refs: nodejs#7475 PR-URL: nodejs#7562 Reviewed-By: Trevor Norris <trev.norris@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Nikolai Vavilov <vvnicholas@gmail.com>

This backports the new `Buffer.alloc()`, `Buffer.allocUnsafe()`, `Buffer.from()`, and `Buffer.allocUnsafeSlow()` APIs for v4. Some backported tests are disabled, but those are not related to the new API. Note that `Buffer.from(arrayBuffer[, byteOffset [, length]])` is not supported in v4.x, only `Buffer.from(arrayBuffer)` is. Refs: #4682 Refs: #5833 Refs: #7475 PR-URL: #7562 Reviewed-By: Trevor Norris <trev.norris@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Nikolai Vavilov <vvnicholas@gmail.com>

Just realized the Buffer.from is defined in Node 4.x and it cannot be overwritten without Object.defineProperty call. Makes it a tad annoying for Node 4.x polyfills.

Just thought I'd mention it since new Buffer is technically deprecated now and many users are still using Node 4.x.

@bminer it is defined because we backported the function. No need to polyfill afaik

Right, but I have systems running 6.x and Node 4.2.2, for example.

So, I have to do this to use Buffer.from everywhere:

if(parseInt(process.versions.node.split(".")[0]) < 6) {
    Object.defineProperty(Buffer, "from", {"value": function() {
        return Buffer.apply(this, arguments);
    } });
}

is there a particular reason you are running on v4. 2.2? this is an
understatement supported and insecure version of v4

On Thu, Oct 6, 2016, 8:26 PM Blake Miner notifications@github.com wrote:

Right, but I have systems running 6.x and Node 4.2.2, for example.

So, I have to do this to use Buffer.from everywhere:

if(parseInt(process.versions.node.split(".")[0]) < 6) {
Object.defineProperty(Buffer, "from", {"value": function() {
return Buffer.apply(this, arguments);
} });
}

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#4682 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAecV7Iyx0ZkNw5MllbyrNIY49E8VUgaks5qxZGqgaJpZM4HEl70
.

@thealphanerd - Good point. I should probably upgrade to latest 4.x. Just ignore me. :)

Post-mortem of https://github.com/nodejs-private/security/issues/202 and #18790 — this comment was not effective as a guard against safeguards being removed in a follow-up commit.
A testcase would have served better.
Upd: filed #22492.

jasnell added wip Issues and PRs that are still a work in progress. buffer Issues and PRs related to the buffer subsystem. semver-major PRs that contain breaking changes and should be released in the next major version. labels Jan 14, 2016

jasnell mentioned this pull request Jan 14, 2016

Buffer(number) is unsafe #4660

Closed

jasnell force-pushed the buffer-safe branch from 0175bd6 to f7e0051 Compare January 14, 2016 17:50

jasnell force-pushed the buffer-safe branch from 689c14a to ecba645 Compare January 14, 2016 18:59

jasnell changed the title buffer: add Buffer.safe() and Buffer.unsafe(), deprecate Buffer(num) buffer: add Buffer.zalloc() and Buffer.alloc(), deprecate Buffer(num) Jan 14, 2016

vitaly-t mentioned this pull request Apr 27, 2016

Doesn't work under Node.js 6.0.0 brianc/node-postgres#1000

Closed

thisconnect mentioned this pull request May 11, 2016

use Buffer.from? thisconnect/fildes#22

Closed

ChALkeR mentioned this pull request Aug 18, 2016

buffer: runtime-deprecate Buffer constructor #7152

Closed

2 tasks

gluons referenced this pull request in gluons/gulp-json2cson Nov 22, 2016

Use 'Buffer.from'. Yarn.

a29366a

seishun mentioned this pull request Sep 11, 2017

buffer: runtime-deprecate Buffer ctor by default #15346

Closed

4 tasks

seishun mentioned this pull request Jun 15, 2018

buffer: runtime-deprecate Buffer constructor everywhere by default #21351

Closed

4 tasks

ChALkeR reviewed Aug 23, 2018

View reviewed changes

ChALkeR mentioned this pull request Aug 23, 2018

Add testcases for all documented safeguards #22492

Closed

ChALkeR mentioned this pull request Oct 29, 2025

doc: replace allocUnsafe with alloc in examples #60477

Open

ChALkeR mentioned this pull request Jan 13, 2026

allocUnsafe is useless since 24 #60423

Closed

+                  flags[kNoZeroFill] = 1;
+                  // Only pay attention to encoding if it's a string. This
+                  // prevents accidentally sending in a number that would
+                  // be interpretted as a start offset.

buffer: add Buffer.from(), Buffer.alloc() and Buffer.allocUnsafe(), soft-deprecate Buffer(num) by jasnell · Pull Request #4682 · nodejs/node · GitHub

Uh oh!

Uh oh!

buffer: add Buffer.from(), Buffer.alloc() and Buffer.allocUnsafe(), soft-deprecate Buffer(num) - #4682

buffer: add Buffer.from(), Buffer.alloc() and Buffer.allocUnsafe(), soft-deprecate Buffer(num)#4682jasnell wants to merge 1 commit intonodejs:masternodejs/node:masterfrom jasnell:buffer-safejasnell/node:buffer-safeCopy head branch name to clipboard

Conversation

jasnell commented Jan 14, 2016

Uh oh!

silverwind commented Jan 14, 2016

Uh oh!

jasnell commented Jan 14, 2016

Uh oh!

jasnell commented Jan 14, 2016

Uh oh!

ChALkeR commented Jan 14, 2016

Uh oh!

saghul commented Jan 14, 2016

Uh oh!

ChALkeR commented Jan 14, 2016

Uh oh!

ChALkeR commented Jan 14, 2016

Uh oh!

rvagg commented Jan 14, 2016

Uh oh!

jasnell commented Jan 14, 2016

Uh oh!

jasnell commented Jan 14, 2016

Uh oh!

saghul commented Jan 14, 2016

Uh oh!

jasnell commented Jan 14, 2016

Uh oh!

ChALkeR commented Jan 14, 2016

Uh oh!

ChALkeR commented Jan 14, 2016

Uh oh!

jasnell commented Jan 14, 2016

Uh oh!

Fishrock123 commented Jan 14, 2016

Uh oh!

jasnell commented Jan 14, 2016

Uh oh!

jasnell commented Jan 14, 2016

Uh oh!

jasnell commented Jan 14, 2016

Uh oh!

MylesBorins commented Jan 14, 2016

Uh oh!

jasnell commented Jan 14, 2016

Uh oh!

Fishrock123 commented Jan 14, 2016

Uh oh!

jasnell commented Jan 14, 2016

Uh oh!

bminer commented Oct 7, 2016

Uh oh!

MylesBorins commented Oct 7, 2016

Uh oh!

bminer commented Oct 7, 2016

Uh oh!

MylesBorins commented Oct 7, 2016

Uh oh!

bminer commented Oct 19, 2016

Uh oh!

ChALkeR Aug 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

20 participants

buffer: add Buffer.from(), Buffer.alloc() and Buffer.allocUnsafe(), soft-deprecate Buffer(num)#4682
jasnell wants to merge 1 commit into
nodejs:masterfrom
jasnell:buffer-safe

ChALkeR Aug 23, 2018 •
edited
Loading