Conversation
Adds the tests from #38422 and #38428. In a `bun build --compile` executable, a `./` specifier longer than the path buffer passed to `new Worker`, a Worker `preload`, `import()` or `require()` aborted the process with `panic: range end index 5012 out of range for slice of length 4095`. #40619 moved the embedded-module lookup into `StandaloneModuleGraph::resolve`, which joins with a checked join and guards the `.js` append, so such a specifier now falls through to the regular resolver. No test on main exercises that path: the 70000 byte probe in `compile/EmbeddedResolveMisses` is rejected by the 1.5x PATH_MAX cap before it reaches the resolver.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 6 remain after this review. WalkthroughChangesThe pull request adds regression tests for oversized runtime specifiers in compiled executables. The tests cover module resolution, Worker error events, and oversized Worker preload paths. Compiled runtime path handling
Suggested reviewers: Merge Risk: ⚪ Minimal · up to This is a localized test-only change covering over-long relative specifiers in compiled executables; no actionable merge-blocking risk remains after normal checks and review. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Full details: Description checkExplanation The description is complete and relevant. It explains the problem, test-only scope, affected cases, expected behavior, platform-specific coverage, and verification results. It does not use the template headings exactly, but it provides the required information. Comment |
|
Status: #38422 and #38428 are closed as superseded by #40619. Their three tests are carried here unchanged apart from the comments. Verified: they abort on the pre-#40619 build (1.4.1-canary d578a8c) and pass on a debug build of main at f189103 with no change under CI (build 108367): 180 of 181 jobs pass. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — test-only regression coverage for already-fixed panics, follows the file's itBundled conventions exactly.
What was reviewed:
- All three tests await conditions (
Promise.withResolverswired toonerror, awaitedimport(), sync try/catch) — no sleeps or timeouts. - Magic lengths are justified in comments (PATH_MAX per platform, 1.5× cap, one-byte-short-then-
.js-append boundary) and useBuffer.alloc(n, fill)per the debug-JSC guidance. RelativeSpecifierLongerThanPathMaxindirectsrequire/importthrough locals so the bundler can't resolve them at build time, and re-resolves an embedded sibling afterward to prove the graph is still usable.
Extended reasoning...
Overview
This PR adds three itBundled compile-mode test cases to test/bundler/bundler_compile.test.ts, covering over-long relative specifiers in standalone (--compile) executables: runtime require()/import() past PATH_MAX, new Worker(spec) past the fixed path buffer (including the exact one-byte-short boundary where only the appended .js overflows), and an over-long Worker preload path. No source code is touched — the panics were already fixed by #40619; this backfills the missing coverage.
Security risks
None. Test-only additions with no runtime, build, or dependency changes. The fixtures run inside the bundler test harness's spawned subprocesses and touch no network or shared filesystem state.
Level of scrutiny
Low. The change is additive test coverage placed next to sibling tests (compile/EmbeddedResolveMisses, compile/WorkerRelativePathTSExtension) using the identical itBundled shape: compile: true, files, entryPointsRaw, run: { stdout, setCwd }. The PR description documents that all three fail with SIGABRT on the pre-fix canary and pass on bun bd test at main, satisfying the "test must fail with USE_SYSTEM_BUN=1" rule.
Other factors
The tests satisfy the REVIEW.md checklist for tests reviewers reject: Buffer.alloc over .repeat, no per-test timeouts, condition-awaited rather than time-awaited, specific error codes asserted (MODULE_NOT_FOUND / ERR_MODULE_NOT_FOUND) rather than bare "threw", comments cite the source constants behind every platform-branched length, and the Worker test enumerates the boundary matrix (./, ../, and each platform's buffer size). The req/imp indirection wrappers correctly defeat build-time resolution so the runtime path is exercised. No prior reviews or outstanding objections on the timeline.
This branch has not been deployed
1 participant
Problem
bun build --compileexecutable, a./specifier longer than the path buffer aborted the process innew Worker, a Workerpreload,import()andrequire():panic: range end index 5012 out of range for slice of length 4095. worker: do not abort a compiled executable on a relative specifier longer than the path buffer #38422 and compile: do not abort on import()/require() of a relative specifier longer than the path buffer #38428 fixed the two code paths separately.StandaloneModuleGraph::resolve(src/resolver/standalone_module_graph.rs:25). It joins withjoin_abs_string_buf_checkedand checksstem_len + 3 > buf.len()before it appends.js. Both panics are gone on main.compile/EmbeddedResolveMisseshits the 1.5xPATH_MAXcap before the resolver.Fix
test/bundler/bundler_compile.test.ts:compile/WorkerRelativePathLongerThanPathBuffer,compile/WorkerPreloadLongerThanPathBufferandcompile/RelativeSpecifierLongerThanPathMax. The comments describe the code after compile: resolve Worker, import() and require() specifiers against embedded modules consistently (incl. Windows) #40619.Runtime failed with SIGABRTand the panic above. On main at f189103 (debug build) all three pass.Background
/$bunfs/root/,B:/~BUN/root/on Windows). A relative specifier is joined onto that root and looked up in the graph, as spelled and under its.jsname.PathBufferis a fixed scratch buffer ofMAX_PATH_BYTES(1024 on macOS, 4096 on Linux, 98302 on Windows). The old join wrote into it unchecked.import()andrequire()specifiers longer than 1.5xMAX_PATH_BYTESare rejected before resolution. Worker specifiers have no cap. The test lengths sit between the two bounds on each platform.Notes
Hand probes on main with a compiled two-file app from a cwd 71 bytes deep.
./specifiers of 4081 to 4085 bytes (the.jsappend window on Linux), 5000, 6145, 98288 and 100000 bytes: the worker fireserror, the preload constructor throws,requirereportsMODULE_NOT_FOUND,importreportsERR_MODULE_NOT_FOUND(or the ENAMETOOLONG cap past 6144 bytes). No abort.One neighbouring abort remains and is out of scope here. From a short cwd (
/tmp/probe), a./specifier of 4081 to 4085 bytes panics inResolver::load_extension(src/resolver/resolver.rs:6012) withrange end index 4097 out of range for slice of length 4096. That is the regular resolver's extension probe. It reproduces with plainbun main.jsas well, before and after #40619. #39626 fixes it.Test runs:
USE_SYSTEM_BUN=1 bun test test/bundler/bundler_compile.test.ts -t "LongerThanPathBuffer|LongerThanPathMax"on 1.4.1-canary d578a8c: 0 pass, 3 fail (SIGABRT).bun bd test test/bundler/bundler_compile.test.ts -t "LongerThanPathBuffer|LongerThanPathMax"on main f189103: 3 pass.bun bd test test/bundler/bundler_compile.test.ts: 85 pass, 1 fail (compile/HelloWorldWithProcessVersionsBun, which also fails on main without this change).no test proof · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/bundler/bundler_compile.test.ts