diff --git a/src/bun_core/string/mod.rs b/src/bun_core/string/mod.rs index 0330f9f437e9..19f1c54608ee 100644 --- a/src/bun_core/string/mod.rs +++ b/src/bun_core/string/mod.rs @@ -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 @@ -299,7 +299,7 @@ impl String { } let () = AssertPtrSized::::OK; debug_assert!(!bytes.is_empty()); - if bytes.len() >= Self::max_length() { + if bytes.len() > Self::max_length() { callback(ctx, bytes.as_ptr().cast_mut().cast::(), bytes.len()); return Self::DEAD; } @@ -322,7 +322,7 @@ impl String { ExternalStringImplFreeFunction, 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(), @@ -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`]. #[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 @@ -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 @@ -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 @@ -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. +pub const WTF_STRING_MAX_LENGTH: usize = i32::MAX as usize; + // ────────────────────────────────────────────────────────────────────────── // move-in: printer (MOVE_DOWN ← `bun_js_printer`) // diff --git a/src/jsc/ZigString.rs b/src/jsc/ZigString.rs index 1a82c01285c2..3058fa4804a8 100644 --- a/src/jsc/ZigString.rs +++ b/src/jsc/ZigString.rs @@ -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; diff --git a/src/jsc/bindings/BunString.cpp b/src/jsc/bindings/BunString.cpp index 08d00c72f9ad..282d142f820f 100644 --- a/src/jsc/bindings/BunString.cpp +++ b/src/jsc/bindings/BunString.cpp @@ -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(reinterpret_cast(ptr), length)))); } diff --git a/src/jsc/bindings/bindings.cpp b/src/jsc/bindings/bindings.cpp index 4069e991a080..b3f4c90e1823 100644 --- a/src/jsc/bindings/bindings.cpp +++ b/src/jsc/bindings/bindings.cpp @@ -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 {}; } } diff --git a/src/jsc/lib.rs b/src/jsc/lib.rs index 0fb901df1940..b59ace1f259f 100644 --- a/src/jsc/lib.rs +++ b/src/jsc/lib.rs @@ -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; @@ -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; diff --git a/test/js/node/fs/fs-oom.test.ts b/test/js/node/fs/fs-oom.test.ts index ea453cd11874..b2951b6756c8 100644 --- a/test/js/node/fs/fs-oom.test.ts +++ b/test/js/node/fs/fs-oom.test.ts @@ -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); @@ -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 diff --git a/test/js/web/fetch/blob-oom.test.ts b/test/js/web/fetch/blob-oom.test.ts index dc7dd16bc092..03c54b6d2b90 100644 --- a/test/js/web/fetch/blob-oom.test.ts +++ b/test/js/web/fetch/blob-oom.test.ts @@ -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(() => { @@ -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.", () => { @@ -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", ); }); @@ -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", ); }); }); @@ -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", ); }); @@ -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", ); }); }); @@ -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); + }); +});