Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions src/jsc/bindings/bindings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3367,15 +3367,19 @@ JSC::EncodedJSValue JSC__JSValue__fromEntries(JSC::JSGlobalObject* globalObject,

if (!clone) {
for (size_t i = 0; i < initialCapacity; ++i) {
auto* value = Zig::toJSStringGC(values[i], globalObject);
RETURN_IF_EXCEPTION(scope, {});
object->putDirect(
vm, JSC::PropertyName(JSC::Identifier::fromString(vm, Zig::toString(keys[i]))),
Zig::toJSStringGC(values[i], globalObject), 0);
value, 0);
RETURN_IF_EXCEPTION(scope, {});
}
} else {
for (size_t i = 0; i < initialCapacity; ++i) {
auto* value = Zig::toJSStringGC(values[i], globalObject);
RETURN_IF_EXCEPTION(scope, {});
object->putDirect(vm, JSC::PropertyName(Zig::toIdentifier(keys[i], globalObject)),
Zig::toJSStringGC(values[i], globalObject), 0);
value, 0);
RETURN_IF_EXCEPTION(scope, {});
}
}
Expand Down Expand Up @@ -3700,7 +3704,7 @@ __attribute__((__always_inline__)) VirtualMachine* JSC__JSGlobalObject__bunVM(JS

JSC::EncodedJSValue ZigString__toValueGC(const ZigString* arg0, JSC::JSGlobalObject* arg1)
{
return JSC::JSValue::encode(JSC::jsString(arg1->vm(), Zig::toStringCopy(*arg0)));
return JSC::JSValue::encode(Zig::toJSStringGC(*arg0, arg1));
}

void JSC__JSValue__toZigString(JSC::EncodedJSValue JSValue0, ZigString* arg1, JSC::JSGlobalObject* arg2)
Expand Down
32 changes: 31 additions & 1 deletion src/jsc/bindings/helpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

#include <JavaScriptCore/Error.h>
#include <JavaScriptCore/Exception.h>
#include <JavaScriptCore/ExceptionHelpers.h>
#include <JavaScriptCore/Identifier.h>
#include <JavaScriptCore/JSGlobalObject.h>
#include <JavaScriptCore/JSString.h>
Expand All @@ -25,6 +26,14 @@ class GlobalObject;
extern "C" size_t Bun__stringSyntheticAllocationLimit;
extern "C" const char* Bun__errnoName(int);

namespace Bun {
namespace ERR {
// Defined in ErrorCode.cpp; forward-declared here because ErrorCode.h cannot
// be included from this header.
Comment thread
robobun marked this conversation as resolved.
Outdated
JSC::EncodedJSValue STRING_TOO_LONG(JSC::ThrowScope& throwScope, JSC::JSGlobalObject* globalObject);
}
}

namespace Zig {

// 8 bit byte
Expand Down Expand Up @@ -191,6 +200,11 @@ static const WTF::String toStringCopy(ZigString str)
return WTF::String::fromUTF8ReplacingInvalidSequences(std::span { untag(str.ptr), str.len });
}

// This will fail if the string is too long. Let's make it explicit instead of an ASSERT.
if (str.len > Bun__stringSyntheticAllocationLimit || str.len > WTF::String::MaxLength) [[unlikely]] {
return {};
}
Comment thread
claude[bot] marked this conversation as resolved.

if (isTaggedUTF16Ptr(str.ptr)) {
std::span<char16_t> out;
auto impl = WTF::StringImpl::tryCreateUninitialized(str.len, out);
Expand Down Expand Up @@ -240,9 +254,25 @@ static const JSC::JSString* toJSString(ZigString str, JSC::JSGlobalObject* globa
return JSC::jsOwnedString(global->vm(), toString(str));
}

// Copies `str` into a GC-managed JSString. Throws ERR_STRING_TOO_LONG (or an
// out-of-memory error) and returns nullptr when the string cannot be created.
Comment thread
robobun marked this conversation as resolved.
static JSC::JSString* toJSStringGC(ZigString str, JSC::JSGlobalObject* global)
{
return JSC::jsString(global->vm(), toStringCopy(str));
auto wtfString = toStringCopy(str);
if (wtfString.isNull() && str.len > 0) [[unlikely]] {
// toStringCopy returns a null string when the input is over the string
// length limit or when allocation fails; surface an error instead of
// silently returning an empty string.
Comment thread
robobun marked this conversation as resolved.
Outdated
auto scope = DECLARE_THROW_SCOPE(global->vm());
size_t maxLength = std::min(Bun__stringSyntheticAllocationLimit, static_cast<size_t>(WTF::String::MaxLength));
if (str.len > maxLength) {
Bun::ERR::STRING_TOO_LONG(scope, global);
} else {
JSC::throwOutOfMemoryError(global, scope);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
return nullptr;
}
return JSC::jsString(global->vm(), WTF::move(wtfString));
}

static const ZigString ZigStringEmpty = ZigString { (unsigned char*)"", 0 };
Expand Down
37 changes: 37 additions & 0 deletions test/js/web/encoding/text-decoder.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -947,3 +947,40 @@
},
30_000,
); // 3072 decodes + repeated Bun.gc(true) take ~4s under ASAN.

describe("TextDecoder.decode above the maximum string length", () => {
const expected = "threw:ERR_STRING_TOO_LONG:Cannot create a string longer than 2147483647 characters";
const script = size => `
try {
const s = new TextDecoder().decode(new Uint8Array(${size}));
console.log("returned:" + s.length);
} catch (e) {
console.log("threw:" + e.code + ":" + e.message);
}`;

// A zeroed input is all-ASCII, so the decoded string's length equals the
// input length. 2**31 exceeds WTF::StringImpl's maximum length (2^31-1);
// decode() used to return "" silently instead of throwing. The input is
// never written, so the 2 GiB Uint8Array stays untouched zero pages.
it("throws ERR_STRING_TOO_LONG for a 2**31 byte ASCII input", async () => {
await using proc = Bun.spawn({
cmd: [bunExe(), "-e", script("2 ** 31")],
env: bunEnv,
});
const [stdout, exitCode] = await Promise.all([proc.stdout.text(), proc.exited]);
expect(stdout.trim()).toBe(expected);
expect(exitCode).toBe(0);

Check warning on line 972 in test/js/web/encoding/text-decoder.test.js

View check run for this annotation

Claude / Claude Code Review

Subprocess tests don't pipe/read stderr

Both new subprocess tests spawn without `stderr: "pipe"` and await only `[proc.stdout.text(), proc.exited]`, unlike the neighboring SharedArrayBuffer test in this file which pipes and reads all three. If the child aborts or is OOM-killed before reaching `console.log`, the diagnostic goes to the parent's inherited stderr and the assertion just shows `expected "" to be "threw:..."`. Add `stderr: "pipe"` and assert `{ stdout, stderr, exitCode }` in both `it` blocks (lines ~966-972 and ~977-985).
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Comment thread
robobun marked this conversation as resolved.
}, 30_000); // scans 2 GiB of input; slow under debug + ASAN.

Check failure on line 973 in test/js/web/encoding/text-decoder.test.js

View check run for this annotation

Claude / Claude Code Review

2 GiB allocation test lacks low-memory guard

The 2^31-byte test wraps `new Uint8Array(2 ** 31)` and `decode()` in one try/catch, so on runners where the 2 GiB reservation itself fails (Windows does not overcommit; Linux under memory pressure or `vm.overcommit_memory=2` refuses it) the catch prints `threw:undefined:Array buffer allocation failed` and the outer `toBe(expected)` fails as a false red. Every sibling 2^31 allocation test in the repo guards this — blob-oom.test.ts:158 / fs-oom.test.ts:62 / buffer.test.js:4642 gate on `skipIf(os.t
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
Comment thread
robobun marked this conversation as resolved.
Outdated

// Same path, exercised cheaply via the synthetic allocation limit so the
// guard is covered without a 2 GiB allocation.
it("throws ERR_STRING_TOO_LONG above the synthetic allocation limit", async () => {
await using proc = Bun.spawn({
cmd: [bunExe(), "-e", script("160 * 1024 * 1024")],
env: { ...bunEnv, BUN_FEATURE_FLAG_SYNTHETIC_MEMORY_LIMIT: "134217728" },
});
const [stdout, exitCode] = await Promise.all([proc.stdout.text(), proc.exited]);
expect(stdout.trim()).toBe(expected);
expect(exitCode).toBe(0);
});
});