Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backport important Buffer changes to v5.x #7169

Merged
merged 2 commits into from
Jun 19, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 67 additions & 17 deletions doc/api/buffer.md
Original file line number Diff line number Diff line change
Expand Up @@ -87,25 +87,27 @@ to one of these new APIs.*
containing a *copy* of the provided string.
* [`Buffer.alloc(size[, fill[, encoding]])`][buffer_alloc] returns a "filled"
`Buffer` instance of the specified size. This method can be significantly
slower than [`Buffer.allocUnsafe(size)`][buffer_allocunsafe] but ensures that
newly created `Buffer` instances never contain old and potentially sensitive
data.
* [`Buffer.allocUnsafe(size)`][buffer_allocunsafe] returns a new `Buffer` of
the specified `size` whose content *must* be initialized using either
[`buf.fill(0)`][] or written to completely.
slower than [`Buffer.allocUnsafe(size)`][buffer_allocunsafe] but ensures
that newly created `Buffer` instances never contain old and potentially
sensitive data.
* [`Buffer.allocUnsafe(size)`][buffer_allocunsafe] and
[`Buffer.allocUnsafeSlow(size)`][buffer_allocunsafeslow] each return a
new `Buffer` of the specified `size` whose content *must* be initialized
using either [`buf.fill(0)`][] or written to completely.

`Buffer` instances returned by `Buffer.allocUnsafe(size)` *may* be allocated
off a shared internal memory pool if the `size` is less than or equal to half
`Buffer.poolSize`.
off a shared internal memory pool if `size` is less than or equal to half
`Buffer.poolSize`. Instances returned by `Buffer.allocUnsafeSlow(size)` *never*
use the shared internal memory pool.

### What makes `Buffer.allocUnsafe(size)` "unsafe"?
### What makes `Buffer.allocUnsafe(size)` and `Buffer.allocUnsafeSlow(size)` "unsafe"?

When calling `Buffer.allocUnsafe()`, the segment of allocated memory is
*uninitialized* (it is not zeroed-out). While this design makes the allocation
of memory quite fast, the allocated segment of memory might contain old data
that is potentially sensitive. Using a `Buffer` created by
`Buffer.allocUnsafe(size)` without *completely* overwriting the memory can
allow this old data to be leaked when the `Buffer` memory is read.
When calling `Buffer.allocUnsafe()` (and `Buffer.allocUnsafeSlow()`), the
segment of allocated memory is *uninitialized* (it is not zeroed-out). While
this design makes the allocation of memory quite fast, the allocated segment of
memory might contain old data that is potentially sensitive. Using a `Buffer`
created by `Buffer.allocUnsafe()` without *completely* overwriting the memory
can allow this old data to be leaked when the `Buffer` memory is read.

While there are clear performance advantages to using `Buffer.allocUnsafe()`,
extra care *must* be taken in order to avoid introducing security
Expand Down Expand Up @@ -240,7 +242,8 @@ Additionally, the [`buf.values()`][], [`buf.keys()`][], and

Node.js can be started using the `--zero-fill-buffers` command line option to
force all newly allocated `Buffer` and `SlowBuffer` instances created using
either `new Buffer(size)` and `new SlowBuffer(size)` to be *automatically
either `new Buffer(size)`, `Buffer.allocUnsafe(size)`,
`Buffer.allocUnsafeSlow(size)` or `new SlowBuffer(size)` to be *automatically
zero-filled* upon creation. Use of this flag *changes the default behavior* of
these methods and *can have a significant impact* on performance. Use of the
`--zero-fill-buffers` option is recommended only when absolutely necessary to
Expand Down Expand Up @@ -449,6 +452,52 @@ Buffer pool if `size` is less than or equal to half `Buffer.poolSize`. The
difference is subtle but can be important when an application requires the
additional performance that `Buffer.allocUnsafe(size)` provides.

### Class Method: Buffer.allocUnsafeSlow(size)

* `size` {Number}

Allocates a new *non-zero-filled* and non-pooled `Buffer` of `size` bytes. The
`size` must be less than or equal to the value of
`require('buffer').kMaxLength` (on 64-bit architectures, `kMaxLength` is
`(2^31)-1`). Otherwise, a [`RangeError`][] is thrown. If a `size` less than 0
is specified, a zero-length `Buffer` will be created.

The underlying memory for `Buffer` instances created in this way is *not
initialized*. The contents of the newly created `Buffer` are unknown and
*may contain sensitive data*. Use [`buf.fill(0)`][] to initialize such
`Buffer` instances to zeroes.

When using `Buffer.allocUnsafe()` to allocate new `Buffer` instances,
allocations under 4KB are, by default, sliced from a single pre-allocated
`Buffer`. This allows applications to avoid the garbage collection overhead of
creating many individually allocated Buffers. This approach improves both
performance and memory usage by eliminating the need to track and cleanup as
many `Persistent` objects.

However, in the case where a developer may need to retain a small chunk of
memory from a pool for an indeterminate amount of time, it may be appropriate
to create an un-pooled Buffer instance using `Buffer.allocUnsafeSlow()` then
copy out the relevant bits.

```js
// need to keep around a few small chunks of memory
const store = [];

socket.on('readable', () => {
const data = socket.read();
// allocate for retained data
const sb = Buffer.allocUnsafeSlow(10);
// copy the data into the new allocation
data.copy(sb, 0, 0, 10);
store.push(sb);
});
```

Use of `Buffer.allocUnsafeSlow()` should be used only as a last resort *after*
a developer has observed undue memory retention in their applications.

A `TypeError` will be thrown if `size` is not a number.

### Class Method: Buffer.byteLength(string[, encoding])

* `string` {String | Buffer | TypedArray | DataView | ArrayBuffer}
Expand Down Expand Up @@ -1787,7 +1836,8 @@ console.log(buf);
[buffer_from_buffer]: #buffer_class_method_buffer_from_buffer
[buffer_from_arraybuf]: #buffer_class_method_buffer_from_arraybuffer_byteoffset_length
[buffer_from_string]: #buffer_class_method_buffer_from_str_encoding
[buffer_allocunsafe]: #buffer_class_method_buffer_allocraw_size
[buffer_allocunsafe]: #buffer_class_method_buffer_allocunsafe_size
[buffer_allocunsafeslow]: #buffer_class_method_buffer_allocunsafeslow_size
[buffer_alloc]: #buffer_class_method_buffer_alloc_size_fill_encoding
[`TypedArray.from()`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/TypedArray/from
[`DataView`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/DataView
Expand Down
14 changes: 14 additions & 0 deletions lib/buffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,20 @@ Buffer.allocUnsafe = function(size) {
return allocate(size);
};

/**
* Equivalent to SlowBuffer(num), by default creates a non-zero-filled
* Buffer instance that is not allocated off the pre-initialized pool.
* If `--zero-fill-buffers` is set, will zero-fill the buffer.
**/
Buffer.allocUnsafeSlow = function(size) {
if (typeof size !== 'number')
throw new TypeError('"size" argument must be a number');
return createBuffer(size, true);
};

// If --zero-fill-buffers command line argument is set, a zero-filled
// buffer is returned.

function SlowBuffer(length) {
if (+length != length)
length = 0;
Expand Down
6 changes: 6 additions & 0 deletions test/parallel/test-buffer-alloc.js
Original file line number Diff line number Diff line change
Expand Up @@ -1435,3 +1435,9 @@ assert.equal(SlowBuffer.prototype.offset, undefined);
assert.throws(function() {
Buffer.from(new ArrayBuffer(0), -1 >>> 0);
}, /RangeError: 'offset' is out of bounds/);

// Unpooled buffer (replaces SlowBuffer)
const ubuf = Buffer.allocUnsafeSlow(10);
assert(ubuf);
assert(ubuf.buffer);
assert.equal(ubuf.buffer.byteLength, 10);
23 changes: 23 additions & 0 deletions test/parallel/test-buffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -1449,3 +1449,26 @@ assert.equal(SlowBuffer.prototype.offset, undefined);
// Check pool offset after that by trying to write string into the pool.
assert.doesNotThrow(() => Buffer.from('abc'));
}


// Test failed or zero-sized Buffer allocations not affecting typed arrays
{
const zeroArray = new Uint32Array(10).fill(0);
const sizes = [1e10, 0, 0.1, -1, 'a', undefined, null, NaN];
const allocators = [
Buffer,
SlowBuffer,
Buffer.alloc,
Buffer.allocUnsafe,
Buffer.allocUnsafeSlow
];
for (const allocator of allocators) {
for (const size of sizes) {
try {
allocator(size);
} catch (e) {
assert.deepStrictEqual(new Uint32Array(10), zeroArray);
}
}
}
}