Skip to content

Conversation

@majiayu000
Copy link

Summary

Fix fs.stat, fs.lstat, fs.readFile (sync/promises) to properly return ENAMETOOLONG error when path exceeds PATH_MAX (4096 bytes), instead of crashing or undefined behavior.

  • Add statA, lstatA, statxA, lstatxA functions to sys.zig following the existing openatA pattern
  • Modify stat(), lstat(), readFileWithOptions() in node_fs.zig to use these new functions
  • Add path length check before sliceZ() calls to prevent buffer overflow

Fixes #25659 (partial - callback-based fs.readFile still needs work)

Test plan

  • bun bd test test/js/node/fs/fs.test.ts -t "ENAMETOOLONG" passes (8 pass, 2 skip)
  • Verify statSync, lstatSync, readFileSync throw ENAMETOOLONG
  • Verify promises.stat, promises.lstat, promises.readFile reject with ENAMETOOLONG

🤖 Generated with Claude Code

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>
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 26, 2025

Walkthrough

Introduces path length validation and ENAMETOOLONG error handling for filesystem operations. Adds new wrapper functions in src/sys.zig that accept byte slices and convert them to POSIX paths with proper error signaling. Updates src/bun.js/node/node_fs.zig to use precomputed path slices with length checks against maximum path boundaries before invoking syscalls. Includes tests validating ENAMETOOLONG behavior across multiple fs APIs.

Changes

Cohort / File(s) Summary
Syscall wrappers
src/sys.zig
Adds four new public functions (statA, lstatA, statxA, lstatxA) that accept byte slices as input, convert them to POSIX paths, and delegate to existing implementations. Maps NAMETOOLONG errors appropriately for path length overflow scenarios.
Node.js filesystem implementation
src/bun.js/node/node_fs.zig
Introduces precomputed path_slice variables in lstat, stat, and readFileWithOptions functions with guards checking path length against bun.MAX_PATH_BYTES. Updates syscall invocations (Syscall.lstatxA, Syscall.lstatA, Syscall.statxA, Syscall.openA) and error paths to use the sliced path to prevent buffer overflow.
Filesystem tests
test/js/node/fs/fs.test.ts
Adds test cases validating ENAMETOOLONG ("name too long") error handling for statSync, lstatSync, promises.stat, promises.lstat, readFileSync, and promises.readFile when paths exceed PATH_MAX. Includes a TODO placeholder for callback-based readFile testing.

Possibly related PRs

Suggested reviewers

  • dylan-conway
  • pfgithub

Pre-merge checks

✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix(fs): handle ENAMETOOLONG for paths exceeding PATH_MAX' clearly and concisely describes the main change: fixing fs operations to handle the ENAMETOOLONG error for paths exceeding PATH_MAX, which matches the core objective of the PR.
Description check ✅ Passed The PR description covers both required template sections: 'Summary' explains what the PR does with specific implementation details and issue reference, and 'Test plan' documents verification steps with concrete test results.
Linked Issues check ✅ Passed The PR successfully implements the coding requirements from #25659: adds *A variants (statA, lstatA, statxA, lstatxA) in sys.zig for non-terminated slices, modifies fs operations to use them with path length checks, and tests validate ENAMETOOLONG handling for stat/lstat/readFile (sync and promises). Callback-based readFile is correctly documented as remaining work.
Out of Scope Changes check ✅ Passed All changes are scoped to the objective of fixing ENAMETOOLONG handling: sys.zig additions provide the *A functions, node_fs.zig uses them with proper path length validation, and tests validate the fix. No unrelated changes detected.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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.path is a PathOrFileDescriptor tagged union with variants .fd and .path. The current code accesses the .path field 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_slice computation inside the .path arm 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 5715b54 and b6b6fd8.

📒 Files selected for processing (3)
  • src/bun.js/node/node_fs.zig
  • src/sys.zig
  • test/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}: Use bun:test with 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 use port: 0 to get a random port
Prefer concurrent tests over sequential tests using test.concurrent or describe.concurrent when multiple tests spawn processes or write files, unless it's very difficult to make them concurrent
When spawning Bun processes in tests, use bunExe and bunEnv from harness to ensure the same build of Bun is used and debug logging is silenced
Use -e flag for single-file tests when spawning Bun processes
Use tempDir() 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, use Promise.withResolvers to create a promise that can be resolved or rejected from a callback
Do not set a timeout on tests. Bun already has timeouts
Use Buffer.alloc(count, fill).toString() instead of 'A'.repeat(count) to create repetitive strings in tests, as ''.repeat is very slow in debug JavaScriptCore builds
Use describe blocks for grouping related tests
Always use await using or using to 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
Use describe.each() for parameterized tests
Use toMatchSnapshot() for snapshot testing
Use beforeAll(), afterEach(), beforeEach() for setup/teardown in tests
Track resources (servers, clients) in arrays for cleanup in afterEach()

Files:

  • test/js/node/fs/fs.test.ts
**/*.test.ts?(x)

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.test.ts?(x): Never use bun test directly - always use bun bd test to run tests with debug build changes
For single-file tests, prefer -e flag over tempDir
For multi-file tests, prefer tempDir and Bun.spawn over single-file tests
Use normalizeBunSnapshot to normalize snapshot output of tests
Never write tests that check for 'panic', 'uncaught exception', or similar strings in test output
Use tempDir from harness to create temporary directories - do not use tmpdirSync or fs.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 use setTimeout in tests; instead await the condition to be met
Verify tests fail with USE_SYSTEM_BUN=1 bun test <file> and pass with bun bd test <file> - tests are invalid if they pass with USE_SYSTEM_BUN=1
Test files must end with .test.ts or .test.tsx
Avoid shell commands like find or grep in 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: 0 in 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 @import at the bottom of the file (auto formatter will move them automatically)

Files:

  • src/sys.zig
  • src/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.zig
  • src/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.ts
  • src/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 to ENAMETOOLONG, then delegates to the existing stat() function. The syscall tag is appropriate.


551-560: LGTM! Correct mirror of statA for lstat semantics.

The function follows the same pattern as statA and correctly delegates to lstat() with the appropriate syscall tag.

src/bun.js/node/node_fs.zig (2)

3895-3919: *lstat: slice-based A syscall usage looks correct

Using path_slice = args.path.slice() with lstatxA/lstatA and propagating errors via err.withPath(path_slice) preserves existing NOENT semantics while avoiding buffer-based sliceZ for overlong paths. This is aligned with the ENAMETOOLONG handling goals.


5733-5769: stat: ENAMETOOLONG-safe path handling looks consistent

stat now uses path_slice = args.path.slice(), gates StandaloneModuleGraph lookup on path_slice.len < bun.MAX_PATH_BYTES, and calls statxA/statA with path_slice, propagating errors via err.withPath(path_slice) while preserving NOENT/throwIfNoEntry semantics. This matches the intended ENAMETOOLONG behavior and is consistent with the lstat changes.

Comment on lines +705 to +714
/// 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);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

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.

Suggested change
/// 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.

Comment on lines +716 to +725
/// 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);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

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.

Suggested change
/// 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().

Comment on lines +1015 to +1062
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");
});
Copy link
Contributor

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.

Suggested change
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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bun throws error on the main thread while fs operation when size of the file name is long after specific range

1 participant