From a4065ebb7efaa18dbc8aae5360d982bd0ed3c89d Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sat, 2 Dec 2017 16:28:06 +0100 Subject: [PATCH 1/4] doc: warn against filling buffer with invalid data --- doc/api/buffer.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/doc/api/buffer.md b/doc/api/buffer.md index 3246e069302a23..0099aee0aa04cd 100644 --- a/doc/api/buffer.md +++ b/doc/api/buffer.md @@ -1254,6 +1254,19 @@ Example: Fill a `Buffer` with a two-byte character console.log(Buffer.allocUnsafe(3).fill('\u0222')); ``` +If `value` is contains invalid characters, it is truncated; if no valid +fill data remains, no filling is performed: + +```js +const buf = Buffer.allocUnsafe(5); +// Prints: +console.log(buf.fill('a')); +// Prints: +console.log(buf.fill('aazz', 'hex')); +// Prints: +console.log(buf.fill('zz', 'hex')); +``` + ### buf.includes(value[, byteOffset][, encoding]) * `size` {integer} The desired length of the new `Buffer`. diff --git a/lib/buffer.js b/lib/buffer.js index b56c032f9e829b..712f81a0bc4b43 100644 --- a/lib/buffer.js +++ b/lib/buffer.js @@ -27,7 +27,7 @@ const { compare: _compare, compareOffset, createFromString, - fill: _fill, + fill: bindingFill, indexOfBuffer, indexOfNumber, indexOfString, @@ -266,7 +266,9 @@ Buffer.alloc = function alloc(size, fill, encoding) { // be interpreted as a start offset. if (typeof encoding !== 'string') encoding = undefined; - return createUnsafeBuffer(size).fill(fill, encoding); + const ret = createUnsafeBuffer(size); + if (fill_(ret, fill, encoding)) + return ret; } return new FastBuffer(size); }; @@ -840,15 +842,20 @@ Buffer.prototype.includes = function includes(val, byteOffset, encoding) { // buffer.fill(buffer[, offset[, end]]) // buffer.fill(string[, offset[, end]][, encoding]) Buffer.prototype.fill = function fill(val, start, end, encoding) { + fill_(this, val, start, end, encoding); + return this; +}; + +function fill_(buf, val, start, end, encoding) { // Handle string cases: if (typeof val === 'string') { if (typeof start === 'string') { encoding = start; start = 0; - end = this.length; + end = buf.length; } else if (typeof end === 'string') { encoding = end; - end = this.length; + end = buf.length; } if (encoding !== undefined && typeof encoding !== 'string') { @@ -877,19 +884,17 @@ Buffer.prototype.fill = function fill(val, start, end, encoding) { } // Invalid ranges are not set to a default, so can range check early. - if (start < 0 || end > this.length) + if (start < 0 || end > buf.length) throw new errors.RangeError('ERR_INDEX_OUT_OF_RANGE'); if (end <= start) - return this; + return 0; start = start >>> 0; - end = end === undefined ? this.length : end >>> 0; + end = end === undefined ? buf.length : end >>> 0; - _fill(this, val, start, end, encoding); - - return this; -}; + return bindingFill(buf, val, start, end, encoding); +} Buffer.prototype.write = function write(string, offset, length, encoding) { diff --git a/src/node_buffer.cc b/src/node_buffer.cc index e8107607904985..ca2c6a89cb012a 100644 --- a/src/node_buffer.cc +++ b/src/node_buffer.cc @@ -591,6 +591,8 @@ void Fill(const FunctionCallbackInfo& args) { THROW_AND_RETURN_IF_OOB(start <= end); THROW_AND_RETURN_IF_OOB(fill_length + start <= ts_obj_length); + args.GetReturnValue().Set(static_cast(fill_length)); + // First check if Buffer has been passed. if (Buffer::HasInstance(args[1])) { SPREAD_BUFFER_ARG(args[1], fill_obj); @@ -612,8 +614,10 @@ void Fill(const FunctionCallbackInfo& args) { enc == UTF8 ? str_obj->Utf8Length() : enc == UCS2 ? str_obj->Length() * sizeof(uint16_t) : str_obj->Length(); - if (str_length == 0) + if (str_length == 0) { + args.GetReturnValue().Set(0); return; + } // Can't use StringBytes::Write() in all cases. For example if attempting // to write a two byte character into a one byte Buffer. @@ -643,7 +647,7 @@ void Fill(const FunctionCallbackInfo& args) { // TODO(trevnorris): Should this throw? Because of the string length was // greater than 0 but couldn't be written then the string was invalid. if (str_length == 0) - return; + return args.GetReturnValue().Set(0); } start_fill: diff --git a/test/parallel/test-buffer-alloc.js b/test/parallel/test-buffer-alloc.js index 15a6b9ebc67510..73dfce3624d9f8 100644 --- a/test/parallel/test-buffer-alloc.js +++ b/test/parallel/test-buffer-alloc.js @@ -1005,3 +1005,14 @@ assert.strictEqual(Buffer.prototype.toLocaleString, Buffer.prototype.toString); const buf = Buffer.from('test'); assert.strictEqual(buf.toLocaleString(), buf.toString()); } + +{ + // Ref: https://github.com/nodejs/node/issues/17423 + const buf = Buffer.alloc(0x1000, 'This is not correctly encoded', 'hex'); + assert(buf.every((byte) => byte === 0), `Buffer was not zeroed out: ${buf}`); +} + +{ + const buf = Buffer.alloc(0x1000, 'c', 'hex'); + assert(buf.every((byte) => byte === 0), `Buffer was not zeroed out: ${buf}`); +} diff --git a/test/parallel/test-buffer-fill.js b/test/parallel/test-buffer-fill.js index 63db4ce13c8c4b..681207e95ef382 100644 --- a/test/parallel/test-buffer-fill.js +++ b/test/parallel/test-buffer-fill.js @@ -468,3 +468,9 @@ assert.strictEqual( assert.strictEqual( Buffer.allocUnsafeSlow(16).fill('Љ', 'utf8').toString('utf8'), 'Љ'.repeat(8)); + +{ + const buf = Buffer.from('a'.repeat(1000)); + buf.fill('This is not correctly encoded', 'hex'); + assert.strictEqual(buf.toString(), 'a'.repeat(1000)); +} From fcb1524655700e1976a3e94f07d02b5f3bb82627 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sat, 2 Dec 2017 16:52:21 +0100 Subject: [PATCH 3/4] [squash] apapirovski nit --- doc/api/buffer.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/api/buffer.md b/doc/api/buffer.md index 6b79a8d1f53087..2d47d2e20e09c9 100644 --- a/doc/api/buffer.md +++ b/doc/api/buffer.md @@ -513,7 +513,7 @@ added: v5.10.0 changes: - version: REPLACEME pr-url: https://github.com/nodejs/node/pull/17428 - description: Specifying an invalid string for `fill` now yields a + description: Specifying an invalid string for `fill` now results in a zero-filled buffer. --> From b9b4d29eba129d5fe909a6d1c70bec850e7db683 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sat, 2 Dec 2017 16:52:36 +0100 Subject: [PATCH 4/4] [squash into first commit] apapirovski nit --- doc/api/buffer.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/api/buffer.md b/doc/api/buffer.md index 2d47d2e20e09c9..b5c29da0ae1e52 100644 --- a/doc/api/buffer.md +++ b/doc/api/buffer.md @@ -1259,7 +1259,7 @@ Example: Fill a `Buffer` with a two-byte character console.log(Buffer.allocUnsafe(3).fill('\u0222')); ``` -If `value` is contains invalid characters, it is truncated; if no valid +If `value` contains invalid characters, it is truncated; if no valid fill data remains, no filling is performed: ```js