resolver: don't abort on a package.json browser map key longer than 1024 bytes

Merged
RO
3mo ago
+232-22

Reproduction

Any package.json the resolver reads (a dependency in node_modules, or the project's own) whose "browser" object has a key longer than 1024 bytes:

mkdir -p node_modules/x && cd node_modules/x
bun -e 'require("fs").writeFileSync("package.json", JSON.stringify({ name: "x", main: "index.js", browser: { ["./" + "b".repeat(1100) + ".js"]: "./y.js" } }))'
echo 'module.exports = 1;' > index.js && cd ../..
echo 'console.log(require("x"))' > index.js
bun index.js
panic: range end index 1103 out of range for slice of length 1024

The process aborts (exit 134) as soon as the package.json is parsed, so it happens at runtime too, not only for --target=browser (the browser map is read regardless of target because parsed package.json files are cached). Expected: prints 1; the mapping is just an entry that nothing matches.

Cause

PackageJSON::parse (src/resolver/package_json.rs) normalizes every browser map key with resolve_path::normalize_string, which writes its output into the thread-local PARSER_BUFFER, a fixed [u8; 1024] (src/paths/resolve_path.rs), with no bounds check. Keys are arbitrary strings from the package.json, so any key whose normalized form is longer than the buffer aborts. The parse side had this limit since the Zig version (same 1024-byte parser_buffer).

Fix

Add resolve_path::normalize_string_spill, the normalize_string counterpart of the existing join_spill / join_z_spill: it uses the thread-local buffer when the result is known to fit and otherwise normalizes into a caller-owned Vec that is grown as needed (untouched in the common case, so the fast path allocates nothing). The browser map loop uses it with one Vec hoisted out of the loop, the same shape as the join_spill callers in fs.readdir and Bun.Glob. StringArrayHashMap::put already copies the key, so the intermediate Box<[u8]> the old shim allocated per entry is gone as well.

The capacity check is input.len() + 1: normalizing only removes bytes, except that on Windows a drive-relative C: normalizes to C:. and a bare UNC volume gains its trailing separator, both one byte longer than the input. The unit test below pins the boundary case (a UNC volume exactly 1024 bytes long); dropping the + 1 makes it fail with the same out-of-range panic.

The remaining normalize_string callers (FileSystem::normalize used by the install folder resolver, the shell rm check being fixed in #37521, and a Windows-only path in run_command.rs) are separate call sites and can move to the new helper independently; the sideEffects entries a few lines below the browser map go through the 4096-byte join buffer instead and are tracked separately too.

Verification

test/js/bun/resolve/resolve.test.ts, next to the existing browser-map long-path test: a dependency with an over-long key is required at runtime, a browser build with an over-long key still applies the other entries of the map, and (Linux/Windows, where PATH_MAX allows the lookup) a bare specifier of that length is actually remapped through the over-long key.

USE_SYSTEM_BUN=1 bun test test/js/bun/resolve/resolve.test.ts -t "browser map with a key longer"
  3 fail (each child aborts with the panic above)

bun bd test test/js/bun/resolve/resolve.test.ts
  76 pass, 2 skip
bun bd test test/bundler/bundler_browser.test.ts test/bundler/esbuild/packagejson.test.ts
  all pass (browser map behavior for normal keys is unchanged)

cargo test -p bun_paths            18 pass (4 new, covering the fast path, the spill, spill reuse, and the +1 boundary)
bun run rust:miri -p bun_paths     18 pass

The new unit tests are the first in bun_paths to reach bun_core::strings' byte searches, so the test module defines scalar stubs for the two highway kernels they reference, the same way string_paths.rs stubs simdutf; under Miri the wrappers take their own scalar path and the stubs are never called.