diff --git a/src/jsc/bindings/JSBuffer.cpp b/src/jsc/bindings/JSBuffer.cpp index d66d3ba5da6a..a6631be7788e 100644 --- a/src/jsc/bindings/JSBuffer.cpp +++ b/src/jsc/bindings/JSBuffer.cpp @@ -81,6 +81,7 @@ #include #include #include +#include extern "C" bool Bun__Node__ZeroFillBuffers; @@ -96,9 +97,6 @@ extern "C" size_t highway_index_of_char(const uint8_t* haystack, size_t haystack extern "C" size_t highway_last_index_of_char(const uint8_t* haystack, size_t haystack_len, uint8_t needle); static constexpr size_t kHighwayNotFound = ~static_cast(0); -// export fn Bun__inspect_singleline(globalThis: *JSGlobalObject, value: JSValue) bun.String -extern "C" BunString Bun__inspect_singleline(JSC::JSGlobalObject* globalObject, JSC::JSValue value); - using namespace JSC; using namespace WebCore; @@ -1144,53 +1142,26 @@ static JSC::EncodedJSValue jsBufferPrototypeFunction_compareBody(JSC::JSGlobalOb size_t sourceEndInit = castedThis->byteLength(); size_t sourceEnd = sourceEndInit; - JSValue targetStartValue = jsUndefined(); - JSValue targetEndValue = jsUndefined(); - JSValue sourceStartValue = jsUndefined(); - JSValue sourceEndValue = jsUndefined(); - - switch (callFrame->argumentCount()) { - default: - sourceEndValue = callFrame->uncheckedArgument(4); - if (sourceEndValue != jsUndefined()) { - Bun::V::validateInteger(throwScope, lexicalGlobalObject, sourceEndValue, "sourceEnd"_s, jsNumber(0), jsNumber(Bun::Buffer::kMaxLength), &sourceEnd); - RETURN_IF_EXCEPTION(throwScope, {}); - } - [[fallthrough]]; - case 4: - sourceStartValue = callFrame->uncheckedArgument(3); - if (sourceStartValue != jsUndefined()) { - Bun::V::validateInteger(throwScope, lexicalGlobalObject, sourceStartValue, "sourceStart"_s, jsNumber(0), jsNumber(Bun::Buffer::kMaxLength), &sourceStart); - RETURN_IF_EXCEPTION(throwScope, {}); - } - [[fallthrough]]; - case 3: - targetEndValue = callFrame->uncheckedArgument(2); - if (targetEndValue != jsUndefined()) { - Bun::V::validateInteger(throwScope, lexicalGlobalObject, targetEndValue, "targetEnd"_s, jsNumber(0), jsNumber(Bun::Buffer::kMaxLength), &targetEnd); - RETURN_IF_EXCEPTION(throwScope, {}); - } - [[fallthrough]]; - case 2: - targetStartValue = callFrame->uncheckedArgument(1); - if (targetStartValue != jsUndefined()) { - Bun::V::validateInteger(throwScope, lexicalGlobalObject, targetStartValue, "targetStart"_s, jsNumber(0), jsNumber(Bun::Buffer::kMaxLength), &targetStart); - RETURN_IF_EXCEPTION(throwScope, {}); - } - break; - case 1: - case 0: - break; + // Node's validateOffset, in argument order: a start by kMaxOffset, an end by its buffer. + JSValue targetStartValue = callFrame->argument(1); + if (!targetStartValue.isUndefined()) { + Bun::V::validateInteger(throwScope, lexicalGlobalObject, targetStartValue, "targetStart"_s, jsNumber(0), jsDoubleNumber(Bun::Buffer::kMaxOffset), &targetStart); + RETURN_IF_EXCEPTION(throwScope, {}); } - - // Validate end values against their respective buffer lengths to prevent OOB access. - // This matches Node.js behavior where targetEnd is validated against target.length - // and sourceEnd is validated against source.length. - if (targetEnd > targetEndInit) { - return Bun::ERR::OUT_OF_RANGE(throwScope, lexicalGlobalObject, "targetEnd"_s, 0, targetEndInit, targetEndValue); + JSValue targetEndValue = callFrame->argument(2); + if (!targetEndValue.isUndefined()) { + Bun::V::validateInteger(throwScope, lexicalGlobalObject, targetEndValue, "targetEnd"_s, jsNumber(0), jsNumber(targetEndInit), &targetEnd); + RETURN_IF_EXCEPTION(throwScope, {}); } - if (sourceEnd > sourceEndInit) { - return Bun::ERR::OUT_OF_RANGE(throwScope, lexicalGlobalObject, "sourceEnd"_s, 0, sourceEndInit, sourceEndValue); + JSValue sourceStartValue = callFrame->argument(3); + if (!sourceStartValue.isUndefined()) { + Bun::V::validateInteger(throwScope, lexicalGlobalObject, sourceStartValue, "sourceStart"_s, jsNumber(0), jsDoubleNumber(Bun::Buffer::kMaxOffset), &sourceStart); + RETURN_IF_EXCEPTION(throwScope, {}); + } + JSValue sourceEndValue = callFrame->argument(4); + if (!sourceEndValue.isUndefined()) { + Bun::V::validateInteger(throwScope, lexicalGlobalObject, sourceEndValue, "sourceEnd"_s, jsNumber(0), jsNumber(sourceEndInit), &sourceEnd); + RETURN_IF_EXCEPTION(throwScope, {}); } // When start >= end for either side, return early per Node.js semantics. @@ -1446,7 +1417,7 @@ static JSC::EncodedJSValue jsBufferPrototypeFunction_fillBody(JSC::JSGlobalObjec // byteLength read further down. // https://github.com/nodejs/node/blob/v22.9.0/lib/buffer.js#L1066-L1079 if (!offsetValue.isUndefined()) { - Bun::V::validateInteger(scope, lexicalGlobalObject, offsetValue, "offset"_s, jsNumber(0), jsNumber(Bun::Buffer::kMaxLength), &offset); + Bun::V::validateInteger(scope, lexicalGlobalObject, offsetValue, "offset"_s, jsNumber(0), jsDoubleNumber(Bun::Buffer::kMaxOffset), &offset); RETURN_IF_EXCEPTION(scope, {}); // Node only reads `end` once `offset` is present: fill(v, undefined, end) // ignores end (it is never validated) and fills the whole buffer. @@ -1815,18 +1786,16 @@ static int64_t indexOf(JSC::JSGlobalObject* lexicalGlobalObject, ThrowScope& sco // and byteLength from the buffer right before use, and check for detachment // after all JS calls in each code path are complete. - // Helper: re-fetch buffer state after JS calls. Returns false if the buffer - // was detached; the caller returns -1 (matches Node.js for a detached - // haystack). A merely EMPTY haystack is not -1; see computeIndexOfRange. - auto refetchBufferState = [&](const uint8_t*& typedVector, size_t& len) -> bool { + // Helper: re-fetch buffer state after JS calls. A detached haystack reads + // as empty, like in Node; computeIndexOfRange never touches a length-0 vector. + auto refetchBufferState = [&](const uint8_t*& typedVector, size_t& len) { if (buffer->isDetached()) [[unlikely]] { typedVector = nullptr; len = 0; - return false; + return; } typedVector = buffer->typedVector(); len = buffer->byteLength(); - return true; }; if (std::isnan(byteOffsetD)) byteOffsetD = dir ? 0 : byteLength; @@ -1835,7 +1804,7 @@ static int64_t indexOf(JSC::JSGlobalObject* lexicalGlobalObject, ThrowScope& sco auto byteValue = static_cast((valueValue.toInt32(lexicalGlobalObject)) % 256); RETURN_IF_EXCEPTION(scope, -1); const uint8_t* typedVector; - if (!refetchBufferState(typedVector, byteLength)) return -1; + refetchBufferState(typedVector, byteLength); return indexOfNumber(lexicalGlobalObject, last, typedVector, byteLength, byteOffsetD, endD, byteValue); } @@ -1855,14 +1824,14 @@ static int64_t indexOf(JSC::JSGlobalObject* lexicalGlobalObject, ThrowScope& sco auto* str = valueValue.toString(lexicalGlobalObject); RETURN_IF_EXCEPTION(scope, -1); const uint8_t* typedVector; - if (!refetchBufferState(typedVector, byteLength)) return -1; + refetchBufferState(typedVector, byteLength); return indexOfString(lexicalGlobalObject, last, typedVector, byteLength, byteOffsetD, endD, str, encoding.value()); } if (auto* array = dynamicDowncast(valueValue)) { if (!encoding.has_value()) encoding = BufferEncodingType::utf8; const uint8_t* typedVector; - if (!refetchBufferState(typedVector, byteLength)) return -1; + refetchBufferState(typedVector, byteLength); // A needle whose backing buffer was detached by a valueOf/toPrimitive // callback above reports byteLength()==0; computeIndexOfRange's // empty-needle path handles that (clamped to `end`) without touching @@ -1952,17 +1921,44 @@ static JSC::EncodedJSValue jsBufferPrototypeFunction_inspectBody(JSC::JSGlobalOb result.append(','); } result.append(' '); - size_t i = 0; + + // Like Node: util.inspect a null-prototype copy of the extras with + // the caller's options, then strip "[Object: null prototype] { " and " }". + JSObject* extras = JSC::constructEmptyObject(vm, globalObject->nullPrototypeObjectStructure()); for (auto ident : array) { - if (i > 0) result.append(", "_s); - result.append(ident.string()); - result.append(": "_s); auto value = castedThis->get(globalObject, ident); RETURN_IF_EXCEPTION(scope, {}); - auto inspected = Bun__inspect_singleline(globalObject, value).transferToWTFString(); + extras->putDirect(vm, ident, value); + } + + // Define the copies (like Node's spread), so no Object.prototype setter runs. + JSObject* options = JSC::constructEmptyObject(globalObject); + if (auto* ctxObject = dynamicDowncast(ctx)) { + PropertyNameArrayBuilder names(vm, PropertyNameMode::StringsAndSymbols, PrivateSymbolMode::Exclude); + ctxObject->methodTable()->getOwnPropertyNames(ctxObject, globalObject, names, DontEnumPropertiesMode::Exclude); RETURN_IF_EXCEPTION(scope, {}); - result.append(inspected); - i++; + for (const auto& name : names) { + JSValue value = ctxObject->get(globalObject, name); + RETURN_IF_EXCEPTION(scope, {}); + options->putDirectMayBeIndex(globalObject, name, value); + RETURN_IF_EXCEPTION(scope, {}); + } + } + options->putDirect(vm, Identifier::fromString(vm, "breakLength"_s), jsDoubleNumber(std::numeric_limits::infinity())); + options->putDirect(vm, Identifier::fromString(vm, "compact"_s), jsBoolean(true)); + + JSFunction* inspectFn = globalObject->utilInspectFunction(); + RETURN_IF_EXCEPTION(scope, {}); + MarkedArgumentBuffer args; + args.append(extras); + args.append(options); + JSValue inspected = JSC::call(globalObject, inspectFn, ArgList(args), "util.inspect"_s); + RETURN_IF_EXCEPTION(scope, {}); + auto inspectedString = inspected.toWTFString(globalObject); + RETURN_IF_EXCEPTION(scope, {}); + constexpr size_t prefixLength = sizeof("[Object: null prototype] { ") - 1; + if (inspectedString.length() > prefixLength + 2) { + result.append(StringView(inspectedString).substring(prefixLength, inspectedString.length() - prefixLength - 2)); } } } diff --git a/src/jsc/bindings/JSBuffer.h b/src/jsc/bindings/JSBuffer.h index e000e5cd2df1..8c127a092301 100644 --- a/src/jsc/bindings/JSBuffer.h +++ b/src/jsc/bindings/JSBuffer.h @@ -49,6 +49,10 @@ bool rejectBytesNoCopyAboveArrayBufferLimit(JSC::JSGlobalObject*, JSC::ThrowScop namespace Buffer { const size_t kMaxLength = MAX_ARRAY_BUFFER_SIZE; +// Node's validateOffset bound (its kMaxLength, 2**53 - 1). Separate from +// kMaxLength here because JSC caps an ArrayBuffer at 2**32 bytes, and a +// start offset past the end of a buffer is an empty range, not an error. +const size_t kMaxOffset = (size_t(1) << 53) - 1; const size_t kStringMaxLength = WTF::String::MaxLength; const size_t MAX_LENGTH = MAX_ARRAY_BUFFER_SIZE; const size_t MAX_STRING_LENGTH = WTF::String::MaxLength; diff --git a/test/js/node/buffer.test.js b/test/js/node/buffer.test.js index 5b4f065be33f..d7499f261725 100644 --- a/test/js/node/buffer.test.js +++ b/test/js/node/buffer.test.js @@ -5,6 +5,7 @@ import { createHash } from "node:crypto"; import { readFileSync } from "node:fs"; import os from "node:os"; import { join } from "node:path"; +import { inspect } from "node:util"; import vm from "node:vm"; const BufferModule = await import("buffer"); @@ -5434,3 +5435,192 @@ describe("read*/write* after JIT tier-up", () => { expect(() => scratch.readIntLE(1.5, 2)).toThrow("an integer"); }); }); + +describe("fill() and compare() offsets past the end of the buffer", () => { + // Node bounds a start offset only by its kMaxLength (2**53 - 1). A start at or past + // the end is an empty range: fill() is a no-op and compare() sees an empty side. + const MAX = 2 ** 53 - 1; + + it("fill() with an offset past the end is a no-op", () => { + for (const offset of [3, 4, 2 ** 32, 2 ** 32 + 1, 2 ** 40, MAX]) { + const buf = Buffer.alloc(3); + expect(buf.fill(1, offset)).toBe(buf); + expect([...buf]).toEqual([0, 0, 0]); + } + expect([...Buffer.alloc(3).fill(1, 2 ** 33, 1)]).toEqual([0, 0, 0]); + }); + + it("fill() rejects an offset above 2**53 - 1 and an end above the length", () => { + expect(() => Buffer.alloc(3).fill(1, 2 ** 53)).toThrow( + expect.objectContaining({ + code: "ERR_OUT_OF_RANGE", + message: `The value of "offset" is out of range. It must be >= 0 && <= ${MAX}. Received 9_007_199_254_740_992`, + }), + ); + expect(() => Buffer.alloc(3).fill(1, 0, 2 ** 33)).toThrow( + expect.objectContaining({ + code: "ERR_OUT_OF_RANGE", + message: 'The value of "end" is out of range. It must be >= 0 && <= 3. Received 8_589_934_592', + }), + ); + }); + + it("compare() with a start past the end compares against an empty range", () => { + const buf = Buffer.alloc(3); + const target = Buffer.alloc(3); + for (const start of [3, 4, 2 ** 32, 2 ** 32 + 1, MAX]) { + expect(buf.compare(target, start)).toBe(1); + expect(buf.compare(target, 0, 3, start)).toBe(-1); + expect(buf.compare(target, start, 3, start)).toBe(0); + } + }); + + it("compare() rejects a start above 2**53 - 1 and an end above the length", () => { + const buf = Buffer.alloc(3); + const target = Buffer.alloc(3); + expect(() => buf.compare(target, 2 ** 53)).toThrow( + expect.objectContaining({ + code: "ERR_OUT_OF_RANGE", + message: `The value of "targetStart" is out of range. It must be >= 0 && <= ${MAX}. Received 9_007_199_254_740_992`, + }), + ); + expect(() => buf.compare(target, 0, 3, 2 ** 53)).toThrow( + expect.objectContaining({ + code: "ERR_OUT_OF_RANGE", + message: `The value of "sourceStart" is out of range. It must be >= 0 && <= ${MAX}. Received 9_007_199_254_740_992`, + }), + ); + expect(() => buf.compare(target, 0, 4)).toThrow( + expect.objectContaining({ + code: "ERR_OUT_OF_RANGE", + message: 'The value of "targetEnd" is out of range. It must be >= 0 && <= 3. Received 4', + }), + ); + expect(() => buf.compare(target, 0, 2 ** 33)).toThrow( + expect.objectContaining({ + code: "ERR_OUT_OF_RANGE", + message: 'The value of "targetEnd" is out of range. It must be >= 0 && <= 3. Received 8_589_934_592', + }), + ); + expect(() => buf.compare(target, 0, 3, 0, 2 ** 33)).toThrow( + expect.objectContaining({ + code: "ERR_OUT_OF_RANGE", + message: 'The value of "sourceEnd" is out of range. It must be >= 0 && <= 3. Received 8_589_934_592', + }), + ); + }); + + it("compare() names the first invalid offset in argument order", () => { + const buf = Buffer.alloc(3); + const target = Buffer.alloc(3); + const named = f => { + try { + f(); + } catch (e) { + return e.message.match(/"(\w+)"/)[1]; + } + }; + expect([ + named(() => buf.compare(target, 0, 4, 0, 4)), + named(() => buf.compare(target, 2 ** 53, 4, 0, 4)), + named(() => buf.compare(target, 0, 3, 2 ** 53, 4)), + named(() => buf.compare(target, 1.5, 4)), + ]).toEqual(["targetEnd", "targetStart", "sourceStart", "targetStart"]); + }); +}); + +describe("indexOf(), lastIndexOf() and includes() on a detached buffer", () => { + const detached = () => { + const buf = Buffer.alloc(8).fill(1); + buf.buffer.transfer(); + return buf; + }; + + it("finds an empty needle at 0 and nothing else, like an empty buffer", () => { + const results = buf => [ + buf.indexOf(""), + buf.lastIndexOf(""), + buf.includes(""), + buf.indexOf(Buffer.alloc(0)), + buf.lastIndexOf(Buffer.alloc(0)), + buf.includes(Buffer.alloc(0)), + buf.indexOf("", 5), + buf.indexOf("", "utf8"), + buf.indexOf("", 0, "ucs2"), + buf.indexOf(1), + buf.lastIndexOf(1), + buf.includes(1), + buf.indexOf("a"), + buf.lastIndexOf("a"), + buf.indexOf(Buffer.from([1])), + buf.includes(Buffer.from([1])), + ]; + const expected = [0, 0, true, 0, 0, true, 0, 0, 0, -1, -1, false, -1, -1, -1, false]; + expect(results(Buffer.alloc(0))).toEqual(expected); + expect(results(detached())).toEqual(expected); + }); +}); + +describe("util.inspect(buffer) with extra own properties", () => { + const custom = Symbol.for("nodejs.util.inspect.custom"); + + it("formats the extra properties like util.inspect does for an object", () => { + const buf = Buffer.from([3]); + buf.x = "y"; + buf["a-b"] = [1, "two"]; + buf[Symbol("s")] = { a: { b: { c: 1 } } }; + const expected = ""; + expect(inspect(buf)).toBe(expected); + // The native formatter (console.log, Bun.inspect) calls the same hook with its own depth. + expect(Bun.inspect(buf, { depth: 2 })).toBe(expected); + expect(inspect({ nested: buf })).toBe(`{\n nested: ${expected}\n}`); + expect(inspect(buf, { depth: 0 })).toBe(""); + expect(inspect(buf, { colors: true })).toBe( + "", + ); + // The hook called directly, outside util.inspect. + expect(buf[custom](0, {})).toBe(expected); + }); + + it("shows non-enumerable properties only with showHidden", () => { + const buf = Buffer.from([3]); + buf.x = "y"; + buf.list = [1, "two"]; + Object.defineProperty(buf, "hidden", { value: "h", enumerable: false }); + expect(inspect(buf)).toBe(""); + expect(inspect(buf, { showHidden: true })).toBe( + "", + ); + expect(buf[custom](0, { showHidden: true })).toBe( + "", + ); + }); + + it("builds its inspect options without running setters on Object.prototype", () => { + Object.defineProperty(Object.prototype, "depth", { + configurable: true, + set() { + throw new Error("polluted depth"); + }, + }); + Object.defineProperty(Object.prototype, "x", { + configurable: true, + set() { + throw new Error("polluted x"); + }, + }); + try { + const buf = Buffer.from([3]); + Object.defineProperty(buf, "x", { value: "y", enumerable: true }); + expect(inspect(buf)).toBe(""); + } finally { + delete Object.prototype.depth; + delete Object.prototype.x; + } + }); + + it("keeps the Node layout for an empty buffer", () => { + expect(inspect(Buffer.alloc(0))).toBe(""); + expect(inspect(Object.assign(Buffer.alloc(0), { q: "z" }))).toBe(""); + }); +});