-
Notifications
You must be signed in to change notification settings - Fork 3.8k
fix(fs): handle ENAMETOOLONG for paths exceeding PATH_MAX #25700
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
fix(fs): handle ENAMETOOLONG for paths exceeding PATH_MAX #25700
Conversation
When fs.stat, fs.lstat, fs.readFile, etc. are called with paths longer than PATH_MAX (4096 bytes), they now properly return an ENAMETOOLONG error instead of crashing or causing undefined behavior. This adds new *A suffix functions (statA, lstatA, statxA, lstatxA) to sys.zig that accept non-null-terminated slices and handle ENAMETOOLONG by catching the error from std.posix.toPosixPath(). The fs operations now use these *A functions directly with the path slice, avoiding the sliceZ() call which could cause buffer overflow on paths exceeding PATH_MAX. Note: The callback-based fs.readFile still throws synchronously for ENAMETOOLONG instead of passing it to the callback. This requires additional work to fix properly and is tracked as a TODO. Fixes oven-sh#25659 (partial) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
WalkthroughIntroduces path length validation and ENAMETOOLONG error handling for filesystem operations. Adds new wrapper functions in Changes
Possibly related PRs
Suggested reviewers
Pre-merge checks✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/bun.js/node/node_fs.zig (1)
4954-4999: readFileWithOptions: unsafe union field access before tag validation
args.pathis aPathOrFileDescriptortagged union with variants.fdand.path. The current code accesses the.pathfield unconditionally before switching on the tag:const path_slice = args.path.path.slice(); // ← Accesses inactive field when tag is .fd const fd_maybe_windows: FileDescriptor = switch (args.path) { .path => brk: { ... }, .fd => |fd| fd, };This reads an inactive union field when the tag is
.fd, which is undefined behavior in Zig and can corrupt state or crash for calls passing a file descriptor instead of a path (e.g.,fs.readFile(fd),fs.readFileSync(fd)).Move
path_slicecomputation inside the.patharm and use the captured payload:- pub fn readFileWithOptions(this: *NodeFS, args: Arguments.ReadFile, comptime flavor: Flavor, comptime string_type: StringType) Maybe(Return.ReadFileWithOptions) { - const path_slice = args.path.path.slice(); - const fd_maybe_windows: FileDescriptor = switch (args.path) { - .path => brk: { - // Only check StandaloneModuleGraph if path fits in buffer (otherwise it can't be in the graph) - if (path_slice.len < bun.MAX_PATH_BYTES) { - if (bun.StandaloneModuleGraph.get()) |graph| { - const path = args.path.path.sliceZ(&this.sync_error_buf); + pub fn readFileWithOptions(this: *NodeFS, args: Arguments.ReadFile, comptime flavor: Flavor, comptime string_type: StringType) Maybe(Return.ReadFileWithOptions) { + const fd_maybe_windows: FileDescriptor = switch (args.path) { + .path => |path_like| brk: { + const path_slice = path_like.slice(); + // Only check StandaloneModuleGraph if path fits in buffer (otherwise it can't be in the graph) + if (path_slice.len < bun.MAX_PATH_BYTES) { + if (bun.StandaloneModuleGraph.get()) |graph| { + const path = path_like.sliceZ(&this.sync_error_buf); if (graph.find(path)) |file| { if (args.encoding == .buffer) { return .{
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
src/bun.js/node/node_fs.zigsrc/sys.zigtest/js/node/fs/fs.test.ts
🧰 Additional context used
📓 Path-based instructions (5)
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}
📄 CodeRabbit inference engine (test/CLAUDE.md)
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}: Usebun:testwith files that end in*.test.{ts,js,jsx,tsx,mjs,cjs}
Do not write flaky tests. Never wait for time to pass in tests; always wait for the condition to be met instead of using an arbitrary amount of time
Never use hardcoded port numbers in tests. Always useport: 0to get a random port
Prefer concurrent tests over sequential tests usingtest.concurrentordescribe.concurrentwhen multiple tests spawn processes or write files, unless it's very difficult to make them concurrent
When spawning Bun processes in tests, usebunExeandbunEnvfromharnessto ensure the same build of Bun is used and debug logging is silenced
Use-eflag for single-file tests when spawning Bun processes
UsetempDir()from harness to create temporary directories with files for multi-file tests instead of creating files manually
Prefer async/await over callbacks in tests
When callbacks must be used and it's just a single callback, usePromise.withResolversto create a promise that can be resolved or rejected from a callback
Do not set a timeout on tests. Bun already has timeouts
UseBuffer.alloc(count, fill).toString()instead of'A'.repeat(count)to create repetitive strings in tests, as ''.repeat is very slow in debug JavaScriptCore builds
Usedescribeblocks for grouping related tests
Always useawait usingorusingto ensure proper resource cleanup in tests for APIs like Bun.listen, Bun.connect, Bun.spawn, Bun.serve, etc
Always check exit codes and test error scenarios in error tests
Usedescribe.each()for parameterized tests
UsetoMatchSnapshot()for snapshot testing
UsebeforeAll(),afterEach(),beforeEach()for setup/teardown in tests
Track resources (servers, clients) in arrays for cleanup inafterEach()
Files:
test/js/node/fs/fs.test.ts
**/*.test.ts?(x)
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.ts?(x): Never usebun testdirectly - always usebun bd testto run tests with debug build changes
For single-file tests, prefer-eflag overtempDir
For multi-file tests, prefertempDirandBun.spawnover single-file tests
UsenormalizeBunSnapshotto normalize snapshot output of tests
Never write tests that check for 'panic', 'uncaught exception', or similar strings in test output
UsetempDirfromharnessto create temporary directories - do not usetmpdirSyncorfs.mkdtempSync
When spawning processes in tests, expect stdout before expecting exit code for more useful error messages on test failure
Do not write flaky tests - do not usesetTimeoutin tests; instead await the condition to be met
Verify tests fail withUSE_SYSTEM_BUN=1 bun test <file>and pass withbun bd test <file>- tests are invalid if they pass with USE_SYSTEM_BUN=1
Test files must end with.test.tsor.test.tsx
Avoid shell commands likefindorgrepin tests - use Bun's Glob and built-in tools instead
Files:
test/js/node/fs/fs.test.ts
test/**/*.test.ts?(x)
📄 CodeRabbit inference engine (CLAUDE.md)
Always use
port: 0in tests - do not hardcode ports or use custom random port number functions
Files:
test/js/node/fs/fs.test.ts
src/**/*.zig
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/*.zig: Private fields in Zig are fully supported using the#prefix:struct { #foo: u32 };
Use decl literals in Zig for declaration initialization:const decl: Decl = .{ .binding = 0, .value = 0 };
Prefer@importat the bottom of the file (auto formatter will move them automatically)
Files:
src/sys.zigsrc/bun.js/node/node_fs.zig
**/*.zig
📄 CodeRabbit inference engine (CLAUDE.md)
In Zig code, be careful with allocators and use defer for cleanup
Files:
src/sys.zigsrc/bun.js/node/node_fs.zig
🧠 Learnings (7)
📓 Common learnings
Learnt from: RiskyMH
Repo: oven-sh/bun PR: 22606
File: src/glob/GlobWalker.zig:449-452
Timestamp: 2025-09-12T18:16:50.754Z
Learning: For Bun codebase: prefer using `std.fs.path.sep` over manual platform separator detection, and use `bun.strings.lastIndexOfChar` instead of `std.mem.lastIndexOfScalar` for string operations.
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 23373
File: src/bun.js/api/BunObject.zig:2514-2521
Timestamp: 2025-10-08T13:56:00.875Z
Learning: For Bun codebase: prefer using `bun.path` utilities (e.g., `bun.path.joinAbsStringBuf`, `bun.path.join`) over `std.fs.path` functions for path operations.
📚 Learning: 2025-11-07T17:28:51.204Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 24423
File: test/js/node/path/browserify.test.js:472-475
Timestamp: 2025-11-07T17:28:51.204Z
Learning: In test files under test/js/node/path/, markovejnovic prefers functional programming patterns (e.g., reduce, map, filter chains) over imperative loops for clarity and readability.
Applied to files:
test/js/node/fs/fs.test.tssrc/bun.js/node/node_fs.zig
📚 Learning: 2025-09-12T18:16:50.754Z
Learnt from: RiskyMH
Repo: oven-sh/bun PR: 22606
File: src/glob/GlobWalker.zig:449-452
Timestamp: 2025-09-12T18:16:50.754Z
Learning: For Bun codebase: prefer using `std.fs.path.sep` over manual platform separator detection, and use `bun.strings.lastIndexOfChar` instead of `std.mem.lastIndexOfScalar` for string operations.
Applied to files:
src/bun.js/node/node_fs.zig
📚 Learning: 2025-10-08T13:56:00.875Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 23373
File: src/bun.js/api/BunObject.zig:2514-2521
Timestamp: 2025-10-08T13:56:00.875Z
Learning: For Bun codebase: prefer using `bun.path` utilities (e.g., `bun.path.joinAbsStringBuf`, `bun.path.join`) over `std.fs.path` functions for path operations.
Applied to files:
src/bun.js/node/node_fs.zig
📚 Learning: 2025-11-24T18:37:11.466Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js} : Use `process.platform` and `process.arch` for platform detection; these values are inlined and dead-code eliminated at build time
Applied to files:
src/bun.js/node/node_fs.zig
📚 Learning: 2025-11-24T18:37:11.466Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Write JS builtins for Bun's Node.js compatibility and APIs, and run `bun bd` after changes
Applied to files:
src/bun.js/node/node_fs.zig
📚 Learning: 2025-11-24T18:37:11.466Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js} : Use private globals and methods with `$` prefix (e.g., `$Array`, `map.$set()`) instead of public JavaScript globals
Applied to files:
src/bun.js/node/node_fs.zig
🔇 Additional comments (4)
src/sys.zig (2)
540-549: LGTM! Clean wrapper for handling non-null-terminated paths.The function correctly catches
std.posix.toPosixPath()errors and maps them toENAMETOOLONG, then delegates to the existingstat()function. The syscall tag is appropriate.
551-560: LGTM! Correct mirror of statA for lstat semantics.The function follows the same pattern as
statAand correctly delegates tolstat()with the appropriate syscall tag.src/bun.js/node/node_fs.zig (2)
3895-3919: *lstat: slice-based A syscall usage looks correctUsing
path_slice = args.path.slice()withlstatxA/lstatAand propagating errors viaerr.withPath(path_slice)preserves existing NOENT semantics while avoiding buffer-basedsliceZfor overlong paths. This is aligned with the ENAMETOOLONG handling goals.
5733-5769: stat: ENAMETOOLONG-safe path handling looks consistent
statnow usespath_slice = args.path.slice(), gatesStandaloneModuleGraphlookup onpath_slice.len < bun.MAX_PATH_BYTES, and callsstatxA/statAwithpath_slice, propagating errors viaerr.withPath(path_slice)while preserving NOENT/throwIfNoEntry semantics. This matches the intended ENAMETOOLONG behavior and is consistent with thelstatchanges.
| /// Like statx() but accepts a non-null-terminated slice and handles ENAMETOOLONG. | ||
| pub fn statxA(file_path: []const u8, comptime fields: []const StatxField) Maybe(PosixStat) { | ||
| const pathZ = std.posix.toPosixPath(file_path) catch return Maybe(PosixStat){ | ||
| .err = .{ | ||
| .errno = @intFromEnum(E.NAMETOOLONG), | ||
| .syscall = .stat, | ||
| }, | ||
| }; | ||
| return statx(&pathZ, fields); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use .statx syscall tag for consistency.
The error uses .syscall = .stat (line 710), but since this function delegates to statx() which invokes the statx syscall, the tag should be .statx for accurate error reporting.
🔎 Suggested fix
pub fn statxA(file_path: []const u8, comptime fields: []const StatxField) Maybe(PosixStat) {
const pathZ = std.posix.toPosixPath(file_path) catch return Maybe(PosixStat){
.err = .{
.errno = @intFromEnum(E.NAMETOOLONG),
- .syscall = .stat,
+ .syscall = .statx,
},
};
return statx(&pathZ, fields);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /// Like statx() but accepts a non-null-terminated slice and handles ENAMETOOLONG. | |
| pub fn statxA(file_path: []const u8, comptime fields: []const StatxField) Maybe(PosixStat) { | |
| const pathZ = std.posix.toPosixPath(file_path) catch return Maybe(PosixStat){ | |
| .err = .{ | |
| .errno = @intFromEnum(E.NAMETOOLONG), | |
| .syscall = .stat, | |
| }, | |
| }; | |
| return statx(&pathZ, fields); | |
| } | |
| /// Like statx() but accepts a non-null-terminated slice and handles ENAMETOOLONG. | |
| pub fn statxA(file_path: []const u8, comptime fields: []const StatxField) Maybe(PosixStat) { | |
| const pathZ = std.posix.toPosixPath(file_path) catch return Maybe(PosixStat){ | |
| .err = .{ | |
| .errno = @intFromEnum(E.NAMETOOLONG), | |
| .syscall = .statx, | |
| }, | |
| }; | |
| return statx(&pathZ, fields); | |
| } |
🤖 Prompt for AI Agents
In src/sys.zig around lines 705 to 714, the error constructed when toPosixPath
fails sets .syscall = .stat but this function delegates to statx and should use
the statx syscall tag; update the error to use .syscall = .statx (replace .stat
with .statx) so the error accurately identifies the syscall, then run a quick
build/test to ensure no enum name changes are needed.
| /// Like lstatx() but accepts a non-null-terminated slice and handles ENAMETOOLONG. | ||
| pub fn lstatxA(file_path: []const u8, comptime fields: []const StatxField) Maybe(PosixStat) { | ||
| const pathZ = std.posix.toPosixPath(file_path) catch return Maybe(PosixStat){ | ||
| .err = .{ | ||
| .errno = @intFromEnum(E.NAMETOOLONG), | ||
| .syscall = .lstat, | ||
| }, | ||
| }; | ||
| return lstatx(&pathZ, fields); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use .statx syscall tag for consistency.
The error uses .syscall = .lstat (line 721), but since lstatx() delegates to statxImpl() which invokes the statx syscall (with AT.SYMLINK_NOFOLLOW flag), the tag should be .statx for accurate error reporting.
🔎 Suggested fix
pub fn lstatxA(file_path: []const u8, comptime fields: []const StatxField) Maybe(PosixStat) {
const pathZ = std.posix.toPosixPath(file_path) catch return Maybe(PosixStat){
.err = .{
.errno = @intFromEnum(E.NAMETOOLONG),
- .syscall = .lstat,
+ .syscall = .statx,
},
};
return lstatx(&pathZ, fields);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /// Like lstatx() but accepts a non-null-terminated slice and handles ENAMETOOLONG. | |
| pub fn lstatxA(file_path: []const u8, comptime fields: []const StatxField) Maybe(PosixStat) { | |
| const pathZ = std.posix.toPosixPath(file_path) catch return Maybe(PosixStat){ | |
| .err = .{ | |
| .errno = @intFromEnum(E.NAMETOOLONG), | |
| .syscall = .lstat, | |
| }, | |
| }; | |
| return lstatx(&pathZ, fields); | |
| } | |
| /// Like lstatx() but accepts a non-null-terminated slice and handles ENAMETOOLONG. | |
| pub fn lstatxA(file_path: []const u8, comptime fields: []const StatxField) Maybe(PosixStat) { | |
| const pathZ = std.posix.toPosixPath(file_path) catch return Maybe(PosixStat){ | |
| .err = .{ | |
| .errno = @intFromEnum(E.NAMETOOLONG), | |
| .syscall = .statx, | |
| }, | |
| }; | |
| return lstatx(&pathZ, fields); | |
| } |
🤖 Prompt for AI Agents
In src/sys.zig around lines 716 to 725, the error constructed when toPosixPath
fails sets .syscall = .lstat which is inaccurate because lstatx() ultimately
uses the statx syscall; change the syscall tag to .statx in the returned
Maybe(PosixStat) error struct so error reports correctly reflect the statx
syscall used by lstatx().
| it("statSync throws ENAMETOOLONG for path exceeding PATH_MAX", () => { | ||
| // Create a path that exceeds PATH_MAX (4096 on most systems) | ||
| const longPath = "/" + Buffer.alloc(4097, "a").toString(); | ||
| expect(() => statSync(longPath)).toThrow("name too long"); | ||
| }); | ||
|
|
||
| it("lstatSync throws ENAMETOOLONG for path exceeding PATH_MAX", () => { | ||
| // Create a path that exceeds PATH_MAX (4096 on most systems) | ||
| const longPath = "/" + Buffer.alloc(4097, "a").toString(); | ||
| expect(() => lstatSync(longPath)).toThrow("name too long"); | ||
| }); | ||
|
|
||
| it("promises.stat throws ENAMETOOLONG for path exceeding PATH_MAX", async () => { | ||
| // Create a path that exceeds PATH_MAX (4096 on most systems) | ||
| const longPath = "/" + Buffer.alloc(4097, "a").toString(); | ||
| await expect(promises.stat(longPath)).rejects.toThrow("name too long"); | ||
| }); | ||
|
|
||
| it("promises.lstat throws ENAMETOOLONG for path exceeding PATH_MAX", async () => { | ||
| // Create a path that exceeds PATH_MAX (4096 on most systems) | ||
| const longPath = "/" + Buffer.alloc(4097, "a").toString(); | ||
| await expect(promises.lstat(longPath)).rejects.toThrow("name too long"); | ||
| }); | ||
|
|
||
| it("readFileSync throws ENAMETOOLONG for path exceeding PATH_MAX", () => { | ||
| // Create a path that exceeds PATH_MAX (4096 on most systems) | ||
| const longPath = "/" + Buffer.alloc(4097, "a").toString(); | ||
| expect(() => readFileSync(longPath)).toThrow("name too long"); | ||
| }); | ||
|
|
||
| // TODO: This test requires fixing the callback-based fs.readFile to properly | ||
| // pass ENAMETOOLONG to the callback instead of throwing synchronously. | ||
| // See https://github.com/oven-sh/bun/issues/25659 | ||
| it.skip("readFile passes ENAMETOOLONG to callback for path exceeding PATH_MAX", (done) => { | ||
| // Create a path that exceeds PATH_MAX (4096 on most systems) | ||
| const longPath = "/" + Buffer.alloc(4097, "a").toString(); | ||
| readFile(longPath, (err) => { | ||
| expect(err).not.toBeNull(); | ||
| expect(err!.message).toContain("name too long"); | ||
| done(); | ||
| }); | ||
| }); | ||
|
|
||
| it("promises.readFile throws ENAMETOOLONG for path exceeding PATH_MAX", async () => { | ||
| // Create a path that exceeds PATH_MAX (4096 on most systems) | ||
| const longPath = "/" + Buffer.alloc(4097, "a").toString(); | ||
| await expect(promises.readFile(longPath)).rejects.toThrow("name too long"); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Consider extracting the long path construction to reduce duplication.
The tests provide excellent coverage for ENAMETOOLONG handling across multiple fs APIs. However, the same path construction pattern "/" + Buffer.alloc(4097, "a").toString() is repeated 7 times (lines 1017, 1023, 1029, 1035, 1041, 1050, 1060), along with the same comment.
🔎 Proposed refactor to reduce duplication
+// Path that exceeds PATH_MAX (4096 on most systems)
+const longPath = "/" + Buffer.alloc(4097, "a").toString();
+
it("statSync throws ENAMETOOLONG for path exceeding PATH_MAX", () => {
- // Create a path that exceeds PATH_MAX (4096 on most systems)
- const longPath = "/" + Buffer.alloc(4097, "a").toString();
expect(() => statSync(longPath)).toThrow("name too long");
});
it("lstatSync throws ENAMETOOLONG for path exceeding PATH_MAX", () => {
- // Create a path that exceeds PATH_MAX (4096 on most systems)
- const longPath = "/" + Buffer.alloc(4097, "a").toString();
expect(() => lstatSync(longPath)).toThrow("name too long");
});
it("promises.stat throws ENAMETOOLONG for path exceeding PATH_MAX", async () => {
- // Create a path that exceeds PATH_MAX (4096 on most systems)
- const longPath = "/" + Buffer.alloc(4097, "a").toString();
await expect(promises.stat(longPath)).rejects.toThrow("name too long");
});
it("promises.lstat throws ENAMETOOLONG for path exceeding PATH_MAX", async () => {
- // Create a path that exceeds PATH_MAX (4096 on most systems)
- const longPath = "/" + Buffer.alloc(4097, "a").toString();
await expect(promises.lstat(longPath)).rejects.toThrow("name too long");
});
it("readFileSync throws ENAMETOOLONG for path exceeding PATH_MAX", () => {
- // Create a path that exceeds PATH_MAX (4096 on most systems)
- const longPath = "/" + Buffer.alloc(4097, "a").toString();
expect(() => readFileSync(longPath)).toThrow("name too long");
});
it.skip("readFile passes ENAMETOOLONG to callback for path exceeding PATH_MAX", (done) => {
- // Create a path that exceeds PATH_MAX (4096 on most systems)
- const longPath = "/" + Buffer.alloc(4097, "a").toString();
readFile(longPath, (err) => {
expect(err).not.toBeNull();
expect(err!.message).toContain("name too long");
done();
});
});
it("promises.readFile throws ENAMETOOLONG for path exceeding PATH_MAX", async () => {
- // Create a path that exceeds PATH_MAX (4096 on most systems)
- const longPath = "/" + Buffer.alloc(4097, "a").toString();
await expect(promises.readFile(longPath)).rejects.toThrow("name too long");
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it("statSync throws ENAMETOOLONG for path exceeding PATH_MAX", () => { | |
| // Create a path that exceeds PATH_MAX (4096 on most systems) | |
| const longPath = "/" + Buffer.alloc(4097, "a").toString(); | |
| expect(() => statSync(longPath)).toThrow("name too long"); | |
| }); | |
| it("lstatSync throws ENAMETOOLONG for path exceeding PATH_MAX", () => { | |
| // Create a path that exceeds PATH_MAX (4096 on most systems) | |
| const longPath = "/" + Buffer.alloc(4097, "a").toString(); | |
| expect(() => lstatSync(longPath)).toThrow("name too long"); | |
| }); | |
| it("promises.stat throws ENAMETOOLONG for path exceeding PATH_MAX", async () => { | |
| // Create a path that exceeds PATH_MAX (4096 on most systems) | |
| const longPath = "/" + Buffer.alloc(4097, "a").toString(); | |
| await expect(promises.stat(longPath)).rejects.toThrow("name too long"); | |
| }); | |
| it("promises.lstat throws ENAMETOOLONG for path exceeding PATH_MAX", async () => { | |
| // Create a path that exceeds PATH_MAX (4096 on most systems) | |
| const longPath = "/" + Buffer.alloc(4097, "a").toString(); | |
| await expect(promises.lstat(longPath)).rejects.toThrow("name too long"); | |
| }); | |
| it("readFileSync throws ENAMETOOLONG for path exceeding PATH_MAX", () => { | |
| // Create a path that exceeds PATH_MAX (4096 on most systems) | |
| const longPath = "/" + Buffer.alloc(4097, "a").toString(); | |
| expect(() => readFileSync(longPath)).toThrow("name too long"); | |
| }); | |
| // TODO: This test requires fixing the callback-based fs.readFile to properly | |
| // pass ENAMETOOLONG to the callback instead of throwing synchronously. | |
| // See https://github.com/oven-sh/bun/issues/25659 | |
| it.skip("readFile passes ENAMETOOLONG to callback for path exceeding PATH_MAX", (done) => { | |
| // Create a path that exceeds PATH_MAX (4096 on most systems) | |
| const longPath = "/" + Buffer.alloc(4097, "a").toString(); | |
| readFile(longPath, (err) => { | |
| expect(err).not.toBeNull(); | |
| expect(err!.message).toContain("name too long"); | |
| done(); | |
| }); | |
| }); | |
| it("promises.readFile throws ENAMETOOLONG for path exceeding PATH_MAX", async () => { | |
| // Create a path that exceeds PATH_MAX (4096 on most systems) | |
| const longPath = "/" + Buffer.alloc(4097, "a").toString(); | |
| await expect(promises.readFile(longPath)).rejects.toThrow("name too long"); | |
| }); | |
| // Path that exceeds PATH_MAX (4096 on most systems) | |
| const longPath = "/" + Buffer.alloc(4097, "a").toString(); | |
| it("statSync throws ENAMETOOLONG for path exceeding PATH_MAX", () => { | |
| expect(() => statSync(longPath)).toThrow("name too long"); | |
| }); | |
| it("lstatSync throws ENAMETOOLONG for path exceeding PATH_MAX", () => { | |
| expect(() => lstatSync(longPath)).toThrow("name too long"); | |
| }); | |
| it("promises.stat throws ENAMETOOLONG for path exceeding PATH_MAX", async () => { | |
| await expect(promises.stat(longPath)).rejects.toThrow("name too long"); | |
| }); | |
| it("promises.lstat throws ENAMETOOLONG for path exceeding PATH_MAX", async () => { | |
| await expect(promises.lstat(longPath)).rejects.toThrow("name too long"); | |
| }); | |
| it("readFileSync throws ENAMETOOLONG for path exceeding PATH_MAX", () => { | |
| expect(() => readFileSync(longPath)).toThrow("name too long"); | |
| }); | |
| // TODO: This test requires fixing the callback-based fs.readFile to properly | |
| // pass ENAMETOOLONG to the callback instead of throwing synchronously. | |
| // See https://github.com/oven-sh/bun/issues/25659 | |
| it.skip("readFile passes ENAMETOOLONG to callback for path exceeding PATH_MAX", (done) => { | |
| readFile(longPath, (err) => { | |
| expect(err).not.toBeNull(); | |
| expect(err!.message).toContain("name too long"); | |
| done(); | |
| }); | |
| }); | |
| it("promises.readFile throws ENAMETOOLONG for path exceeding PATH_MAX", async () => { | |
| await expect(promises.readFile(longPath)).rejects.toThrow("name too long"); | |
| }); |
🤖 Prompt for AI Agents
In test/js/node/fs/fs.test.ts around lines 1015 to 1062 the longPath
construction ("/" + Buffer.alloc(4097, "a").toString()) and its comment are
duplicated across multiple tests; extract that into a single shared constant
(e.g., const longPath = "/" + Buffer.alloc(4097, "a").toString()) and reuse it
in each test, move the explanatory comment once next to the constant, and ensure
skipped test still references the same constant so all tests use the identical
path construction.
Summary
Fix
fs.stat,fs.lstat,fs.readFile(sync/promises) to properly returnENAMETOOLONGerror when path exceedsPATH_MAX(4096 bytes), instead of crashing or undefined behavior.statA,lstatA,statxA,lstatxAfunctions tosys.zigfollowing the existingopenatApatternstat(),lstat(),readFileWithOptions()innode_fs.zigto use these new functionssliceZ()calls to prevent buffer overflowFixes #25659 (partial - callback-based
fs.readFilestill needs work)Test plan
bun bd test test/js/node/fs/fs.test.ts -t "ENAMETOOLONG"passes (8 pass, 2 skip)statSync,lstatSync,readFileSyncthrow ENAMETOOLONGpromises.stat,promises.lstat,promises.readFilereject with ENAMETOOLONG🤖 Generated with Claude Code