Skip to content

Commit

Permalink
fix(node/buffer): utf8ToBytes should return a Uint8Array (denoland#20769
Browse files Browse the repository at this point in the history
)
  • Loading branch information
aapoalas committed Oct 8, 2023
1 parent edeccef commit effb5e1
Show file tree
Hide file tree
Showing 9 changed files with 67 additions and 120 deletions.
2 changes: 2 additions & 0 deletions cli/tests/node_compat/config.jsonc
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand Down
27 changes: 23 additions & 4 deletions cli/tests/node_compat/test/parallel/test-buffer-alloc.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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);
Expand All @@ -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]);
}
}

Expand Down
31 changes: 14 additions & 17 deletions cli/tests/node_compat/test/parallel/test-buffer-arraybuffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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
{
Expand Down
11 changes: 5 additions & 6 deletions cli/tests/node_compat/test/parallel/test-buffer-bytelength.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'],
Expand All @@ -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)));
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);

Expand Down
9 changes: 4 additions & 5 deletions cli/tests/node_compat/test/parallel/test-buffer-from.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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(
Expand Down Expand Up @@ -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);
});

Expand Down
4 changes: 2 additions & 2 deletions cli/tests/node_compat/test/parallel/test-buffer-includes.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
4 changes: 2 additions & 2 deletions cli/tests/node_compat/test/parallel/test-buffer-indexof.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
95 changes: 14 additions & 81 deletions ext/node/polyfills/internal/buffer.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand All @@ -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.
Expand Down
4 changes: 1 addition & 3 deletions tools/node_compat/TODO.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit effb5e1

Please sign in to comment.