Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
22 changes: 14 additions & 8 deletions src/bun_core/string/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ impl String {
/// that calls `callback(ctx, buffer, len)` when the impl is destroyed.
///
/// External strings are WTF strings whose bytes live elsewhere; `bytes` is
/// borrowed (not copied). If `bytes.len() >= max_length()`, `callback` is
/// borrowed (not copied). If `bytes.len() > max_length()`, `callback` is
/// invoked immediately and a `dead` string is returned.
///
/// `Ctx` must be a pointer-sized type (raw pointer or `&T`); enforced by
Expand All @@ -299,7 +299,7 @@ impl String {
}
let () = AssertPtrSized::<Ctx>::OK;
debug_assert!(!bytes.is_empty());
if bytes.len() >= Self::max_length() {
if bytes.len() > Self::max_length() {
Comment thread
robobun marked this conversation as resolved.
callback(ctx, bytes.as_ptr().cast_mut().cast::<c_void>(), bytes.len());
return Self::DEAD;
}
Expand All @@ -322,7 +322,7 @@ impl String {
ExternalStringImplFreeFunction<Ctx>,
extern "C" fn(*mut c_void, *mut c_void, usize),
>(callback) });
// SAFETY: bytes describes a valid slice; len < max_length checked.
// SAFETY: bytes describes a valid slice; len <= max_length checked.
let s = unsafe {
BunString__createExternal(
bytes.as_ptr(),
Expand All @@ -336,11 +336,13 @@ impl String {
s
}

/// Max `WTF::StringImpl` length (in characters, not bytes).
/// Reads the process-wide [`STRING_ALLOCATION_LIMIT`] data slot.
/// Max `WTF::StringImpl` length (in characters, not bytes):
/// [`STRING_ALLOCATION_LIMIT`] clamped to [`WTF_STRING_MAX_LENGTH`].
Comment thread
robobun marked this conversation as resolved.
#[inline]
pub fn max_length() -> usize {
STRING_ALLOCATION_LIMIT.load(Ordering::Relaxed)
STRING_ALLOCATION_LIMIT
.load(Ordering::Relaxed)
.min(WTF_STRING_MAX_LENGTH)
}

/// `bun.String.createStaticExternal` — wraps `bytes` in a
Expand Down Expand Up @@ -405,7 +407,7 @@ impl String {
if bytes.is_empty() {
return Self::EMPTY;
}
if bytes.len() >= Self::max_length() {
if bytes.len() > Self::max_length() {
return Self::DEAD;
}
// Do NOT call `into_boxed_slice()` — when `len < capacity` it issues a
Expand All @@ -424,7 +426,7 @@ impl String {
if bytes.is_empty() {
return Self::EMPTY;
}
if bytes.len() >= Self::max_length() {
if bytes.len() > Self::max_length() {
return Self::DEAD;
}
// See `create_external_globally_allocated_latin1` — avoid the
Expand Down Expand Up @@ -2101,6 +2103,10 @@ pub mod lexer_tables {
#[unsafe(export_name = "Bun__stringSyntheticAllocationLimit")]
pub static STRING_ALLOCATION_LIMIT: AtomicUsize = AtomicUsize::new(u32::MAX as usize);

/// Mirror of `WTF::StringImpl::MaxLength` (`INT32_MAX`), which C++ enforces
/// with `RELEASE_ASSERT` in the `StringImplShape` constructors.
Comment thread
robobun marked this conversation as resolved.
pub const WTF_STRING_MAX_LENGTH: usize = i32::MAX as usize;

// ──────────────────────────────────────────────────────────────────────────
// move-in: printer (MOVE_DOWN ← `bun_js_printer`)
//
Expand Down
2 changes: 1 addition & 1 deletion src/jsc/ZigString.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ pub unsafe fn to_external_u16(ptr: *const u16, len: usize, global: &JSGlobalObje
let _ = global
.err(
crate::ErrorCode::STRING_TOO_LONG,
format_args!("Cannot create a string longer than 2^32-1 characters"),
format_args!("Cannot create a string longer than 2147483647 characters"),
)
.throw();
return JSValue::ZERO;
Expand Down
3 changes: 3 additions & 0 deletions src/jsc/bindings/BunString.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,9 @@ extern "C" [[ZIG_EXPORT(zero_is_throw)]] JSC::EncodedJSValue BunString__createUT
return JSValue::encode(jsEmptyString(vm));
}
if (simdutf::validate_ascii(ptr, length)) {
if (length > WTF::String::MaxLength) [[unlikely]] {
return Bun::ERR::STRING_TOO_LONG(scope, globalObject);
}
return JSValue::encode(jsString(vm, WTF::String(std::span<const Latin1Character>(reinterpret_cast<const Latin1Character*>(ptr), length))));
}

Expand Down
4 changes: 2 additions & 2 deletions src/jsc/bindings/bindings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2530,8 +2530,8 @@ extern "C" JSC::EncodedJSValue ZigString__toJSONObject(const ZigString* strPtr,
if (str.isNull()) {
// isNull() will be true for empty strings and for strings which are too long.
// So we need to check the length is plausibly due to a long string.
if (strPtr->len > Bun__stringSyntheticAllocationLimit) {
scope.throwException(globalObject, Bun::createError(globalObject, Bun::ErrorCode::ERR_STRING_TOO_LONG, "Cannot parse a JSON string longer than 2^32-1 characters"_s));
if (strPtr->len > Bun__stringSyntheticAllocationLimit || strPtr->len > WTF::String::MaxLength) {
scope.throwException(globalObject, Bun::createError(globalObject, Bun::ErrorCode::ERR_STRING_TOO_LONG, "Cannot parse a JSON string longer than 2147483647 characters"_s));
return {};
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/jsc/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1649,7 +1649,7 @@ impl ZigStringJsc for bun_core::ZigString {
let _ = global
.err(
crate::ErrorCode::STRING_TOO_LONG,
format_args!("Cannot create a string longer than 2^32-1 characters"),
format_args!("Cannot create a string longer than 2147483647 characters"),
)
.throw();
return JSValue::ZERO;
Expand Down Expand Up @@ -1684,7 +1684,7 @@ impl ZigStringJsc for bun_core::ZigString {
let _ = global
.err(
crate::ErrorCode::STRING_TOO_LONG,
format_args!("Cannot create a string longer than 2^32-1 characters"),
format_args!("Cannot create a string longer than 2147483647 characters"),
)
.throw();
return JSValue::ZERO;
Expand Down
53 changes: 52 additions & 1 deletion test/js/node/fs/fs-oom.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { memfd_create, setSyntheticAllocationLimitForTesting } from "bun:internal-for-testing";
import { describe, expect, test } from "bun:test";
import { closeSync, readFileSync, writeFileSync, writeSync } from "fs";
import { closeSync, readFileSync, truncateSync, writeFileSync, writeSync } from "fs";
import { bunEnv, bunExe, isASAN, isLinux, isPosix, tempDir } from "harness";
import os from "node:os";
import { join } from "path";
setSyntheticAllocationLimitForTesting(128 * 1024 * 1024);

Expand Down Expand Up @@ -48,6 +49,56 @@ if (isLinux) {
});
}

// Files in [2^31, 2^32) bytes used to abort the process when decoded to a
// string: the guards in front of WTF string construction only checked the
// synthetic allocation limit (2^32 - 1 by default) and missed
// WTF::StringImpl::MaxLength (2^31 - 1), tripping a RELEASE_ASSERT in
// StringImplShape. The fs layer reports the dead string as ENOMEM (it speaks
// errno), matching the existing >= 2^32 and /dev/zero behavior above.
// 2^31 - 1 is the largest length WTF accepts and must keep working. The file
// is sparse so only the in-memory read costs 2 GiB; each case runs in a
// subprocess to keep the peak away from the test runner, and the block skips
// on small machines (same gate as buffer.test.js's 4 GiB case).
describe.skipIf(os.totalmem() < 10 * 1024 ** 3)("readFileSync at the 2 GiB string limit", () => {
const spawnRead = async (size: number) => {
using dir = tempDir("readfile-2gib", {});
const file = join(String(dir), "big.txt");
writeFileSync(file, "x");
truncateSync(file, size);
await using proc = Bun.spawn({
cmd: [
bunExe(),
"-e",
`
try {
const s = require("fs").readFileSync(${JSON.stringify(file)}, "utf8");
console.log(JSON.stringify({ length: s.length }));
} catch (e) {
console.log(JSON.stringify({ name: e.name, code: e.code }));
}
`,
],
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
return { result: JSON.parse(stdout.trim() || JSON.stringify({ stdout, stderr, exitCode })), exitCode };
};

test("2^31 bytes throws ENOMEM instead of aborting", async () => {
const { result, exitCode } = await spawnRead(2 ** 31);
expect(result).toEqual({ name: "Error", code: "ENOMEM" });
expect(exitCode).toBe(0);
});

test("2^31 - 1 bytes still decodes", async () => {
const { result, exitCode } = await spawnRead(2 ** 31 - 1);
expect(result).toEqual({ length: 2 ** 31 - 1 });
expect(exitCode).toBe(0);
});
});

// The UTF-8 -> UTF-16 converters behind `fs.readFile*(.., "utf8")`,
// `Buffer.prototype.toString("utf8")` and `TextDecoder.decode` must surface a
// failed output-buffer allocation as a catchable error, never a process abort
Expand Down
96 changes: 88 additions & 8 deletions test/js/web/fetch/blob-oom.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { setSyntheticAllocationLimitForTesting } from "bun:internal-for-testing";
import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, test } from "bun:test";
import { unlinkSync } from "fs";
import { tempDirWithFiles } from "harness";
import { truncateSync, unlinkSync, writeFileSync } from "fs";
import { bunEnv, bunExe, tempDir, tempDirWithFiles } from "harness";
import os from "node:os";
import path from "path";
describe("Memory", () => {
beforeAll(() => {
Expand All @@ -20,13 +21,15 @@ describe("Memory", () => {
test(".json() should throw an OOM without crashing the process.", () => {
const array = [buf, buf, buf, buf, buf, buf, buf, buf, buf];
expect(async () => await new Blob(array).json()).toThrow(
"Cannot parse a JSON string longer than 2^32-1 characters",
"Cannot parse a JSON string longer than 2147483647 characters",
);
});

test(".text() should throw an OOM without crashing the process.", () => {
const array = [buf, buf, buf, buf, buf, buf, buf, buf, buf];
expect(async () => await new Blob(array).text()).toThrow("Cannot create a string longer than 2^32-1 characters");
expect(async () => await new Blob(array).text()).toThrow(
"Cannot create a string longer than 2147483647 characters",
);
});

test(".bytes() should throw an OOM without crashing the process.", () => {
Expand All @@ -52,7 +55,7 @@ describe("Memory", () => {

test(".text() should throw an OOM without crashing the process.", () => {
expect(async () => await new Response(blob).text()).toThrow(
"Cannot create a string longer than 2^32-1 characters",
"Cannot create a string longer than 2147483647 characters",
);
});

Expand All @@ -66,7 +69,7 @@ describe("Memory", () => {

test(".json() should throw an OOM without crashing the process.", async () => {
expect(async () => await new Response(blob).json()).toThrow(
"Cannot parse a JSON string longer than 2^32-1 characters",
"Cannot parse a JSON string longer than 2147483647 characters",
);
});
});
Expand All @@ -83,7 +86,7 @@ describe("Memory", () => {

test(".text() should throw an OOM without crashing the process.", () => {
expect(async () => await new Request("http://localhost:3000", { body: blob }).text()).toThrow(
"Cannot create a string longer than 2^32-1 characters",
"Cannot create a string longer than 2147483647 characters",
);
});

Expand All @@ -97,7 +100,7 @@ describe("Memory", () => {

test(".json() should throw an OOM without crashing the process.", async () => {
expect(async () => await new Request("http://localhost:3000", { body: blob }).json()).toThrow(
"Cannot parse a JSON string longer than 2^32-1 characters",
"Cannot parse a JSON string longer than 2147483647 characters",
);
});
});
Expand Down Expand Up @@ -142,3 +145,80 @@ describe("Bun.file", () => {
expect(async () => await Bun.file(tmpFile).arrayBuffer()).not.toThrow();
});
});

// Byte lengths in [2^31, 2^32) used to abort the process instead of throwing:
// the Rust-side guards in front of WTF string construction only checked the
// synthetic allocation limit (2^32 - 1 by default) and missed
// WTF::StringImpl::MaxLength (2^31 - 1), tripping
// "ASSERTION FAILED: data.size() <= MaxLength" / a RELEASE_ASSERT in
// StringImplShape. Lengths >= 2^32 were already caught. These allocate a real
// 2 GiB, so each case runs in a subprocess to keep the peak away from the
// test runner, and the block skips on small machines (same gate as
// buffer.test.js's 4 GiB case).
describe.skipIf(os.totalmem() < 10 * 1024 ** 3)("byte sources at the 2 GiB string limit", () => {
test("Blob.text() and Blob.json() at 2^31 bytes throw ERR_STRING_TOO_LONG instead of aborting", async () => {
await using proc = Bun.spawn({
cmd: [
bunExe(),
"-e",
`
const results = [];
const report = e => ({ name: e.name, code: e.code, message: e.message });
const blob = new Blob([new Uint8Array(2 ** 31)]);
await blob.text().then(() => results.push("TEXT_UNEXPECTED_SUCCESS"), e => results.push(report(e)));
await blob.json().then(() => results.push("JSON_UNEXPECTED_SUCCESS"), e => results.push(report(e)));
console.log(JSON.stringify(results));
`,
],
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(JSON.parse(stdout.trim() || JSON.stringify({ stdout, stderr, exitCode }))).toEqual([
{
name: "Error",
code: "ERR_STRING_TOO_LONG",
message: "Cannot create a string longer than 2147483647 characters",
},
{
name: "Error",
code: "ERR_STRING_TOO_LONG",
message: "Cannot parse a JSON string longer than 2147483647 characters",
},
]);
expect(exitCode).toBe(0);
});

test("Bun.file().text() at 2^31 bytes throws ERR_STRING_TOO_LONG instead of aborting", async () => {
using dir = tempDir("blob-2gib", {});
const file = path.join(String(dir), "big.txt");
// Sparse where the filesystem supports it; reads back as 'x' + NUL bytes.
writeFileSync(file, "x");
truncateSync(file, 2 ** 31);
await using proc = Bun.spawn({
cmd: [
bunExe(),
"-e",
`
const results = [];
const report = e => ({ name: e.name, code: e.code, message: e.message });
await Bun.file(${JSON.stringify(file)}).text().then(() => results.push("UNEXPECTED_SUCCESS"), e => results.push(report(e)));
console.log(JSON.stringify(results));
`,
],
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(JSON.parse(stdout.trim() || JSON.stringify({ stdout, stderr, exitCode }))).toEqual([
{
name: "Error",
code: "ERR_STRING_TOO_LONG",
message: "Cannot create a string longer than 2147483647 characters",
},
]);
expect(exitCode).toBe(0);
});
});