diff --git a/src/runtime/shell/builtin/mkdir.rs b/src/runtime/shell/builtin/mkdir.rs index a4695d61c6b2..829711d08dbf 100644 --- a/src/runtime/shell/builtin/mkdir.rs +++ b/src/runtime/shell/builtin/mkdir.rs @@ -298,6 +298,7 @@ impl ShellMkdirTask { use bun_paths::{Platform, platform, resolve_path}; // We have to give an absolute path to our mkdir implementation for it // to work with cwd. + let mut spill = Vec::new(); let filepath: &bun_core::ZStr = if Platform::AUTO.is_absolute(&this.filepath) { // Owned `Vec`; ensure NUL-terminated. if this.filepath.last() != Some(&0) { @@ -305,9 +306,22 @@ impl ShellMkdirTask { } bun_core::ZStr::from_buf(&this.filepath, this.filepath.len() - 1) } else { - resolve_path::join_z::(&[&this.cwd_path, &this.filepath]) + resolve_path::join_z_spill::( + &mut spill, + &[&this.cwd_path, &this.filepath], + ) }; + // `NodeFS` expects the `Valid::path_string_length` bound its JS callers + // enforce; past it, `PathLike::slice_z` yields "" and mkdir reports ENOENT. + if filepath.len() >= bun_paths::MAX_PATH_BYTES { + this.err = Some( + bun_sys::Error::from_code(bun_sys::E::ENAMETOOLONG, bun_sys::Tag::mkdir) + .with_path(filepath.as_bytes()), + ); + return; + } + let mut node_fs = NodeFS::default(); let args = fs_args::Mkdir { path: PathLike::String(bun_ptr::cow_slice::CowSlice::init_unchecked( diff --git a/src/runtime/shell/builtin/touch.rs b/src/runtime/shell/builtin/touch.rs index 95ae1398deee..c4ef56c0ff31 100644 --- a/src/runtime/shell/builtin/touch.rs +++ b/src/runtime/shell/builtin/touch.rs @@ -265,15 +265,17 @@ impl ShellTouchTask { pub(crate) fn run_from_thread_pool(this: &mut ShellTouchTask) { use bun_paths::resolve_path::{self, Platform, platform}; use bun_sys::FdExt as _; - // We have to give an absolute path. - let mut buf = bun_paths::PathBuffer::uninit(); + // We have to give an absolute path. An operand that does not fit the + // path buffer is still passed on whole, so the OS reports ENAMETOOLONG + // for it like for any other operand. + let mut spill = Vec::new(); let filepath: &bun_core::ZStr = if Platform::AUTO.is_absolute(&this.filepath) { - // Re-terminate into the path buffer (`filepath` is the bare argv - // bytes without the trailing NUL). - resolve_path::join_z_buf::(buf.as_mut_slice(), &[&this.filepath]) + // Re-terminate (`filepath` is the bare argv bytes without the + // trailing NUL). + resolve_path::join_z_spill::(&mut spill, &[&this.filepath]) } else { - resolve_path::join_z_buf::( - buf.as_mut_slice(), + resolve_path::join_z_spill::( + &mut spill, &[&this.cwd_path, &this.filepath], ) }; diff --git a/test/js/bun/shell/commands/mkdir.test.ts b/test/js/bun/shell/commands/mkdir.test.ts new file mode 100644 index 000000000000..8fd48977b24e --- /dev/null +++ b/test/js/bun/shell/commands/mkdir.test.ts @@ -0,0 +1,77 @@ +import { expect, test } from "bun:test"; +import { bunEnv, bunExe, isWindows, tempDir } from "harness"; +import { join } from "node:path"; + +// A relative operand was joined onto the cwd in a fixed 4096-byte buffer, so an +// operand longer than that crashed the process, and an operand longer than a +// PathBuffer (however it was spelled) was handed to the fs layer as "" and +// reported as ENOENT. Runs in a child process so a crash shows up as a failed +// assertion rather than taking the test runner down with it. +test("operands longer than the path buffers are reported, not a crash", async () => { + using dir = tempDir("mkdir-long-operand", {}); + const fixture = /* ts */ ` + import { $ } from "bun"; + import { existsSync } from "node:fs"; + $.nothrow(); + const dir = process.argv[1]; + const long = Buffer.alloc(5000, "a").toString(); + // Past the path buffer on every platform, Windows included. + const huge = Buffer.alloc(100_000, "h").toString(); + // Longer than the buffers as written, but normalizes down to one component. + const dotSlashes = Buffer.alloc(6000, "./").toString(); + const run = async (...args: string[]) => { + const { exitCode, stderr } = await $\`mkdir \${args}\`.quiet(); + return { exitCode, stderr: stderr.toString() }; + }; + console.log(JSON.stringify({ + cwd: process.cwd(), + relative: await run(long), + absolute: await run(dir + "/" + long), + parents: await run("-p", long), + huge: await run(huge), + mixed: { ...(await run(long, "short")), shortCreated: existsSync(dir + "/short") }, + dotSlashes: { ...(await run(dotSlashes + "normalized")), created: existsSync(dir + "/normalized") }, + absoluteDotSlashes: { + ...(await run(dir + "/" + dotSlashes + "as-written")), + created: existsSync(dir + "/as-written"), + }, + })); + `; + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", fixture, String(dir)], + env: bunEnv, + cwd: String(dir), + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + const { cwd, ...results } = JSON.parse(stdout); + + const long = Buffer.alloc(5000, "a").toString(); + const huge = Buffer.alloc(100_000, "h").toString(); + const dotSlashes = Buffer.alloc(6000, "./").toString(); + // mkdir reports the path it operated on: a relative operand joined onto the + // cwd, an absolute one as written. + const tooLong = (path: string) => `mkdir: ${path}: File name too long\n`; + // 5000 bytes fits Windows' much larger path buffer, so there the OS picks + // the error; what matters is that each operand fails on its own. + const failed = (path: string) => + isWindows ? { exitCode: 1, stderr: expect.stringMatching(/^mkdir: /) } : { exitCode: 1, stderr: tooLong(path) }; + expect(results).toEqual({ + relative: failed(join(cwd, long)), + absolute: failed(`${dir}/${long}`), + parents: failed(join(cwd, long)), + huge: { exitCode: 1, stderr: tooLong(join(cwd, huge)) }, + mixed: { ...failed(join(cwd, long)), shortCreated: true }, + dotSlashes: { exitCode: 0, stderr: "", created: true }, + // An absolute operand is not normalized (`..` through a symlink means + // something else to the kernel), so like the kernel and coreutils, mkdir + // bounds it as written. On Windows it fits the much larger buffer and the + // fs layer normalizes it while converting it to a wide path, so it works. + absoluteDotSlashes: isWindows + ? { exitCode: 0, stderr: "", created: true } + : { ...failed(`${dir}/${dotSlashes}as-written`), created: false }, + }); + expect(exitCode).toBe(0); +}); diff --git a/test/js/bun/shell/commands/touch.test.ts b/test/js/bun/shell/commands/touch.test.ts new file mode 100644 index 000000000000..8c5e69391006 --- /dev/null +++ b/test/js/bun/shell/commands/touch.test.ts @@ -0,0 +1,63 @@ +import { expect, test } from "bun:test"; +import { bunEnv, bunExe, isWindows, tempDir } from "harness"; +import { join } from "node:path"; + +// Every operand, absolute or not, was joined into a fixed-size path buffer, so +// an operand longer than that crashed the process. Runs in a child process so a +// crash shows up as a failed assertion rather than taking the test runner down +// with it. +test("operands longer than the path buffer are reported, not a crash", async () => { + using dir = tempDir("touch-long-operand", {}); + const fixture = /* ts */ ` + import { $ } from "bun"; + import { existsSync } from "node:fs"; + $.nothrow(); + const dir = process.argv[1]; + const long = Buffer.alloc(5000, "a").toString(); + // Past the path buffer on every platform, Windows included. + const huge = Buffer.alloc(100_000, "h").toString(); + // Longer than the buffer as written, but normalizes down to one component. + const dotSlashes = Buffer.alloc(6000, "./").toString() + "normalized"; + const run = async (...args: string[]) => { + const { exitCode, stderr } = await $\`touch \${args}\`.quiet(); + return { exitCode, stderr: stderr.toString() }; + }; + console.log(JSON.stringify({ + cwd: process.cwd(), + relative: await run(long), + absolute: await run(dir + "/" + long), + huge: await run(huge), + mixed: { ...(await run(long, "short")), shortCreated: existsSync(dir + "/short") }, + dotSlashes: { ...(await run(dotSlashes)), created: existsSync(dir + "/normalized") }, + })); + `; + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", fixture, String(dir)], + env: bunEnv, + cwd: String(dir), + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + const { cwd, ...results } = JSON.parse(stdout); + + const long = Buffer.alloc(5000, "a").toString(); + const huge = Buffer.alloc(100_000, "h").toString(); + // The over-long path is passed to the OS whole, and touch reports the path + // it operated on: a relative operand joined onto the cwd. Which errno + // Windows picks for it is up to the OS; what matters is that each operand + // fails on its own. + const failed = (path: string) => + isWindows + ? { exitCode: 1, stderr: expect.stringMatching(/^touch: /) } + : { exitCode: 1, stderr: `touch: ${path}: File name too long\n` }; + expect(results).toEqual({ + relative: failed(join(cwd, long)), + absolute: failed(`${dir}/${long}`), + huge: failed(join(cwd, huge)), + mixed: { ...failed(join(cwd, long)), shortCreated: true }, + dotSlashes: { exitCode: 0, stderr: "", created: true }, + }); + expect(exitCode).toBe(0); +});