Repository navigation
Conversation
A working directory can be PATH_MAX - 1 bytes long. Three joins of <cwd>/<name> assumed that the result fits a path buffer: - PackageJSON::parse joined <dir>/package.json into the 4096-byte thread-local join buffer. - dir_info_uncached joined <dir>/tsconfig.json and <dir>/jsconfig.json into a PathBuffer. - PackageManager::init copied <cwd>/package.json plus a NUL into a pooled PathBuffer. From a working directory of 4083..4095 bytes (legal on Linux) bun run, bun -e, bun test, bun install and every bun pm command aborted with a slice index panic. PackageJSON::parse now uses join_abs_string_spill. The file is opened relative to the directory fd, so bun run works from such a directory. A tsconfig.json or jsconfig.json whose path does not fit is skipped, because baseUrl, paths and extends are joined onto that path in path buffers. PackageManager::init builds the path on the heap and lets File::openat report ENAMETOOLONG, which takes the existing could-not-open route and exits 1.
|
Status Reproduction (Linux, release build of main): make a working directory of 4090 bytes out of 200-byte components, write a With a With this branch: Tests: Self-reviewed: 12 concerns raised. Addressed: the |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Essentials Run ID: 📒 Files selected for processing (5)
Included review availability: Your plan provides up to 10 included reviews per hour; 0 remain after this review. WalkthroughThe change updates package and configuration path construction near ChangesPATH_MAX support
Suggested reviewers: Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to The PATH_MAX handling changes and boundary coverage do not leave an identified merge-blocking risk. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| const PATH_MAX = process.platform === "linux" || process.platform === "android" ? 4096 : 1024; | ||
| const SEGMENT = Buffer.alloc(200, "d").toString(); | ||
|
|
||
| /** An absolute path below `root` whose UTF-8 encoding is exactly `length` bytes long. */ | ||
| function pathOfLength(root: string, length: number): string { | ||
| let path = root; | ||
| // Leave room for the final component, which has to stay under NAME_MAX (255). | ||
| while (length - Buffer.byteLength(path) > 256) path = join(path, SEGMENT); | ||
| path = join(path, Buffer.alloc(length - Buffer.byteLength(path) - 1, "L").toString()); | ||
| expect(Buffer.byteLength(path)).toBe(length); | ||
| return path; | ||
| } |
There was a problem hiding this comment.
🟡 nit (optional): The PATH_MAX constant, SEGMENT, and pathOfLength() helper are byte-identical across test/cli/run/run_command.test.ts and test/cli/install/bun-pm.test.ts; REVIEW.md requires extracting a duplicated multi-line block to a shared helper the second time it appears in the same diff. Fix: move pathOfLength (and its PATH_MAX/SEGMENT inputs) into test/harness.ts and import it in both suites, which covers the 2 sites listed. Same pattern at 2 sites (test/cli/run/run_command.test.ts:50, test/cli/install/bun-pm.test.ts:1104).
Extended reasoning...
Both new test suites define the same 8-line pathOfLength helper plus const PATH_MAX = process.platform === "linux" || process.platform === "android" ? 4096 : 1024; and const SEGMENT = Buffer.alloc(200, "d").toString(); verbatim. REVIEW.md ("Code style & idioms reviewers enforce"): "The second time a multi-line block appears in your diff, extract a named helper and use it at EVERY parallel site." A future tweak to the length arithmetic (e.g. NAME_MAX handling or a new platform's PATH_MAX) has to be applied twice or the two suites diverge. No PATH_MAX/pathOfLength helper exists in test/harness.ts today, so the extraction target is harness.
Verification: nit — The duplication is real and byte-identical across both files added in this diff. test/cli/run/run_command.test.ts (added block starting ~line 50): ``` const PATH_MAX = process.platform === "linux" || process.platform === "android" ? 4096 : 1024; const SEGMENT = Buffer.alloc(200, "d").toString(); /** An absolute path below root whose UTF-8 encoding is exactly `length` bytes long.…
|
This is Claude (an AI assistant) writing on Jarred's behalf. Included in #44968, in a different form: a package.json, tsconfig.json or jsconfig.json whose path does not fit is treated as unreadable instead of being read. |
Problem
bun run <script>,bun -e,bun test,bun installand everybun pmcommand abort.bun run:panic: range end index 4102 out of range for slice of length 4095.bun install,bun pm cache:panic: range end index 4103 out of range for slice of length 4096.<cwd>/<name>:package.jsoninPackageJSON::parse(src/resolver/package_json.rs:385),tsconfig.jsonandjsconfig.jsonindir_info_uncached(src/resolver/resolver.rs:6400),package.jsoninPackageManager::init(src/install/PackageManager.rs:1586).Fix
PackageJSON::parseusesjoin_abs_string_spill. The resolver opens the file relative to the directory fd, sobun run <script>works. resolver: report candidate paths that do not fit a path buffer as not found instead of aborting #39626 hides such apackage.jsonfrom the resolver instead. A maintainer has to pick one (see notes).dir_info_uncachedusesabs_buf_checkedand skips atsconfig.jsonorjsconfig.jsonwhose path does not fit:baseUrl,pathsandextendsare joined onto that path.PackageManager::initbuilds the path in aVec.File::openatreturnsENAMETOOLONGfor it, which takes the existing route:ENAMETOOLONG: File name too long: could not open "<cwd>/package.json" (open), exit 1.test/cli/run/run_command.test.ts, 5 intest/cli/install/bun-pm.test.ts. The unfixed build fails 10.Background
PathBufferis bun's fixed buffer for one path:MAX_PATH_BYTESlong, NUL included (4096 on Linux, 1024 on macOS).join_abs_stringandabs_bufarepath.resolveinto a fixed buffer and assume the result fits.join_abs_string_spillfalls back to the heap.abs_buf_checkedreturnsNone.PackageManager::initruns first in every package manager command. It opens the nearestpackage.jsonby absolute path.Notes
Origin. Found by an internal report, reproduced on the 1.4.3-canary release build. There is no GitHub issue.
Behavior by cwd length, Linux x64, release build of main against this branch. The project has a
package.jsonwith"scripts": {"hi": "echo hi"}.bun run hibeforebun install,bun pm cachebeforepanic: range end index 4096 out of range for slice of length 4095with atsconfig.jsonpanic: index out of bounds: the len is 4096 but the index is 4096panic: range end index 4096 out of range for slice of length 4095panic: range end index 4097 out of range for slice of length 4096panic: range end index 4107 out of range for slice of length 4095panic: range end index 4108 out of range for slice of length 4096bun run hiandbun -eprint their output and exit 0 at every length from 4079 to 4095, with and without abun initstyletsconfig.json.tsconfig.jsonboundary is exact. With"jsxImportSource": "zzfoo",bun -e 'console.log(<div/>)'fails onzzfoo/jsx-runtimeat a cwd of 4082 bytes (config loaded) and prints<div />at 4083 (config skipped).The contract choice against #39626. #39626 (open) makes
DirEntry::add_entrydrop every directory entry whose absolute path does not fit a path buffer. With it,package.json,tsconfig.jsonandjsconfig.jsonin such a directory do not exist for the resolver, andbun run hireportsScript not found "hi". This PR has the same result for the two config files and a different result forpackage.json: the resolver reads it throughopenat(dir_fd, "package.json"), as it already does on macOS for a path between 1024 and 4096 bytes. If #39626 lands in its current shape after this PR, the twopackage.jsonrows of the newbun runtests must expectScript not found. If a maintainer prefers that contract now, thePackageManager.rshunk and thebun-pm.test.tstests stand alone.Why
PackageManager::initstops and does not continue upward. Apackage.jsoncan exist in that directory, and the command would then run against a parent project without a word. The function already stops onEACCES,EISDIR,ELOOPand every other error that is notENOENT. The full output is the existing two lines for a failed open ininit. The second line iserror: An internal error occurred (ENAMETOOLONG)in a release build, the same as for apackage.jsondirectory (EISDIR) today. #39650 makes the--filterwalk infilter_arg.rscontinue upward for the same condition. That walk already continues onENOENT,EACCESandEPERM, so each function keeps its own rule for an unreadable file.What still does not work from such a directory.
bun run <script>fails withE2BIG: /usr/bin/bash: Argument list too long (posix_spawn())when the path has many short components (81 components at 4090 bytes fail, 61 pass).bun runadds<ancestor>/node_modules/.bintoPATHfor each ancestor. This is not new and not tied toPATH_MAX: main does the same from a 2000-byte cwd with 10-byte components.Module not found, because its absolute path does not fit.package.jsonwith asideEffectsarray still aborts in thesideEffectsjoin (resolver: don't abort on a package.json sideEffects entry longer than the join buffer #37529).bun run --filterstill aborts (bun run --filter: stop panicking on workspace paths longer than the path buffer #39650).file:andlink:dependencies: install: stop aborting on link: specifiers and patch paths longer than the path buffers #38359.$HOME/.npmrc: install: do not abort when $XDG_CONFIG_HOME or $HOME is too long for the .npmrc path buffer #38372.bun execaborts from a cwd of 4090 bytes or more (src/runtime/cli/exec_command.rs:69, the<cwd>/[eval]label).bun pm binaborts from a cwd of 4079..4082 bytes (src/runtime/cli/package_manager_command.rs:366,<cwd>/node_modules/.bin). No open PR covers these two.baseUrlandextendsjoins of atsconfig.jsonthat is loaded (cwd of 4082 bytes or less) are unchanged. resolver: report candidate paths that do not fit a path buffer as not found instead of aborting #39626 and Report an error instead of aborting when a user-supplied path does not fit a path buffer #41760 both change them.package_json.rshunk redundant and harmless. Keep the process's working directory in one place, bun_core::cwd #40372 rewrites other parts ofPackageManager::initand leaves this loop as it is.Other details.
init(the workspace root search) copies<parent>/package.jsoninto aPathBufferin the same way. It needs no change: each parent is shorter than the directory whosepackage.jsonthe first loop opened.bun run --filterpassesFd::invalid()),read_file_with_allocatoropens the absolute path, getsENAMETOOLONGfromopenat_a, andPackageJSON::parselogsCannot read file "<dir>": ENAMETOOLONGand returnsNone. That is its existing route for a failed open.PATH_MAXis 1024. The 4096-byte thread-local join buffer cannot overflow from a legal directory there, but the twoPathBuffersites can (cwd of 1010..1023 bytes). The tests size the directory fromPATH_MAXper platform. Windows is skipped: the path buffer there is longer than any path the OS accepts.renameSync, because a file in the final directory is too long to write by its absolute path.test/cli/run/run_command.test.ts,filter-workspace.test.ts,tsconfig-override.test.ts,test/cli/install/bun-pm.test.ts,bun-pm-pkg.test.ts,bun-add.test.ts,bun-remove.test.ts,bun-workspaces.test.ts,bad-workspace.test.ts,test/config/bunfig/bunfig-errors.test.ts,test/bundler/esbuild/tsconfig.test.ts,test/js/bun/resolve/resolve.test.ts,resolve-error.test.ts,import-meta.test.js,tsconfig-extends-leak.test.ts.cargo clippy -p bun_resolver -p bun_installandcargo fmt --checkare clean.no test proof · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/cli/run/run_command.test.ts, test/cli/install/bun-pm.test.ts