buffer: zero-fill buffer allocated with invalid content

Zero-fill when `Buffer.alloc()` receives invalid fill data.

A solution like https://github.com/nodejs/node/pull/17427 which switches
to throwing makes sense, but is likely a breaking change.

This suggestion leaves the behaviour of `buffer.fill()` untouched,
since any change to it would be a breaking change, and lets
`Buffer.alloc()` check whether any filling took place or not.

PR-URL: https://github.com/nodejs/node/pull/17428
Refs: https://github.com/nodejs/node/pull/17427
Refs: https://github.com/nodejs/node/issues/17423
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>

1 files changed, 16 insertions, 11 deletions

diff --git a/lib/buffer.js b/lib/buffer.js
index b56c032f9e..8899d260ad 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) > 0)
+      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) {