From 3778be86055eb41de85e516739c19adcc80c9320 Mon Sep 17 00:00:00 2001 From: Meghan Denny Date: Thu, 12 Dec 2024 19:51:57 -0800 Subject: [PATCH] misc fixes --- src/bun.js/ConsoleObject.zig | 3 +- src/bun.js/bindings/JSBuffer.cpp | 67 +++++++++------ src/bun.js/bindings/bindings.zig | 4 +- src/bun.js/node/buffer.zig | 2 + src/js/builtins/JSBufferConstructor.ts | 4 +- src/js/internal/buffer.ts | 5 +- test/js/node/test/common/index.js | 2 +- .../node/test/parallel/test-buffer-alloc.js | 24 +++--- .../test/parallel/test-buffer-arraybuffer.js | 86 +++++++++---------- 9 files changed, 106 insertions(+), 91 deletions(-) diff --git a/src/bun.js/ConsoleObject.zig b/src/bun.js/ConsoleObject.zig index 5f08103018..c929cb22e9 100644 --- a/src/bun.js/ConsoleObject.zig +++ b/src/bun.js/ConsoleObject.zig @@ -724,7 +724,6 @@ pub const FormatOptions = struct { if (try arg1.getBooleanLoose(globalThis, "sorted")) |opt| { formatOptions.ordered_properties = opt; } - if (try arg1.getBooleanLoose(globalThis, "compact")) |opt| { formatOptions.single_line = opt; } @@ -2148,7 +2147,7 @@ pub const Formatter = struct { this.addForNewLine(description.len + "()".len); writer.print(comptime Output.prettyFmt("Symbol({any})", enable_ansi_colors), .{description}); } else { - writer.print(comptime Output.prettyFmt("Symbol", enable_ansi_colors), .{}); + writer.print(comptime Output.prettyFmt("Symbol()", enable_ansi_colors), .{}); } }, .Error => { diff --git a/src/bun.js/bindings/JSBuffer.cpp b/src/bun.js/bindings/JSBuffer.cpp index 4692c33cb6..4108a8e007 100644 --- a/src/bun.js/bindings/JSBuffer.cpp +++ b/src/bun.js/bindings/JSBuffer.cpp @@ -45,6 +45,7 @@ #include #include #include +#include #include #include #include @@ -2320,8 +2321,16 @@ JSC_DEFINE_HOST_FUNCTION(jsFunction_BufferFrom_Array, (JSGlobalObject * lexicalG { auto& vm = lexicalGlobalObject->vm(); auto throwScope = DECLARE_THROW_SCOPE(vm); - throwException(lexicalGlobalObject, throwScope, createError(lexicalGlobalObject, "TODO jsFunction_BufferFrom_Array"_s)); - return {}; + auto* globalObject = reinterpret_cast(lexicalGlobalObject); + + auto arrayValue = callFrame->argument(0); + auto* constructor = lexicalGlobalObject->m_typedArrayUint8.constructor(lexicalGlobalObject); + MarkedArgumentBuffer argsBuffer; + argsBuffer.append(arrayValue); + JSValue target = globalObject->JSBufferConstructor(); + auto* object = JSC::construct(lexicalGlobalObject, constructor, target, argsBuffer, "Buffer failed to construct"_s); + RETURN_IF_EXCEPTION(throwScope, {}); + RELEASE_AND_RETURN(throwScope, JSC::JSValue::encode(object)); } JSC_DEFINE_HOST_FUNCTION(jsFunction_BufferFrom_ArraybufferByteoffsetLength, (JSGlobalObject * lexicalGlobalObject, CallFrame* callFrame)) { @@ -2334,28 +2343,6 @@ JSC_DEFINE_HOST_FUNCTION(jsFunction_BufferFrom_ArraybufferByteoffsetLength, (JSG auto offsetValue = callFrame->argument(1); auto lengthValue = callFrame->argument(2); - // This closely matches `new Uint8Array(buffer, byteOffset, length)` in JavaScriptCore's implementation. - // See Source/JavaScriptCore/runtime/JSGenericTypedArrayViewConstructorInlines.h - size_t offset = 0; - std::optional length; - if (argsCount > 1) { - - if (!offsetValue.isUndefined()) { - Bun::V::validateInteger(throwScope, lexicalGlobalObject, offsetValue, jsString(vm, String("offset"_s)), jsNumber(0), jsUndefined()); - RETURN_IF_EXCEPTION(throwScope, {}); - offset = offsetValue.asNumber(); - } - - if (argsCount > 2) { - // If the length value is present but undefined, treat it as missing. - if (!lengthValue.isUndefined()) { - length = lengthValue.toTypedArrayIndex(globalObject, "length"_s); - // TOOD: return Node.js error - RETURN_IF_EXCEPTION(throwScope, {}); - } - } - } - auto* jsBuffer = jsCast(arrayBufferValue.asCell()); RefPtr buffer = jsBuffer->impl(); if (buffer->isDetached()) { @@ -2364,6 +2351,38 @@ JSC_DEFINE_HOST_FUNCTION(jsFunction_BufferFrom_ArraybufferByteoffsetLength, (JSG return {}; } size_t byteLength = buffer->byteLength(); + double byteLengthD = byteLength; + + // This closely matches `new Uint8Array(buffer, byteOffset, length)` in JavaScriptCore's implementation. + // See Source/JavaScriptCore/runtime/JSGenericTypedArrayViewConstructorInlines.h + size_t offset = 0; + std::optional length; + size_t maxLength = byteLength; + if (argsCount > 1) { + + if (!offsetValue.isUndefined()) { + offsetValue = jsDoubleNumber(offsetValue.toNumber(lexicalGlobalObject)); + RETURN_IF_EXCEPTION(throwScope, {}); + if (std::isnan(offsetValue.asNumber())) offsetValue = jsNumber(0); + if ((byteLengthD - offsetValue.asNumber()) < 0) return Bun::ERR::BUFFER_OUT_OF_BOUNDS(throwScope, lexicalGlobalObject, "offset"_s); + maxLength = byteLengthD - offsetValue.asNumber(); + offset = offsetValue.asNumber(); + } + + if (!lengthValue.isUndefined()) { + lengthValue = jsDoubleNumber(lengthValue.toNumber(lexicalGlobalObject)); + RETURN_IF_EXCEPTION(throwScope, {}); + length = lengthValue.asNumber(); + if (length > 0) { + if (length > maxLength) { + return Bun::ERR::BUFFER_OUT_OF_BOUNDS(throwScope, lexicalGlobalObject, "length"_s); + } + } else { + length = 0; + } + } + } + if (offset > byteLength) { return Bun::ERR::BUFFER_OUT_OF_BOUNDS(throwScope, globalObject, "offset"_s); } diff --git a/src/bun.js/bindings/bindings.zig b/src/bun.js/bindings/bindings.zig index 173add2802..87a683fd5b 100644 --- a/src/bun.js/bindings/bindings.zig +++ b/src/bun.js/bindings/bindings.zig @@ -594,7 +594,7 @@ pub const ZigString = extern struct { return (@intFromPtr(this._unsafe_ptr_do_not_use) & (1 << 63)) != 0; } - pub inline fn utf16Slice(this: *const ZigString) []align(1) const u16 { + pub fn utf16Slice(this: *const ZigString) []align(1) const u16 { if (comptime bun.Environment.allow_assert) { if (this.len > 0 and !this.is16Bit()) { @panic("ZigString.utf16Slice() called on a latin1 string.\nPlease use .toSlice() instead or carefully check that .is16Bit() is false first."); @@ -604,7 +604,7 @@ pub const ZigString = extern struct { return @as([*]align(1) const u16, @ptrCast(untagged(this._unsafe_ptr_do_not_use)))[0..this.len]; } - pub inline fn utf16SliceAligned(this: *const ZigString) []const u16 { + pub fn utf16SliceAligned(this: *const ZigString) []const u16 { if (comptime bun.Environment.allow_assert) { if (this.len > 0 and !this.is16Bit()) { @panic("ZigString.utf16SliceAligned() called on a latin1 string.\nPlease use .toSlice() instead or carefully check that .is16Bit() is false first."); diff --git a/src/bun.js/node/buffer.zig b/src/bun.js/node/buffer.zig index 86d4fc73c1..af51604049 100644 --- a/src/bun.js/node/buffer.zig +++ b/src/bun.js/node/buffer.zig @@ -49,6 +49,8 @@ pub const BufferVectorized = struct { Encoder.writeU8(str.slice().ptr, str.slice().len, buf.ptr, buf.len, .hex), } catch return false; + if (written == 0 and str.length() > 0) return false; + switch (written) { 0 => return true, 1 => { diff --git a/src/js/builtins/JSBufferConstructor.ts b/src/js/builtins/JSBufferConstructor.ts index 3ec3801c39..ace7afd533 100644 --- a/src/js/builtins/JSBufferConstructor.ts +++ b/src/js/builtins/JSBufferConstructor.ts @@ -3,7 +3,7 @@ $constructor; export function from(value, encodingOrOffset, length) { const { fromString, fromArrayBuffer, fromObject } = require("internal/buffer"); - const isAnyArrayBuffer = value => value instanceof ArrayBuffer || value instanceof SharedArrayBuffer; + const { isAnyArrayBuffer } = require("node:util/types"); if (typeof value === "string") return fromString(value, encodingOrOffset); @@ -12,7 +12,7 @@ export function from(value, encodingOrOffset, length) { const valueOf = value.valueOf && value.valueOf(); if (valueOf != null && valueOf !== value && (typeof valueOf === "string" || typeof valueOf === "object")) { - return from(valueOf, encodingOrOffset, length); + return Buffer.from(valueOf, encodingOrOffset, length); } const b = fromObject(value); diff --git a/src/js/internal/buffer.ts b/src/js/internal/buffer.ts index f412a7f97b..4c35a94e5f 100644 --- a/src/js/internal/buffer.ts +++ b/src/js/internal/buffer.ts @@ -1,5 +1,6 @@ const { validateNumber, validateInteger } = require("internal/validators"); const { ERR_INVALID_ARG_TYPE, ERR_OUT_OF_RANGE, ERR_BUFFER_OUT_OF_BOUNDS } = require("internal/errors"); +const { isAnyArrayBuffer } = require("node:util/types"); const BufferFrom1 = $newCppFunction("JSBuffer.cpp", "jsFunction_BufferFrom_Array", 0); const BufferFrom2 = $newCppFunction("JSBuffer.cpp", "jsFunction_BufferFrom_ArraybufferByteoffsetLength", 0); @@ -80,10 +81,6 @@ function fromArrayBuffer(arrayBuffer: ArrayBuffer, byteOffset?: number, length?: return BufferFrom2(arrayBuffer, byteOffset, length); } -function isAnyArrayBuffer(value) { - return value instanceof ArrayBuffer || value instanceof SharedArrayBuffer; -} - function fromObject(obj) { if (obj.length !== undefined || isAnyArrayBuffer(obj.buffer)) { if (typeof obj.length !== "number") { diff --git a/test/js/node/test/common/index.js b/test/js/node/test/common/index.js index 6196ea7abd..bdd4027518 100644 --- a/test/js/node/test/common/index.js +++ b/test/js/node/test/common/index.js @@ -889,7 +889,7 @@ function invalidArgTypeHelper(input) { if (input.constructor?.name) { return ` Received an instance of ${input.constructor.name}`; } - return ` Received ${inspect(input, { depth: -1 })}`; + return ` Received ${inspect(input, { depth: 0 })}`; } let inspected = inspect(input, { colors: false }); diff --git a/test/js/node/test/parallel/test-buffer-alloc.js b/test/js/node/test/parallel/test-buffer-alloc.js index 1a54da9e40..faca8fe454 100644 --- a/test/js/node/test/parallel/test-buffer-alloc.js +++ b/test/js/node/test/parallel/test-buffer-alloc.js @@ -1123,15 +1123,14 @@ assert.strictEqual(SlowBuffer.prototype.offset, undefined); // Test that ParseArrayIndex handles full uint32 -// TODO: -// { -// const errMsg = common.expectsError({ -// code: 'ERR_BUFFER_OUT_OF_BOUNDS', -// name: 'RangeError', -// message: '"offset" is outside of buffer bounds' -// }); -// assert.throws(() => Buffer.from(new ArrayBuffer(0), -1 >>> 0), errMsg); -// } +{ + const errMsg = common.expectsError({ + code: 'ERR_BUFFER_OUT_OF_BOUNDS', + name: 'RangeError', + message: '"offset" is outside of buffer bounds' + }); + assert.throws(() => Buffer.from(new ArrayBuffer(0), -1 >>> 0), errMsg); +} // ParseArrayIndex() should reject values that don't fit in a 32 bits size_t. assert.throws(() => { @@ -1149,13 +1148,12 @@ assert.throws(() => { } // Regression test to verify that an empty ArrayBuffer does not throw. -// Buffer.from(new ArrayBuffer()); +Buffer.from(new ArrayBuffer()); // Test that ArrayBuffer from a different context is detected correctly. const arrayBuf = vm.runInNewContext('new ArrayBuffer()'); -// TODO: -// Buffer.from(arrayBuf); -// Buffer.from({ buffer: arrayBuf }); +Buffer.from(arrayBuf); +Buffer.from({ buffer: arrayBuf }); assert.throws(() => Buffer.alloc({ valueOf: () => 1 }), /"size" argument must be of type number/); diff --git a/test/js/node/test/parallel/test-buffer-arraybuffer.js b/test/js/node/test/parallel/test-buffer-arraybuffer.js index 53430c37b0..de1b064788 100644 --- a/test/js/node/test/parallel/test-buffer-arraybuffer.js +++ b/test/js/node/test/parallel/test-buffer-arraybuffer.js @@ -35,18 +35,18 @@ assert.strictEqual(dv.getFloat64(8, true), 3.1415); // Now test protecting users from doing stupid things -// 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, ' + + 'Buffer, ArrayBuffer, Array, or Array-like Object. Received ' + + 'an instance of AB' +}); // Test the byteOffset and length arguments { @@ -64,16 +64,16 @@ assert.strictEqual(dv.getFloat64(8, true), 3.1415); buf[0] = 9; assert.strictEqual(ab[1], 9); - // assert.throws(() => Buffer.from(ab.buffer, 6), { - // code: 'ERR_BUFFER_OUT_OF_BOUNDS', - // name: 'RangeError', - // message: '"offset" is outside of buffer bounds' - // }); - // assert.throws(() => Buffer.from(ab.buffer, 3, 6), { - // code: 'ERR_BUFFER_OUT_OF_BOUNDS', - // name: 'RangeError', - // message: '"length" is outside of buffer bounds' - // }); + assert.throws(() => Buffer.from(ab.buffer, 6), { + code: 'ERR_BUFFER_OUT_OF_BOUNDS', + name: 'RangeError', + message: '"offset" is outside of buffer bounds' + }); + assert.throws(() => Buffer.from(ab.buffer, 3, 6), { + code: 'ERR_BUFFER_OUT_OF_BOUNDS', + name: 'RangeError', + message: '"length" is outside of buffer bounds' + }); } // Test the deprecated Buffer() version also @@ -108,30 +108,30 @@ assert.strictEqual(dv.getFloat64(8, true), 3.1415); // If byteOffset is not numeric, it defaults to 0. const ab = new ArrayBuffer(10); const expected = Buffer.from(ab, 0); - // assert.deepStrictEqual(Buffer.from(ab, 'fhqwhgads'), expected); - // assert.deepStrictEqual(Buffer.from(ab, NaN), expected); - // assert.deepStrictEqual(Buffer.from(ab, {}), expected); - // assert.deepStrictEqual(Buffer.from(ab, []), expected); + assert.deepStrictEqual(Buffer.from(ab, 'fhqwhgads'), expected); + assert.deepStrictEqual(Buffer.from(ab, NaN), expected); + assert.deepStrictEqual(Buffer.from(ab, {}), expected); + assert.deepStrictEqual(Buffer.from(ab, []), expected); // If byteOffset can be converted to a number, it will be. - // assert.deepStrictEqual(Buffer.from(ab, [1]), Buffer.from(ab, 1)); + assert.deepStrictEqual(Buffer.from(ab, [1]), Buffer.from(ab, 1)); // If byteOffset is Infinity, throw. - // assert.throws(() => { - // Buffer.from(ab, Infinity); - // }, { - // code: 'ERR_BUFFER_OUT_OF_BOUNDS', - // name: 'RangeError', - // message: '"offset" is outside of buffer bounds' - // }); + assert.throws(() => { + Buffer.from(ab, Infinity); + }, { + code: 'ERR_BUFFER_OUT_OF_BOUNDS', + name: 'RangeError', + message: '"offset" is outside of buffer bounds' + }); } { // If length is not numeric, it defaults to 0. const ab = new ArrayBuffer(10); const expected = Buffer.from(ab, 0, 0); - // assert.deepStrictEqual(Buffer.from(ab, 0, 'fhqwhgads'), expected); - // assert.deepStrictEqual(Buffer.from(ab, 0, NaN), expected); + assert.deepStrictEqual(Buffer.from(ab, 0, 'fhqwhgads'), expected); + assert.deepStrictEqual(Buffer.from(ab, 0, NaN), expected); assert.deepStrictEqual(Buffer.from(ab, 0, {}), expected); assert.deepStrictEqual(Buffer.from(ab, 0, []), expected); @@ -139,13 +139,13 @@ assert.strictEqual(dv.getFloat64(8, true), 3.1415); assert.deepStrictEqual(Buffer.from(ab, 0, [1]), Buffer.from(ab, 0, 1)); // If length is Infinity, throw. - // assert.throws(() => { - // Buffer.from(ab, 0, Infinity); - // }, { - // code: 'ERR_BUFFER_OUT_OF_BOUNDS', - // name: 'RangeError', - // message: '"length" is outside of buffer bounds' - // }); + assert.throws(() => { + Buffer.from(ab, 0, Infinity); + }, { + code: 'ERR_BUFFER_OUT_OF_BOUNDS', + name: 'RangeError', + message: '"length" is outside of buffer bounds' + }); } // Test an array like entry with the length set to NaN.