fix(node/buffer): utf8ToBytes should return a Uint8Array (#20769)

9 files changed, 67 insertions, 120 deletions

diff --git a/cli/tests/node_compat/config.jsonc b/cli/tests/node_compat/config.jsonc
index d1ad44f41..8a31f4dc2 100644
--- a/cli/tests/node_compat/config.jsonc
+++ b/cli/tests/node_compat/config.jsonc
@@ -161,6 +161,7 @@
       "test-assert.js",
       "test-bad-unicode.js",
       "test-btoa-atob.js",
+      "test-buffer-alloc.js",
       "test-buffer-arraybuffer.js",
       "test-buffer-ascii.js",
       "test-buffer-badhex.js",
@@ -175,6 +176,7 @@
       "test-buffer-fakes.js",
       "test-buffer-from.js",
       "test-buffer-includes.js",
+      "test-buffer-indexof.js",
       "test-buffer-inheritance.js",
       "test-buffer-isencoding.js",
       "test-buffer-iterator.js",
diff --git a/cli/tests/node_compat/test/parallel/test-buffer-alloc.js b/cli/tests/node_compat/test/parallel/test-buffer-alloc.js
index c93e80f63..35b29ae95 100644
--- a/cli/tests/node_compat/test/parallel/test-buffer-alloc.js
+++ b/cli/tests/node_compat/test/parallel/test-buffer-alloc.js
@@ -2,14 +2,14 @@
 // deno-lint-ignore-file
 
 // Copyright Joyent and Node contributors. All rights reserved. MIT license.
-// Taken from Node 16.13.0
-// This file is automatically generated by "node/_tools/setup.ts". Do not modify this file manually
+// Taken from Node 18.12.1
+// This file is automatically generated by `tools/node_compat/setup.ts`. Do not modify this file manually.
 
 'use strict';
 const common = require('../common');
 
 const assert = require('assert');
-const vm = require('vm');
+// const vm = require('vm');
 
 const SlowBuffer = require('buffer').SlowBuffer;
 
@@ -48,7 +48,25 @@ assert.strictEqual(d.length, 0);
   assert.strictEqual(b.offset, 0);
 }
 
+// Test creating a Buffer from a Uint8Array
+{
+  const ui8 = new Uint8Array(4).fill(42);
+  const e = Buffer.from(ui8);
+  for (const [index, value] of e.entries()) {
+    assert.strictEqual(value, ui8[index]);
+  }
+}
+// Test creating a Buffer from a Uint8Array (old constructor)
+{
+  const ui8 = new Uint8Array(4).fill(42);
+  const e = Buffer(ui8);
+  for (const [key, value] of e.entries()) {
+    assert.strictEqual(value, ui8[key]);
+  }
+}
+
 // Test creating a Buffer from a Uint32Array
+// Note: it is implicitly interpreted as Array of integers modulo 256
 {
   const ui32 = new Uint32Array(4).fill(42);
   const e = Buffer.from(ui32);
@@ -57,11 +75,12 @@ assert.strictEqual(d.length, 0);
   }
 }
 // Test creating a Buffer from a Uint32Array (old constructor)
+// Note: it is implicitly interpreted as Array of integers modulo 256
 {
   const ui32 = new Uint32Array(4).fill(42);
   const e = Buffer(ui32);
   for (const [key, value] of e.entries()) {
-    assert.deepStrictEqual(value, ui32[key]);
+    assert.strictEqual(value, ui32[key]);
   }
 }
 
diff --git a/cli/tests/node_compat/test/parallel/test-buffer-arraybuffer.js b/cli/tests/node_compat/test/parallel/test-buffer-arraybuffer.js
index 9f515736e..286481758 100644
--- a/cli/tests/node_compat/test/parallel/test-buffer-arraybuffer.js
+++ b/cli/tests/node_compat/test/parallel/test-buffer-arraybuffer.js
@@ -2,8 +2,8 @@
 // deno-lint-ignore-file
 
 // Copyright Joyent and Node contributors. All rights reserved. MIT license.
-// Taken from Node 16.13.0
-// This file is automatically generated by "node/_tools/setup.ts". Do not modify this file manually
+// Taken from Node 18.12.1
+// This file is automatically generated by `tools/node_compat/setup.ts`. Do not modify this file manually.
 
 'use strict';
 
@@ -42,21 +42,18 @@ assert.strictEqual(dv.getFloat64(8, true), 3.1415);
 
 // Now test protecting users from doing stupid things
 
-// TODO(Soremwar)
-// There is an inconsistency on feross implementation on how buffers are checked
-// Enable once it's sorted out
-// assert.throws(function() {
-//   function AB() { }
-//   Object.setPrototypeOf(AB, ArrayBuffer);
-//   Object.setPrototypeOf(AB.prototype, ArrayBuffer.prototype);
-// 	Buffer.from(new AB());
-// }, {
-//   code: 'ERR_INVALID_ARG_TYPE',
-//   name: 'TypeError',
-//   message: 'The first argument must be of type string or an instance of ' +
-//            'Buffer, ArrayBuffer, or Array or an Array-like Object. Received ' +
-//            'an instance of AB'
-// });
+assert.throws(function() {
+  function AB() { }
+  Object.setPrototypeOf(AB, ArrayBuffer);
+  Object.setPrototypeOf(AB.prototype, ArrayBuffer.prototype);
+  Buffer.from(new AB());
+}, {
+  code: 'ERR_INVALID_ARG_TYPE',
+  name: 'TypeError',
+  message: 'The first argument must be of type string or an instance of ' +
+           'Buffer, ArrayBuffer, or Array or an Array-like Object. Received ' +
+           'an instance of AB'
+});
 
 // Test the byteOffset and length arguments
 {
diff --git a/cli/tests/node_compat/test/parallel/test-buffer-bytelength.js b/cli/tests/node_compat/test/parallel/test-buffer-bytelength.js
index e23b3c3c2..7fb62c78f 100644
--- a/cli/tests/node_compat/test/parallel/test-buffer-bytelength.js
+++ b/cli/tests/node_compat/test/parallel/test-buffer-bytelength.js
@@ -2,15 +2,15 @@
 // deno-lint-ignore-file
 
 // Copyright Joyent and Node contributors. All rights reserved. MIT license.
-// Taken from Node 16.13.0
-// This file is automatically generated by "node/_tools/setup.ts". Do not modify this file manually
+// Taken from Node 18.12.1
+// This file is automatically generated by `tools/node_compat/setup.ts`. Do not modify this file manually.
 
 'use strict';
 
 const common = require('../common');
 const assert = require('assert');
 const SlowBuffer = require('buffer').SlowBuffer;
-const vm = require('vm');
+// const vm = require('vm');
 
 [
   [32, 'latin1'],
@@ -30,8 +30,6 @@ const vm = require('vm');
   );
 });
 
-assert.strictEqual(Buffer.byteLength('', undefined, true), -1);
-
 assert(ArrayBuffer.isView(new Buffer(10)));
 assert(ArrayBuffer.isView(new SlowBuffer(10)));
 assert(ArrayBuffer.isView(Buffer.alloc(10)));
@@ -98,6 +96,7 @@ assert.strictEqual(Buffer.byteLength('aGkk', 'base64'), 3);
 assert.strictEqual(
   Buffer.byteLength('bHNrZGZsa3NqZmtsc2xrZmFqc2RsZmtqcw==', 'base64'), 25
 );
+// base64url
 assert.strictEqual(Buffer.byteLength('aGVsbG8gd29ybGQ', 'base64url'), 11);
 assert.strictEqual(Buffer.byteLength('aGVsbG8gd29ybGQ', 'BASE64URL'), 11);
 assert.strictEqual(Buffer.byteLength('bm9kZS5qcyByb2NrcyE', 'base64url'), 14);
@@ -128,7 +127,7 @@ assert.strictEqual(Buffer.byteLength('Il était tué', 'utf8'), 14);
 
 // TODO(Soremwar)
 // Enable once vm module is available
-// // Test that ArrayBuffer from a different context is detected correctly
+// Test that ArrayBuffer from a different context is detected correctly
 // const arrayBuf = vm.runInNewContext('new ArrayBuffer()');
 // assert.strictEqual(Buffer.byteLength(arrayBuf), 0);
 
diff --git a/cli/tests/node_compat/test/parallel/test-buffer-from.js b/cli/tests/node_compat/test/parallel/test-buffer-from.js
index ef023cf0b..6483e2a63 100644
--- a/cli/tests/node_compat/test/parallel/test-buffer-from.js
+++ b/cli/tests/node_compat/test/parallel/test-buffer-from.js
@@ -2,14 +2,14 @@
 // deno-lint-ignore-file
 
 // Copyright Joyent and Node contributors. All rights reserved. MIT license.
-// Taken from Node 16.13.0
-// This file is automatically generated by "node/_tools/setup.ts". Do not modify this file manually
+// Taken from Node 18.12.1
+// This file is automatically generated by `tools/node_compat/setup.ts`. Do not modify this file manually.
 
 'use strict';
 
 const common = require('../common');
 const { deepStrictEqual, throws } = require('assert');
-const { runInNewContext } = require('vm');
+// const { runInNewContext } = require('vm');
 
 const checkString = 'test';
 
@@ -36,7 +36,6 @@ class MyBadPrimitive {
 deepStrictEqual(Buffer.from(new String(checkString)), check);
 deepStrictEqual(Buffer.from(new MyString()), check);
 deepStrictEqual(Buffer.from(new MyPrimitive()), check);
-
 // TODO(Soremwar)
 // Enable once again when vm works correctly
 // deepStrictEqual(
@@ -66,7 +65,7 @@ deepStrictEqual(Buffer.from(new MyPrimitive()), check);
              'Buffer, ArrayBuffer, or Array or an Array-like Object.' +
              common.invalidArgTypeHelper(input)
   };
-	throws(() => Buffer.from(input), errObj);
+  throws(() => Buffer.from(input), errObj);
   throws(() => Buffer.from(input, 'hex'), errObj);
 });
 
diff --git a/cli/tests/node_compat/test/parallel/test-buffer-includes.js b/cli/tests/node_compat/test/parallel/test-buffer-includes.js
index 66da7bfd3..797ec8246 100644
--- a/cli/tests/node_compat/test/parallel/test-buffer-includes.js
+++ b/cli/tests/node_compat/test/parallel/test-buffer-includes.js
@@ -2,8 +2,8 @@
 // deno-lint-ignore-file
 
 // Copyright Joyent and Node contributors. All rights reserved. MIT license.
-// Taken from Node 16.13.0
-// This file is automatically generated by "node/_tools/setup.ts". Do not modify this file manually
+// Taken from Node 18.12.1
+// This file is automatically generated by `tools/node_compat/setup.ts`. Do not modify this file manually.
 
 'use strict';
 const common = require('../common');
diff --git a/cli/tests/node_compat/test/parallel/test-buffer-indexof.js b/cli/tests/node_compat/test/parallel/test-buffer-indexof.js
index e98e34349..802e0208b 100644
--- a/cli/tests/node_compat/test/parallel/test-buffer-indexof.js
+++ b/cli/tests/node_compat/test/parallel/test-buffer-indexof.js
@@ -2,8 +2,8 @@
 // deno-lint-ignore-file
 
 // Copyright Joyent and Node contributors. All rights reserved. MIT license.
-// Taken from Node 16.13.0
-// This file is automatically generated by "node/_tools/setup.ts". Do not modify this file manually
+// Taken from Node 18.12.1
+// This file is automatically generated by `tools/node_compat/setup.ts`. Do not modify this file manually.
 
 'use strict';
 const common = require('../common');
diff --git a/ext/node/polyfills/internal/buffer.mjs b/ext/node/polyfills/internal/buffer.mjs
index 6f3121233..cd69a199f 100644
--- a/ext/node/polyfills/internal/buffer.mjs
+++ b/ext/node/polyfills/internal/buffer.mjs
@@ -744,12 +744,12 @@ Buffer.prototype.utf8Slice = function utf8Slice(string, offset, length) {
 };
 
 Buffer.prototype.utf8Write = function utf8Write(string, offset, length) {
-  return blitBuffer(
-    utf8ToBytes(string, this.length - offset),
-    this,
-    offset,
-    length,
-  );
+  offset = offset || 0;
+  const maxLength = Math.min(length || Infinity, this.length - offset);
+  const buf = offset || maxLength < this.length
+    ? this.subarray(offset, maxLength + offset)
+    : this;
+  return utf8Encoder.encodeInto(string, buf).written;
 };
 
 Buffer.prototype.write = function write(string, offset, length, encoding) {
@@ -1687,80 +1687,13 @@ function checkIntBI(value, min, max, buf, offset, byteLength2) {
   checkBounds(buf, offset, byteLength2);
 }
 
-function utf8ToBytes(string, units) {
-  units = units || Infinity;
-  let codePoint;
-  const length = string.length;
-  let leadSurrogate = null;
-  const bytes = [];
-  for (let i = 0; i < length; ++i) {
-    codePoint = string.charCodeAt(i);
-    if (codePoint > 55295 && codePoint < 57344) {
-      if (!leadSurrogate) {
-        if (codePoint > 56319) {
-          if ((units -= 3) > -1) {
-            bytes.push(239, 191, 189);
-          }
-          continue;
-        } else if (i + 1 === length) {
-          if ((units -= 3) > -1) {
-            bytes.push(239, 191, 189);
-          }
-          continue;
-        }
-        leadSurrogate = codePoint;
-        continue;
-      }
-      if (codePoint < 56320) {
-        if ((units -= 3) > -1) {
-          bytes.push(239, 191, 189);
-        }
-        leadSurrogate = codePoint;
-        continue;
-      }
-      codePoint = (leadSurrogate - 55296 << 10 | codePoint - 56320) + 65536;
-    } else if (leadSurrogate) {
-      if ((units -= 3) > -1) {
-        bytes.push(239, 191, 189);
-      }
-    }
-    leadSurrogate = null;
-    if (codePoint < 128) {
-      if ((units -= 1) < 0) {
-        break;
-      }
-      bytes.push(codePoint);
-    } else if (codePoint < 2048) {
-      if ((units -= 2) < 0) {
-        break;
-      }
-      bytes.push(codePoint >> 6 | 192, codePoint & 63 | 128);
-    } else if (codePoint < 65536) {
-      if ((units -= 3) < 0) {
-        break;
-      }
-      bytes.push(
-        codePoint >> 12 | 224,
-        codePoint >> 6 & 63 | 128,
-        codePoint & 63 | 128,
-      );
-    } else if (codePoint < 1114112) {
-      if ((units -= 4) < 0) {
-        break;
-      }
-      bytes.push(
-        codePoint >> 18 | 240,
-        codePoint >> 12 & 63 | 128,
-        codePoint >> 6 & 63 | 128,
-        codePoint & 63 | 128,
-      );
-    } else {
-      throw new Error("Invalid code point");
-    }
-  }
-  return bytes;
-}
-
+/**
+ * @param {Uint8Array} src Source buffer to read from
+ * @param {Buffer} dst Destination buffer to write to
+ * @param {number} [offset] Byte offset to write at in the destination buffer
+ * @param {number} [byteLength] Optional number of bytes to, at most, write into destination buffer.
+ * @returns {number} Number of bytes written to destination buffer
+ */
 function blitBuffer(src, dst, offset, byteLength = Infinity) {
   const srcLength = src.length;
   // Establish the number of bytes to be written
@@ -1771,7 +1704,7 @@ function blitBuffer(src, dst, offset, byteLength = Infinity) {
     // The length of the source sets an upper bound being the source of data.
     srcLength,
     // The length of the destination minus any offset into it sets an upper bound.
-    dst.length - offset,
+    dst.length - (offset || 0),
   );
   if (bytesToWrite < srcLength) {
     // Resize the source buffer to the number of bytes we're about to write.
diff --git a/tools/node_compat/TODO.md b/tools/node_compat/TODO.md
index 870fad293..41dfb70b0 100644
--- a/tools/node_compat/TODO.md
+++ b/tools/node_compat/TODO.md
@@ -3,7 +3,7 @@
 
 NOTE: This file should not be manually edited. Please edit `cli/tests/node_compat/config.json` and run `deno task setup` in `tools/node_compat` dir instead.
 
-Total: 2926
+Total: 2924
 
 - [abort/test-abort-backtrace.js](https://github.com/nodejs/node/tree/v18.12.1/test/abort/test-abort-backtrace.js)
 - [abort/test-abort-fatal-error.js](https://github.com/nodejs/node/tree/v18.12.1/test/abort/test-abort-fatal-error.js)
@@ -252,14 +252,12 @@ Total: 2926
 - [parallel/test-blocklist.js](https://github.com/nodejs/node/tree/v18.12.1/test/parallel/test-blocklist.js)
 - [parallel/test-bootstrap-modules.js](https://github.com/nodejs/node/tree/v18.12.1/test/parallel/test-bootstrap-modules.js)
 - [parallel/test-broadcastchannel-custom-inspect.js](https://github.com/nodejs/node/tree/v18.12.1/test/parallel/test-broadcastchannel-custom-inspect.js)
-- [parallel/test-buffer-alloc.js](https://github.com/nodejs/node/tree/v18.12.1/test/parallel/test-buffer-alloc.js)
 - [parallel/test-buffer-backing-arraybuffer.js](https://github.com/nodejs/node/tree/v18.12.1/test/parallel/test-buffer-backing-arraybuffer.js)
 - [parallel/test-buffer-compare.js](https://github.com/nodejs/node/tree/v18.12.1/test/parallel/test-buffer-compare.js)
 - [parallel/test-buffer-constructor-deprecation-error.js](https://github.com/nodejs/node/tree/v18.12.1/test/parallel/test-buffer-constructor-deprecation-error.js)
 - [parallel/test-buffer-constructor-node-modules-paths.js](https://github.com/nodejs/node/tree/v18.12.1/test/parallel/test-buffer-constructor-node-modules-paths.js)
 - [parallel/test-buffer-constructor-outside-node-modules.js](https://github.com/nodejs/node/tree/v18.12.1/test/parallel/test-buffer-constructor-outside-node-modules.js)
 - [parallel/test-buffer-fill.js](https://github.com/nodejs/node/tree/v18.12.1/test/parallel/test-buffer-fill.js)
-- [parallel/test-buffer-indexof.js](https://github.com/nodejs/node/tree/v18.12.1/test/parallel/test-buffer-indexof.js)
 - [parallel/test-buffer-inspect.js](https://github.com/nodejs/node/tree/v18.12.1/test/parallel/test-buffer-inspect.js)
 - [parallel/test-buffer-pending-deprecation.js](https://github.com/nodejs/node/tree/v18.12.1/test/parallel/test-buffer-pending-deprecation.js)
 - [parallel/test-buffer-pool-untransferable.js](https://github.com/nodejs/node/tree/v18.12.1/test/parallel/test-buffer-pool-untransferable.js)