From 87a9b4516755399dacdf9b01ff2caf1f3dc02096 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 11 Aug 2026 04:46:03 +0000 Subject: [PATCH 1/4] install(isolated): fail the package with ENAMETOOLONG when a walked entry does not fit the path buffer The isolated linker's Hardlinker and FileCopier append each walker entry to CheckLength::ASSUME paths, so an entry longer than the remaining buffer panics in Buf::append instead of failing the package. Build them on length-checked paths (Path::into_checked) and report the overflow as ENAMETOOLONG, guard the cwd + dest join on Windows the same way, and propagate the errors from creating a directory entry and from fstat on a source file that were previously discarded. --- src/install/PackageManager/patchPackage.rs | 4 +- src/install/isolated_install/FileCopier.rs | 138 ++++++++++-------- src/install/isolated_install/Hardlinker.rs | 61 +++++--- src/install/isolated_install/Installer.rs | 16 +- src/paths/Path.rs | 24 ++- src/paths/lib.rs | 3 +- .../isolated-install-long-paths.test.ts | 93 ++++++++++++ 7 files changed, 242 insertions(+), 97 deletions(-) create mode 100644 test/cli/install/isolated-install-long-paths.test.ts diff --git a/src/install/PackageManager/patchPackage.rs b/src/install/PackageManager/patchPackage.rs index a416be8d9293..c3212eb711df 100644 --- a/src/install/PackageManager/patchPackage.rs +++ b/src/install/PackageManager/patchPackage.rs @@ -1231,8 +1231,8 @@ fn overwrite_package_in_node_modules_folder( let mut copier: FileCopier = FileCopier::init( cached_package_folder.fd, - src_path, - dest_subpath, + src_path.into_checked(), + dest_subpath.into_checked(), ignore_directories, )?; diff --git a/src/install/isolated_install/FileCopier.rs b/src/install/isolated_install/FileCopier.rs index 2cbed827c689..119a63cd8d6a 100644 --- a/src/install/isolated_install/FileCopier.rs +++ b/src/install/isolated_install/FileCopier.rs @@ -4,6 +4,7 @@ use core::ptr; use bun_alloc::AllocError; #[cfg(not(windows))] use bun_core::{Global, fmt as bun_fmt}; +use bun_paths::path_options::{CheckLength, Kind, PathSeparators}; use bun_paths::{self, OSPathChar, OSPathSlice}; use bun_sys::{self as sys, Dir, E, EntryKind, Fd, walker_skippable, walker_skippable::Walker}; @@ -11,13 +12,12 @@ use bun_sys::{self as sys, Dir, E, EntryKind, Fd, walker_skippable, walker_skipp // u16 on Windows — encoded via `OSPathChar` so `slice()`/`slice_z()` produce // the platform-native width. The auto separator mode normalizes `/` → `\` on Windows // during `from`/`append`, which is load-bearing for the Win32 calls below. +// Length-checked for the same reason as the `Hardlinker` paths: the walker's +// entry paths appended on Windows are not bounded by any path buffer. type AbsPathAutoOs = - bun_paths::AbsPath; -type PathAutoOs = bun_paths::Path< - OSPathChar, - { bun_paths::path_options::Kind::ANY }, - { bun_paths::path_options::PathSeparators::AUTO }, ->; + bun_paths::AbsPath; +type PathAutoOs = + bun_paths::Path; pub(crate) struct FileCopier { pub(crate) src_path: AbsPathAutoOs, @@ -110,74 +110,85 @@ impl FileCopier { // A `path.save()` ResetScope would hold `&mut Path` and keep // `self.src_path` / `self.dest_subpath` exclusively borrowed - // for the rest of the iteration. Capture the saved length and + // for the rest of the iteration. Capture the saved lengths and // restore via `set_length` after the body instead. let src_saved_len = self.src_path.len(); - let _ = self.src_path.append(entry.path.as_slice()); - let dest_saved_len = self.dest_subpath.len(); - let _ = self.dest_subpath.append(entry.path.as_slice()); - - let result: sys::Result<()> = match entry.kind { - EntryKind::Directory => { - // SAFETY: FFI — both `slice_z()` are NUL-terminated WStrs. - if unsafe { - bun_sys::windows::CreateDirectoryExW( - self.src_path.slice_z().as_ptr(), - self.dest_subpath.slice_z().as_ptr(), - ptr::null_mut(), - ) - } == 0 - { - let _ = bun_sys::make_path::make_path::( - &dest_dir, - entry.path.as_slice(), - ); - } - sys::Result::Ok(()) + + let result: sys::Result<()> = 'entry: { + if self.src_path.append(entry.path.as_slice()).is_err() + || self.dest_subpath.append(entry.path.as_slice()).is_err() + { + break 'entry sys::Result::Err(sys::Error::from_code( + E::ENAMETOOLONG, + sys::Tag::copyfile, + )); } - EntryKind::File => { - match bun_sys::copy_file::copy_file( - self.src_path.slice_z(), - self.dest_subpath.slice_z(), - ) { - sys::Result::Ok(()) => sys::Result::Ok(()), - sys::Result::Err(first_err) => { - // Retry after creating the parent directory. - // For root-level files (`index.js`, - // `package.json`, `LICENSE`) `dirname` is - // null and there is no missing parent to - // create — `dest_dir` itself was already - // opened above — so the original error is the - // real failure and must propagate. Silently - // continuing here would let a staged - // global-store entry be renamed into place - // with files missing. - match bun_paths::Dirname::dirname::(entry.path.as_slice()) { - None => sys::Result::Err(first_err), - Some(entry_dirname) => { - let _ = bun_sys::make_path::make_path::( - &dest_dir, - entry_dirname, - ); - bun_sys::copy_file::copy_file( - self.src_path.slice_z(), - self.dest_subpath.slice_z(), - ) + + match entry.kind { + EntryKind::Directory => { + // SAFETY: FFI — both `slice_z()` are NUL-terminated WStrs. + if unsafe { + bun_sys::windows::CreateDirectoryExW( + self.src_path.slice_z().as_ptr(), + self.dest_subpath.slice_z().as_ptr(), + ptr::null_mut(), + ) + } == 0 + { + // CreateDirectoryExW also fails when the directory already + // exists; `make_path` treats that as success and reports + // anything else. + bun_sys::make_path::make_path::( + &dest_dir, + entry.path.as_slice(), + ) + } else { + sys::Result::Ok(()) + } + } + EntryKind::File => { + match bun_sys::copy_file::copy_file( + self.src_path.slice_z(), + self.dest_subpath.slice_z(), + ) { + sys::Result::Ok(()) => sys::Result::Ok(()), + sys::Result::Err(first_err) => { + // Retry after creating the parent directory. + // For root-level files (`index.js`, + // `package.json`, `LICENSE`) `dirname` is + // null and there is no missing parent to + // create — `dest_dir` itself was already + // opened above — so the original error is the + // real failure and must propagate. Silently + // continuing here would let a staged + // global-store entry be renamed into place + // with files missing. + match bun_paths::Dirname::dirname::(entry.path.as_slice()) + { + None => sys::Result::Err(first_err), + Some(entry_dirname) => { + let _ = bun_sys::make_path::make_path::( + &dest_dir, + entry_dirname, + ); + bun_sys::copy_file::copy_file( + self.src_path.slice_z(), + self.dest_subpath.slice_z(), + ) + } } } } } + _ => unreachable!(), } - _ => unreachable!(), }; self.src_path.set_length(src_saved_len); self.dest_subpath.set_length(dest_saved_len); - if let sys::Result::Err(err) = result { - return sys::Result::Err(err); - } + result?; } #[cfg(not(windows))] { @@ -220,10 +231,9 @@ impl FileCopier { #[cfg(unix)] { - let stat = match bun_sys::fstat(src.handle()) { - sys::Result::Ok(s) => s, - sys::Result::Err(_) => continue, - }; + // `dest` has already been created (or truncated) above, so + // skipping this entry would leave an empty file behind. + let stat = bun_sys::fstat(src.handle())?; // SAFETY: fchmod is safe to call with any fd + mode; errors are ignored (`_ =`). unsafe { let _ = bun_sys::c::fchmod(dest.handle().native(), stat.st_mode); diff --git a/src/install/isolated_install/Hardlinker.rs b/src/install/isolated_install/Hardlinker.rs index 4856f2de0c90..fef52f6a643f 100644 --- a/src/install/isolated_install/Hardlinker.rs +++ b/src/install/isolated_install/Hardlinker.rs @@ -7,16 +7,17 @@ use bun_sys::{self as sys, EntryKind, Fd, FdExt}; // on Windows — encoded here via the `OSPathChar` type alias so the struct's // `slice()`/`slice_z()` produce the platform-native width without per-field // `#[cfg]` divergence. -#[cfg(windows)] -use bun_paths::path_options::AssumeOk as _; +// +// The walker's entry paths get appended to these per entry, and the walker +// opens every directory relative to its parent, so an entry path can be longer +// than any path buffer. `CheckLength::CHECK` turns that into `ENAMETOOLONG` +// for the package (what the hoisted linker reports for the same tree) instead +// of an out-of-bounds write in `append`. +use bun_paths::path_options::{CheckLength, Kind, PathSeparators}; use bun_paths::{AbsPath, OSPathChar, OSPathSlice, Path}; -type OsAbsPath = AbsPath; -type OsPath = Path< - OSPathChar, - { bun_paths::path_options::Kind::ANY }, - { bun_paths::path_options::PathSeparators::AUTO }, ->; +type OsAbsPath = AbsPath; +type OsPath = Path; pub(crate) struct Hardlinker { pub(crate) src: OsAbsPath, @@ -24,6 +25,10 @@ pub(crate) struct Hardlinker { pub(crate) walker: Walker, } +fn name_too_long() -> sys::Error { + sys::Error::from_code(sys::E::ENAMETOOLONG, sys::Tag::link) +} + impl Hardlinker { pub(crate) fn init( folder_dir: Fd, @@ -97,25 +102,27 @@ impl Hardlinker { // A `path.save()` ResetScope would hold `&mut Path` and keep // `self.src`/`self.dest` exclusively borrowed for the rest of - // the iteration. Capture the saved length directly and restore + // the iteration. Capture the saved lengths directly and restore // via `set_length` after the body (and before any error return) // so the truncation happens on every exit. let src_saved_len = self.src.len(); - // `OsAbsPath`/`OsPath` use `CheckLength::ASSUME`, so `append`'s - // `Err(MaxPathExceeded)` arm is statically unreachable -- see - // `path_options::AssumeOk`. - self.src.append(entry.path.as_slice()).assume_ok(); - let dest_saved_len = self.dest.len(); - self.dest.append(entry.path.as_slice()).assume_ok(); let err: Option = 'body: { + if self.src.append(entry.path.as_slice()).is_err() + || self.dest.append(entry.path.as_slice()).is_err() + { + break 'body Some(name_too_long()); + } + match entry.kind { EntryKind::Directory => { - let _ = sys::make_path::make_path::( + if let sys::Result::Err(mkdir_err) = sys::make_path::make_path::( &sys::Dir::cwd(), self.dest.slice(), - ); + ) { + break 'body Some(mkdir_err); + } } EntryKind::File => { let mut destfile_path_buf = bun_paths::w_path_buffer_pool::get(); @@ -136,6 +143,15 @@ impl Hardlinker { dest_slice, ] }; + // `join_string_buf_w_same` and `add_nt_path_prefix_if_needed` + // write into the pooled buffers unchecked: the parts, a + // separator per part, the `\??\` prefix and the NUL must fit. + let needed_len = + dest_parts.iter().map(|part| part.len() + 1).sum::() + + bun_paths::windows::NT_OBJECT_PREFIX.len(); + if needed_len > destfile_path_buf2.len() { + break 'body Some(name_too_long()); + } let joined = bun_paths::resolve_path::join_string_buf_w_same::< bun_paths::platform::Windows, >( @@ -246,12 +262,19 @@ impl Hardlinker { // `len()` and restore via `set_length()` after the body so the // truncation runs on every exit. let dest_saved_len = self.dest.len(); - let _ = self.dest.append(entry.path.as_bytes()); // OOM/capacity: fire-and-forget let err: Option = 'body: { + if self.dest.append(entry.path.as_bytes()).is_err() { + break 'body Some(name_too_long()); + } + match entry.kind { EntryKind::Directory => { - let _ = Fd::cwd().make_path(self.dest.slice()); + if let sys::Result::Err(mkdir_err) = + Fd::cwd().make_path(self.dest.slice()) + { + break 'body Some(mkdir_err); + } } EntryKind::File => { match sys::linkat( diff --git a/src/install/isolated_install/Installer.rs b/src/install/isolated_install/Installer.rs index 6b31bbfd28a5..f34a634e2bbe 100644 --- a/src/install/isolated_install/Installer.rs +++ b/src/install/isolated_install/Installer.rs @@ -917,8 +917,8 @@ impl Task { let mut hardlinker = Hardlinker::init( folder_dir, - src, - dest, + src.into_checked(), + dest.into_checked(), &[bun_paths::os_path_literal!("node_modules")], )?; @@ -1029,8 +1029,8 @@ impl Task { let mut file_copier = FileCopier::init( folder_dir, - src_path.into_sep::<{ PathSeparators::AUTO }>(), - dest.into_sep::<{ PathSeparators::AUTO }>(), + src_path.into_checked(), + dest.into_checked(), &[bun_paths::os_path_literal!("node_modules")], )?; @@ -1338,8 +1338,8 @@ impl Task { let mut hardlinker = Hardlinker::init( cached_package_dir.unwrap(), - src, - dest_subpath, + src.into_checked(), + dest_subpath.into_checked(), &[], )?; @@ -1420,8 +1420,8 @@ impl Task { let mut file_copier = FileCopier::init( cached_package_dir.unwrap(), - src_path.into_sep::<{ PathSeparators::AUTO }>(), - dest_subpath.into_sep::<{ PathSeparators::AUTO }>(), + src_path.into_checked(), + dest_subpath.into_checked(), &[], )?; diff --git a/src/paths/Path.rs b/src/paths/Path.rs index dbfaf781fe3a..2044dd477040 100644 --- a/src/paths/Path.rs +++ b/src/paths/Path.rs @@ -35,7 +35,7 @@ pub mod options { } #[derive(PartialEq, Eq, Clone, Copy, Debug)] - pub(crate) enum CheckLength { + pub enum CheckLength { AssumeAlwaysLessThanMaxPath, CheckForGreaterThanMaxPath, } @@ -80,7 +80,9 @@ pub mod options { } impl CheckLength { pub(crate) const ASSUME: u8 = 0; - pub(crate) const CHECK: u8 = 1; + /// Pass as the `CHECK` const param (or convert with + /// [`Path::into_checked`]) when unbounded input gets appended. + pub const CHECK: u8 = 1; #[inline(always)] pub(crate) const fn from_u8(v: u8) -> Self { if v == 0 { @@ -852,6 +854,24 @@ impl /// nominally distinct, hence this explicit conversion. #[inline] pub fn into_sep(self) -> Path { + self.reinterpret() + } + + /// Reinterpret this path as length-checked: from here on `append` and + /// friends return `Err(MaxPathExceeded)` for input that does not fit + /// instead of panicking. For paths built with the `ASSUME`-only helpers + /// (`PathLike`) that are about to receive unbounded input, e.g. the + /// entries of a directory walk. Like `SEP_OPT`, `CHECK` only selects how + /// later mutations behave, so this is a no-op move. + #[inline] + pub fn into_checked(self) -> Path { + self.reinterpret() + } + + #[inline] + fn reinterpret( + self, + ) -> Path { // Explicit field move (not `transmute`): `Path`/`Buf` are `repr(Rust)`, so // Rust gives no layout-compat guarantee between distinct const-generic // instantiations. Rebuilding field-by-field is layout-agnostic and diff --git a/src/paths/lib.rs b/src/paths/lib.rs index 27218cb3dd16..ef92b6eca491 100644 --- a/src/paths/lib.rs +++ b/src/paths/lib.rs @@ -436,8 +436,7 @@ pub use env_path::{EnvPath, EnvPathInput, PathComponentBuilder}; // ────────────────────────────────────────────────────────────────────────── pub mod windows { /// `\??\` — NT object-manager prefix (UTF-16). - pub(crate) const NT_OBJECT_PREFIX: [u16; 4] = - ['\\' as u16, '?' as u16, '?' as u16, '\\' as u16]; + pub const NT_OBJECT_PREFIX: [u16; 4] = ['\\' as u16, '?' as u16, '?' as u16, '\\' as u16]; /// `\??\UNC\` — NT object-manager UNC prefix (UTF-16). pub(crate) const NT_UNC_OBJECT_PREFIX: [u16; 8] = [ '\\' as u16, diff --git a/test/cli/install/isolated-install-long-paths.test.ts b/test/cli/install/isolated-install-long-paths.test.ts new file mode 100644 index 000000000000..b51b45e66ad2 --- /dev/null +++ b/test/cli/install/isolated-install-long-paths.test.ts @@ -0,0 +1,93 @@ +// The isolated linker copies a folder dependency with a directory walk that +// opens every directory relative to its parent, so the walk reaches entries +// whose path is longer than PATH_MAX, while the destination path for each +// entry is built in a PATH_MAX-sized buffer. Such a package has to fail with +// ENAMETOOLONG, as it does with the hoisted linker, instead of crashing the +// install. +// +// Skipped on Windows: the path buffers there hold 32767 UTF-16 units, which is +// also the limit of the filesystem, so a tree that overflows them cannot be +// created in the first place. +import { describe, expect, test } from "bun:test"; +import { bunEnv, bunExe, isMacOS, isWindows, tempDir } from "harness"; +import { mkdirSync, readFileSync, renameSync, writeFileSync } from "node:fs"; +import { join } from "node:path"; + +const PATH_MAX = isMacOS ? 1024 : 4096; +const SEGMENT = Buffer.alloc(100, "d").toString(); +// Every mkdir/rename issued by createDeepChain names at most CHUNK_DEPTH + 1 +// segments below the temp dir, which fits PATH_MAX on every platform. +const CHUNK_DEPTH = 4; +const CHUNK = Array(CHUNK_DEPTH).fill(SEGMENT).join("/"); +const CHUNK_BYTES = CHUNK.length + 1; + +const OVERFLOWING_CHUNKS = Math.ceil((PATH_MAX + 256) / CHUNK_BYTES); +const FITTING_CHUNKS = Math.floor(PATH_MAX / 2 / CHUNK_BYTES); + +/** + * Creates a straight chain of `chunks * CHUNK_DEPTH` directories in `pkgDir`, + * optionally with a file at the bottom, and returns the chain relative to + * `pkgDir`. The chunks are created side by side in `staging` and then renamed + * into each other from the bottom up, so the finished chain can be longer than + * PATH_MAX even though no single syscall sees more than one chunk of it. + */ +function createDeepChain(pkgDir: string, staging: string, chunks: number, leaf?: string): string { + for (let i = 0; i < chunks; i++) { + mkdirSync(join(staging, String(i), CHUNK), { recursive: true }); + } + if (leaf !== undefined) { + writeFileSync(join(staging, String(chunks - 1), CHUNK, leaf), "deep"); + } + for (let i = chunks - 1; i > 0; i--) { + renameSync(join(staging, String(i), SEGMENT), join(staging, String(i - 1), CHUNK, SEGMENT)); + } + renameSync(join(staging, "0", SEGMENT), join(pkgDir, SEGMENT)); + return Array(chunks * CHUNK_DEPTH).fill(SEGMENT).join("/"); +} + +async function installFolderDependency(chunks: number, leaf?: string) { + using dir = tempDir("isolated-long-paths", { + "package.json": JSON.stringify({ name: "proj", dependencies: { pkg: "file:./pkg" } }), + "pkg/package.json": JSON.stringify({ name: "pkg", version: "1.0.0" }), + }); + const chain = createDeepChain(join(String(dir), "pkg"), join(String(dir), "staging"), chunks, leaf); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "install", "--linker", "isolated"], + cwd: String(dir), + env: { ...bunEnv, BUN_INSTALL_CACHE_DIR: join(String(dir), "cache") }, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + const installedLeaf = + exitCode === 0 && leaf !== undefined + ? readFileSync(join(String(dir), "node_modules", "pkg", chain, leaf), "utf8") + : undefined; + return { stdout, stderr, exitCode, installedLeaf }; +} + +describe.skipIf(isWindows)("isolated linker and folder dependencies deeper than PATH_MAX", () => { + test.concurrent("a file below PATH_MAX depth fails the package with ENAMETOOLONG", async () => { + const { stderr, exitCode } = await installFolderDependency(OVERFLOWING_CHUNKS, "leaf.txt"); + expect(stderr).toContain("ENAMETOOLONG"); + expect(stderr).toContain("failed to link package: pkg@"); + expect(exitCode).toBe(1); + }); + + test.concurrent("a directory below PATH_MAX depth fails the package with ENAMETOOLONG", async () => { + const { stderr, exitCode } = await installFolderDependency(OVERFLOWING_CHUNKS); + expect(stderr).toContain("ENAMETOOLONG"); + expect(stderr).toContain("failed to link package: pkg@"); + expect(exitCode).toBe(1); + }); + + test.concurrent("a deep tree that fits PATH_MAX still installs", async () => { + const { stdout, stderr, exitCode, installedLeaf } = await installFolderDependency(FITTING_CHUNKS, "leaf.txt"); + expect(stderr).not.toContain("ENAMETOOLONG"); + expect(stdout).toMatch(/\d+ packages? installed/); + expect(exitCode).toBe(0); + expect(installedLeaf).toBe("deep"); + }); +}); From 1b4890af33a54f3130024c54b2f6241e3182d37f Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 11 Aug 2026 05:02:27 +0000 Subject: [PATCH 2/4] test: run the deep folder dependency case on Windows too --- .../isolated-install-long-paths.test.ts | 46 +++++++++---------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/test/cli/install/isolated-install-long-paths.test.ts b/test/cli/install/isolated-install-long-paths.test.ts index b51b45e66ad2..83f26c236794 100644 --- a/test/cli/install/isolated-install-long-paths.test.ts +++ b/test/cli/install/isolated-install-long-paths.test.ts @@ -1,28 +1,26 @@ // The isolated linker copies a folder dependency with a directory walk that // opens every directory relative to its parent, so the walk reaches entries -// whose path is longer than PATH_MAX, while the destination path for each -// entry is built in a PATH_MAX-sized buffer. Such a package has to fail with -// ENAMETOOLONG, as it does with the hoisted linker, instead of crashing the -// install. -// -// Skipped on Windows: the path buffers there hold 32767 UTF-16 units, which is -// also the limit of the filesystem, so a tree that overflows them cannot be -// created in the first place. +// whose path is longer than the buffer the destination path of each entry is +// built in. Such a package has to fail with ENAMETOOLONG, as it does with the +// hoisted linker, instead of crashing the install. import { describe, expect, test } from "bun:test"; import { bunEnv, bunExe, isMacOS, isWindows, tempDir } from "harness"; import { mkdirSync, readFileSync, renameSync, writeFileSync } from "node:fs"; import { join } from "node:path"; -const PATH_MAX = isMacOS ? 1024 : 4096; const SEGMENT = Buffer.alloc(100, "d").toString(); // Every mkdir/rename issued by createDeepChain names at most CHUNK_DEPTH + 1 -// segments below the temp dir, which fits PATH_MAX on every platform. +// segments below the temp dir, which fits the path limit of every platform. const CHUNK_DEPTH = 4; const CHUNK = Array(CHUNK_DEPTH).fill(SEGMENT).join("/"); const CHUNK_BYTES = CHUNK.length + 1; +// Size of the destination path buffers on POSIX. On Windows they hold 32767 +// UTF-16 units, which is also the filesystem's limit, so a tree that overflows +// them cannot be created there; a tree this deep has to install fine instead. +const PATH_MAX = isMacOS ? 1024 : 4096; const OVERFLOWING_CHUNKS = Math.ceil((PATH_MAX + 256) / CHUNK_BYTES); -const FITTING_CHUNKS = Math.floor(PATH_MAX / 2 / CHUNK_BYTES); +const FITTING_CHUNKS = isWindows ? Math.ceil(4096 / CHUNK_BYTES) : Math.floor(PATH_MAX / 2 / CHUNK_BYTES); /** * Creates a straight chain of `chunks * CHUNK_DEPTH` directories in `pkgDir`, @@ -42,7 +40,9 @@ function createDeepChain(pkgDir: string, staging: string, chunks: number, leaf?: renameSync(join(staging, String(i), SEGMENT), join(staging, String(i - 1), CHUNK, SEGMENT)); } renameSync(join(staging, "0", SEGMENT), join(pkgDir, SEGMENT)); - return Array(chunks * CHUNK_DEPTH).fill(SEGMENT).join("/"); + return Array(chunks * CHUNK_DEPTH) + .fill(SEGMENT) + .join("/"); } async function installFolderDependency(chunks: number, leaf?: string) { @@ -68,26 +68,26 @@ async function installFolderDependency(chunks: number, leaf?: string) { return { stdout, stderr, exitCode, installedLeaf }; } -describe.skipIf(isWindows)("isolated linker and folder dependencies deeper than PATH_MAX", () => { - test.concurrent("a file below PATH_MAX depth fails the package with ENAMETOOLONG", async () => { +test.concurrent("isolated linker installs a folder dependency with a deep tree that fits the path buffer", async () => { + const { stdout, stderr, exitCode, installedLeaf } = await installFolderDependency(FITTING_CHUNKS, "leaf.txt"); + expect(stderr).not.toContain("ENAMETOOLONG"); + expect(stdout).toMatch(/\d+ packages? installed/); + expect(exitCode).toBe(0); + expect(installedLeaf).toBe("deep"); +}); + +describe.skipIf(isWindows)("isolated linker and a folder dependency with a tree deeper than PATH_MAX", () => { + test.concurrent("fails the package with ENAMETOOLONG when a file does not fit", async () => { const { stderr, exitCode } = await installFolderDependency(OVERFLOWING_CHUNKS, "leaf.txt"); expect(stderr).toContain("ENAMETOOLONG"); expect(stderr).toContain("failed to link package: pkg@"); expect(exitCode).toBe(1); }); - test.concurrent("a directory below PATH_MAX depth fails the package with ENAMETOOLONG", async () => { + test.concurrent("fails the package with ENAMETOOLONG when only directories do not fit", async () => { const { stderr, exitCode } = await installFolderDependency(OVERFLOWING_CHUNKS); expect(stderr).toContain("ENAMETOOLONG"); expect(stderr).toContain("failed to link package: pkg@"); expect(exitCode).toBe(1); }); - - test.concurrent("a deep tree that fits PATH_MAX still installs", async () => { - const { stdout, stderr, exitCode, installedLeaf } = await installFolderDependency(FITTING_CHUNKS, "leaf.txt"); - expect(stderr).not.toContain("ENAMETOOLONG"); - expect(stdout).toMatch(/\d+ packages? installed/); - expect(exitCode).toBe(0); - expect(installedLeaf).toBe("deep"); - }); }); From e25088134e77ad39bd46e7c41899d0bf5f5501fa Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 11 Aug 2026 12:31:06 +0000 Subject: [PATCH 3/4] install(isolated): propagate a failed destination create in FileCopier; cover the copy backend, the PATH_MAX boundary and the Windows absolute path The unix FileCopier printed the error and exited the process when the destination of an entry could not be created after creating its parent; every caller already reports a returned error as the package failing. Tests now also cover the hardlink and copyfile backends walking a cached package, the destination path one byte below and exactly at PATH_MAX, and on Windows an entry whose cwd-relative store path fits but whose absolute destination does not, which is what the join guard in the Hardlinker is for. --- src/install/isolated_install/FileCopier.rs | 17 +- .../isolated-install-long-paths.test.ts | 241 +++++++++++++----- 2 files changed, 184 insertions(+), 74 deletions(-) diff --git a/src/install/isolated_install/FileCopier.rs b/src/install/isolated_install/FileCopier.rs index 119a63cd8d6a..85ca9a0d25ac 100644 --- a/src/install/isolated_install/FileCopier.rs +++ b/src/install/isolated_install/FileCopier.rs @@ -2,8 +2,6 @@ use core::ptr; use bun_alloc::AllocError; -#[cfg(not(windows))] -use bun_core::{Global, fmt as bun_fmt}; use bun_paths::path_options::{CheckLength, Kind, PathSeparators}; use bun_paths::{self, OSPathChar, OSPathSlice}; use bun_sys::{self as sys, Dir, E, EntryKind, Fd, walker_skippable, walker_skippable::Walker}; @@ -205,7 +203,7 @@ impl FileCopier { let dest = match dest_dir.create_file_z(entry.path, Default::default()) { Ok(f) => f, - Err(_) => 'dest: { + Err(_) => { if let Some(entry_dirname) = bun_paths::Dirname::dirname::(entry.path) { @@ -214,18 +212,7 @@ impl FileCopier { entry_dirname, ); } - - match dest_dir.create_file_z(entry.path, Default::default()) { - Ok(f) => break 'dest f, - Err(err) => { - bun_core::pretty_errorln!( - "{}: copy file {}", - bstr::BStr::new(err.name()), - bun_fmt::fmt_os_path(entry.path, Default::default()), - ); - Global::exit(1); - } - } + dest_dir.create_file_z(entry.path, Default::default())? } }; diff --git a/test/cli/install/isolated-install-long-paths.test.ts b/test/cli/install/isolated-install-long-paths.test.ts index 83f26c236794..97fd24b77385 100644 --- a/test/cli/install/isolated-install-long-paths.test.ts +++ b/test/cli/install/isolated-install-long-paths.test.ts @@ -1,93 +1,216 @@ -// The isolated linker copies a folder dependency with a directory walk that -// opens every directory relative to its parent, so the walk reaches entries -// whose path is longer than the buffer the destination path of each entry is -// built in. Such a package has to fail with ENAMETOOLONG, as it does with the -// hoisted linker, instead of crashing the install. +// The isolated linker installs a package with a directory walk that opens every +// directory relative to its parent, so it reaches entries whose path is longer +// than the buffer each entry's destination path is built in. Such an entry has +// to fail the package with ENAMETOOLONG (as it does with the hoisted linker) +// instead of crashing or exiting the install, whichever backend ends up +// materializing the package. import { describe, expect, test } from "bun:test"; -import { bunEnv, bunExe, isMacOS, isWindows, tempDir } from "harness"; -import { mkdirSync, readFileSync, renameSync, writeFileSync } from "node:fs"; +import { bunEnv, bunExe, isLinux, isWindows, tempDir } from "harness"; +import { cpSync, mkdirSync, readdirSync, readFileSync, realpathSync, renameSync, rmSync, writeFileSync } from "node:fs"; import { join } from "node:path"; -const SEGMENT = Buffer.alloc(100, "d").toString(); -// Every mkdir/rename issued by createDeepChain names at most CHUNK_DEPTH + 1 -// segments below the temp dir, which fits the path limit of every platform. +// Size of the buffers the destination paths are built in: PATH_MAX bytes on +// POSIX (bun_core::MAX_PATH_BYTES), 32767 UTF-16 units on Windows. +const PATH_BUFFER_LEN = isWindows ? 32767 : isLinux ? 4096 : 1024; +// Where the isolated linker materializes the `file:./pkg` dependency used +// below, relative to the project. The destination of an entry is this, a +// separator and the entry's path inside the package. +const STORE_PKG_DIR = join("node_modules", ".bun", "pkg@file+pkg", "node_modules", "pkg"); + +const SEGMENT_LEN = 100; +const SEGMENT = Buffer.alloc(SEGMENT_LEN, "d").toString(); +// createDeepChain never names more directories of a chain than this in one +// path, which keeps everything it hands to the filesystem well below the 1024 +// bytes of macOS. const CHUNK_DEPTH = 4; -const CHUNK = Array(CHUNK_DEPTH).fill(SEGMENT).join("/"); -const CHUNK_BYTES = CHUNK.length + 1; -// Size of the destination path buffers on POSIX. On Windows they hold 32767 -// UTF-16 units, which is also the filesystem's limit, so a tree that overflows -// them cannot be created there; a tree this deep has to install fine instead. -const PATH_MAX = isMacOS ? 1024 : 4096; -const OVERFLOWING_CHUNKS = Math.ceil((PATH_MAX + 256) / CHUNK_BYTES); -const FITTING_CHUNKS = isWindows ? Math.ceil(4096 / CHUNK_BYTES) : Math.floor(PATH_MAX / 2 / CHUNK_BYTES); +type Leaf = { name: string; contents: string }; +const LEAF: Leaf = { name: "leaf.js", contents: "module.exports = 'deep';" }; + +/** Directory names whose joined relative path is exactly `joinedLen` long. */ +function chainDirs(joinedLen: number): string[] { + const dirs: string[] = []; + let remaining = joinedLen; + while (remaining > 2 * SEGMENT_LEN + 1) { + dirs.push(SEGMENT); + remaining -= SEGMENT_LEN + 1; + } + dirs.push(Buffer.alloc(remaining, "e").toString()); + expect(dirs.join("/")).toHaveLength(joinedLen); + return dirs; +} /** - * Creates a straight chain of `chunks * CHUNK_DEPTH` directories in `pkgDir`, - * optionally with a file at the bottom, and returns the chain relative to - * `pkgDir`. The chunks are created side by side in `staging` and then renamed - * into each other from the bottom up, so the finished chain can be longer than - * PATH_MAX even though no single syscall sees more than one chunk of it. + * Nests `dirs` inside `pkgDir`, optionally with a file in the deepest one. The + * chain may be longer than any path the filesystem accepts: it is created as + * chunks of CHUNK_DEPTH directories side by side in `staging`, and the chunks + * are renamed into each other from the deepest one up, so no single operation + * names more than one chunk of it. */ -function createDeepChain(pkgDir: string, staging: string, chunks: number, leaf?: string): string { - for (let i = 0; i < chunks; i++) { - mkdirSync(join(staging, String(i), CHUNK), { recursive: true }); +function createDeepChain(pkgDir: string, staging: string, dirs: string[], leaf?: Leaf) { + const chunks: string[][] = []; + for (let i = 0; i < dirs.length; i += CHUNK_DEPTH) { + chunks.push(dirs.slice(i, i + CHUNK_DEPTH)); + } + for (let i = 0; i < chunks.length; i++) { + mkdirSync(join(staging, String(i), ...chunks[i]), { recursive: true }); } if (leaf !== undefined) { - writeFileSync(join(staging, String(chunks - 1), CHUNK, leaf), "deep"); + const deepest = chunks.length - 1; + writeFileSync(join(staging, String(deepest), ...chunks[deepest], leaf.name), leaf.contents); } - for (let i = chunks - 1; i > 0; i--) { - renameSync(join(staging, String(i), SEGMENT), join(staging, String(i - 1), CHUNK, SEGMENT)); + for (let i = chunks.length - 1; i > 0; i--) { + renameSync(join(staging, String(i), chunks[i][0]), join(staging, String(i - 1), ...chunks[i - 1], chunks[i][0])); } - renameSync(join(staging, "0", SEGMENT), join(pkgDir, SEGMENT)); - return Array(chunks * CHUNK_DEPTH) - .fill(SEGMENT) - .join("/"); + renameSync(join(staging, "0", chunks[0][0]), join(pkgDir, chunks[0][0])); } -async function installFolderDependency(chunks: number, leaf?: string) { - using dir = tempDir("isolated-long-paths", { +/** A project depending on `file:./pkg`, with `dirs` (and `leaf`) inside pkg. */ +function projectWithDeepFolderDependency(name: string, dirs: string[], leaf?: Leaf) { + const dir = tempDir(name, { "package.json": JSON.stringify({ name: "proj", dependencies: { pkg: "file:./pkg" } }), "pkg/package.json": JSON.stringify({ name: "pkg", version: "1.0.0" }), }); - const chain = createDeepChain(join(String(dir), "pkg"), join(String(dir), "staging"), chunks, leaf); + createDeepChain(join(String(dir), "pkg"), join(String(dir), "staging"), dirs, leaf); + return dir; +} +async function bunInstall(projectDir: string, ...args: string[]) { await using proc = Bun.spawn({ - cmd: [bunExe(), "install", "--linker", "isolated"], - cwd: String(dir), - env: { ...bunEnv, BUN_INSTALL_CACHE_DIR: join(String(dir), "cache") }, + cmd: [bunExe(), "install", "--linker", "isolated", ...args], + cwd: projectDir, + env: { ...bunEnv, BUN_INSTALL_CACHE_DIR: join(projectDir, ".cache") }, stdout: "pipe", stderr: "pipe", }); const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); - - const installedLeaf = - exitCode === 0 && leaf !== undefined - ? readFileSync(join(String(dir), "node_modules", "pkg", chain, leaf), "utf8") - : undefined; - return { stdout, stderr, exitCode, installedLeaf }; + return { stdout, stderr, exitCode }; } -test.concurrent("isolated linker installs a folder dependency with a deep tree that fits the path buffer", async () => { - const { stdout, stderr, exitCode, installedLeaf } = await installFolderDependency(FITTING_CHUNKS, "leaf.txt"); +type InstallResult = Awaited>; + +function expectInstalled({ stdout, stderr, exitCode }: InstallResult) { expect(stderr).not.toContain("ENAMETOOLONG"); expect(stdout).toMatch(/\d+ packages? installed/); expect(exitCode).toBe(0); - expect(installedLeaf).toBe("deep"); +} + +function expectPackageFailure({ stdout, stderr, exitCode }: InstallResult, packageName: string) { + expect(stderr).toContain("ENAMETOOLONG"); + expect(stderr).toContain(`failed to link package: ${packageName}@`); + expect(stdout).toContain("Failed to install 1 package"); + expect(exitCode).toBe(1); +} + +/** + * Reads an installed file by its project-relative path from a process running + * in the project, for files whose path joined to the project directory is + * longer than what this process may hand to the filesystem. + */ +async function readInstalledFile(projectDir: string, relativePath: string) { + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", `process.stdout.write(require("fs").readFileSync(${JSON.stringify(relativePath)}, "utf8"))`], + cwd: projectDir, + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(exitCode).toBe(0); + return stdout; +} + +test.concurrent("a folder dependency with a deep tree that fits installs", async () => { + // Past the 260 characters of Windows' legacy MAX_PATH, well inside the buffer everywhere. + const dirs = chainDirs(isWindows ? 4096 : PATH_BUFFER_LEN / 2); + using dir = projectWithDeepFolderDependency("isolated-deep-fits", dirs, LEAF); + + expectInstalled(await bunInstall(String(dir))); + expect(readFileSync(join(String(dir), "node_modules", "pkg", ...dirs, LEAF.name), "utf8")).toBe(LEAF.contents); }); -describe.skipIf(isWindows)("isolated linker and a folder dependency with a tree deeper than PATH_MAX", () => { - test.concurrent("fails the package with ENAMETOOLONG when a file does not fit", async () => { - const { stderr, exitCode } = await installFolderDependency(OVERFLOWING_CHUNKS, "leaf.txt"); - expect(stderr).toContain("ENAMETOOLONG"); - expect(stderr).toContain("failed to link package: pkg@"); - expect(exitCode).toBe(1); +// On Windows the buffer holds as much as the filesystem does, so no tree that +// exists overflows it with a store-relative path; the Windows test at the +// bottom overflows with the absolute path instead. +describe.skipIf(isWindows)("a folder dependency with a tree deeper than PATH_MAX", () => { + test.concurrent("fails with ENAMETOOLONG when a directory does not fit", async () => { + using dir = projectWithDeepFolderDependency("isolated-deep-dir", chainDirs(PATH_BUFFER_LEN + 256)); + expectPackageFailure(await bunInstall(String(dir)), "pkg"); }); - test.concurrent("fails the package with ENAMETOOLONG when only directories do not fit", async () => { - const { stderr, exitCode } = await installFolderDependency(OVERFLOWING_CHUNKS); - expect(stderr).toContain("ENAMETOOLONG"); - expect(stderr).toContain("failed to link package: pkg@"); - expect(exitCode).toBe(1); + // The two trees below differ by one byte of destination path. Their + // directories fit either way, so the file is the entry that decides. + const leaf: Leaf = { name: "f", contents: LEAF.contents }; + const dirsForDestinationLength = (destinationLen: number) => + chainDirs(destinationLen - (STORE_PKG_DIR.length + 1) - (1 + leaf.name.length)); + + test.concurrent("installs a file whose destination path is one byte short of PATH_MAX", async () => { + const dirs = dirsForDestinationLength(PATH_BUFFER_LEN - 1); + const destination = join(STORE_PKG_DIR, ...dirs, leaf.name); + expect(destination).toHaveLength(PATH_BUFFER_LEN - 1); + using dir = projectWithDeepFolderDependency("isolated-exact-fits", dirs, leaf); + + expectInstalled(await bunInstall(String(dir))); + expect(await readInstalledFile(String(dir), destination)).toBe(leaf.contents); }); + + test.concurrent("fails with ENAMETOOLONG for a file whose destination path is exactly PATH_MAX", async () => { + const dirs = dirsForDestinationLength(PATH_BUFFER_LEN); + expect(join(STORE_PKG_DIR, ...dirs, leaf.name)).toHaveLength(PATH_BUFFER_LEN); + using dir = projectWithDeepFolderDependency("isolated-exact-over", dirs, leaf); + + expectPackageFailure(await bunInstall(String(dir)), "pkg"); + }); +}); + +// Folder dependencies are always hardlinked; --backend selects how packages +// from the cache are materialized. Install once to populate the cache, add the +// deep tree to the cached copy, and reinstall from it. +describe.skipIf(isWindows)("a cached package with a tree deeper than PATH_MAX", () => { + for (const backend of ["hardlink", "copyfile"]) { + test.concurrent(`fails with ENAMETOOLONG with the ${backend} backend`, async () => { + using dir = tempDir(`isolated-cached-${backend}`, { + "package.json": JSON.stringify({ name: "proj", dependencies: { bar: "file:./bar-0.0.2.tgz" } }), + }); + cpSync(join(import.meta.dir, "bar-0.0.2.tgz"), join(String(dir), "bar-0.0.2.tgz")); + expectInstalled(await bunInstall(String(dir))); + + const cacheEntries = join(String(dir), ".cache", "bar"); + const [cachedPackage] = readdirSync(cacheEntries).map(entry => realpathSync(join(cacheEntries, entry))); + createDeepChain(cachedPackage, join(String(dir), "staging"), chainDirs(PATH_BUFFER_LEN + 256), LEAF); + rmSync(join(String(dir), "node_modules"), { recursive: true }); + + expectPackageFailure(await bunInstall(String(dir), "--backend", backend), "bar"); + }); + } }); + +// On Windows a file is linked by its absolute destination, `\??\`, the working +// directory and the store path, assembled in a buffer of PATH_BUFFER_LEN units +// after the store-relative part passed its own length check. The package's own +// paths are shorter than their destinations by the store prefix, so a package +// that exists on disk can still have a destination that does not fit. The +// entry below sits in the middle of that window. +test.skipIf(!isWindows)( + "a folder dependency whose absolute destination does not fit fails with ENAMETOOLONG", + async () => { + using dir = tempDir("isolated-absolute-over", { + "package.json": JSON.stringify({ name: "proj", dependencies: { pkg: "file:./pkg" } }), + "pkg/package.json": JSON.stringify({ name: "pkg", version: "1.0.0" }), + }); + const projectDir = realpathSync.native(String(dir)); + const destinationPrefix = `\\??\\${projectDir}\\${STORE_PKG_DIR}\\`; + const sourcePrefix = `\\\\?\\${projectDir}\\pkg\\`; + const entryLen = PATH_BUFFER_LEN - Math.ceil((destinationPrefix.length + sourcePrefix.length) / 2); + expect(destinationPrefix.length + entryLen).toBeGreaterThanOrEqual(PATH_BUFFER_LEN); + expect(sourcePrefix.length + entryLen).toBeLessThan(PATH_BUFFER_LEN); + // A long file name keeps every directory of the chain addressable, so the + // file is the entry that does not fit. + const leaf: Leaf = { name: Buffer.alloc(SEGMENT_LEN, "f").toString(), contents: LEAF.contents }; + const dirs = chainDirs(entryLen - 1 - leaf.name.length); + createDeepChain(join(String(dir), "pkg"), join(String(dir), "staging"), dirs, leaf); + + expectPackageFailure(await bunInstall(String(dir)), "pkg"); + }, +); From 7d99e1d4f139f164e1a9515c499e56908dbf95a8 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 11 Aug 2026 12:47:36 +0000 Subject: [PATCH 4/4] install(isolated): keep the Windows FileCopier match in place; shorten comments --- src/install/isolated_install/FileCopier.rs | 115 +++++++++------------ src/install/isolated_install/Hardlinker.rs | 11 +- src/paths/Path.rs | 10 +- 3 files changed, 54 insertions(+), 82 deletions(-) diff --git a/src/install/isolated_install/FileCopier.rs b/src/install/isolated_install/FileCopier.rs index 85ca9a0d25ac..a0f7090374e9 100644 --- a/src/install/isolated_install/FileCopier.rs +++ b/src/install/isolated_install/FileCopier.rs @@ -10,8 +10,7 @@ use bun_sys::{self as sys, Dir, E, EntryKind, Fd, walker_skippable, walker_skipp // u16 on Windows — encoded via `OSPathChar` so `slice()`/`slice_z()` produce // the platform-native width. The auto separator mode normalizes `/` → `\` on Windows // during `from`/`append`, which is load-bearing for the Win32 calls below. -// Length-checked for the same reason as the `Hardlinker` paths: the walker's -// entry paths appended on Windows are not bounded by any path buffer. +// Length-checked like the Hardlinker's paths: the walker entries appended on Windows are unbounded. type AbsPathAutoOs = bun_paths::AbsPath; type PathAutoOs = @@ -112,75 +111,63 @@ impl FileCopier { // restore via `set_length` after the body instead. let src_saved_len = self.src_path.len(); let dest_saved_len = self.dest_subpath.len(); + let appended = self.src_path.append(entry.path.as_slice()).is_ok() + && self.dest_subpath.append(entry.path.as_slice()).is_ok(); - let result: sys::Result<()> = 'entry: { - if self.src_path.append(entry.path.as_slice()).is_err() - || self.dest_subpath.append(entry.path.as_slice()).is_err() - { - break 'entry sys::Result::Err(sys::Error::from_code( - E::ENAMETOOLONG, - sys::Tag::copyfile, - )); + let result: sys::Result<()> = match entry.kind { + _ if !appended => { + sys::Result::Err(sys::Error::from_code(E::ENAMETOOLONG, sys::Tag::copyfile)) } - - match entry.kind { - EntryKind::Directory => { - // SAFETY: FFI — both `slice_z()` are NUL-terminated WStrs. - if unsafe { - bun_sys::windows::CreateDirectoryExW( - self.src_path.slice_z().as_ptr(), - self.dest_subpath.slice_z().as_ptr(), - ptr::null_mut(), - ) - } == 0 - { - // CreateDirectoryExW also fails when the directory already - // exists; `make_path` treats that as success and reports - // anything else. - bun_sys::make_path::make_path::( - &dest_dir, - entry.path.as_slice(), - ) - } else { - sys::Result::Ok(()) - } + EntryKind::Directory => { + // SAFETY: FFI — both `slice_z()` are NUL-terminated WStrs. + if unsafe { + bun_sys::windows::CreateDirectoryExW( + self.src_path.slice_z().as_ptr(), + self.dest_subpath.slice_z().as_ptr(), + ptr::null_mut(), + ) + } == 0 + { + // Also taken when the directory exists; make_path treats that as success. + bun_sys::make_path::make_path::(&dest_dir, entry.path.as_slice()) + } else { + sys::Result::Ok(()) } - EntryKind::File => { - match bun_sys::copy_file::copy_file( - self.src_path.slice_z(), - self.dest_subpath.slice_z(), - ) { - sys::Result::Ok(()) => sys::Result::Ok(()), - sys::Result::Err(first_err) => { - // Retry after creating the parent directory. - // For root-level files (`index.js`, - // `package.json`, `LICENSE`) `dirname` is - // null and there is no missing parent to - // create — `dest_dir` itself was already - // opened above — so the original error is the - // real failure and must propagate. Silently - // continuing here would let a staged - // global-store entry be renamed into place - // with files missing. - match bun_paths::Dirname::dirname::(entry.path.as_slice()) - { - None => sys::Result::Err(first_err), - Some(entry_dirname) => { - let _ = bun_sys::make_path::make_path::( - &dest_dir, - entry_dirname, - ); - bun_sys::copy_file::copy_file( - self.src_path.slice_z(), - self.dest_subpath.slice_z(), - ) - } + } + EntryKind::File => { + match bun_sys::copy_file::copy_file( + self.src_path.slice_z(), + self.dest_subpath.slice_z(), + ) { + sys::Result::Ok(()) => sys::Result::Ok(()), + sys::Result::Err(first_err) => { + // Retry after creating the parent directory. + // For root-level files (`index.js`, + // `package.json`, `LICENSE`) `dirname` is + // null and there is no missing parent to + // create — `dest_dir` itself was already + // opened above — so the original error is the + // real failure and must propagate. Silently + // continuing here would let a staged + // global-store entry be renamed into place + // with files missing. + match bun_paths::Dirname::dirname::(entry.path.as_slice()) { + None => sys::Result::Err(first_err), + Some(entry_dirname) => { + let _ = bun_sys::make_path::make_path::( + &dest_dir, + entry_dirname, + ); + bun_sys::copy_file::copy_file( + self.src_path.slice_z(), + self.dest_subpath.slice_z(), + ) } } } } - _ => unreachable!(), } + _ => unreachable!(), }; self.src_path.set_length(src_saved_len); @@ -218,8 +205,6 @@ impl FileCopier { #[cfg(unix)] { - // `dest` has already been created (or truncated) above, so - // skipping this entry would leave an empty file behind. let stat = bun_sys::fstat(src.handle())?; // SAFETY: fchmod is safe to call with any fd + mode; errors are ignored (`_ =`). unsafe { diff --git a/src/install/isolated_install/Hardlinker.rs b/src/install/isolated_install/Hardlinker.rs index fef52f6a643f..2fafc8eedc17 100644 --- a/src/install/isolated_install/Hardlinker.rs +++ b/src/install/isolated_install/Hardlinker.rs @@ -7,15 +7,10 @@ use bun_sys::{self as sys, EntryKind, Fd, FdExt}; // on Windows — encoded here via the `OSPathChar` type alias so the struct's // `slice()`/`slice_z()` produce the platform-native width without per-field // `#[cfg]` divergence. -// -// The walker's entry paths get appended to these per entry, and the walker -// opens every directory relative to its parent, so an entry path can be longer -// than any path buffer. `CheckLength::CHECK` turns that into `ENAMETOOLONG` -// for the package (what the hoisted linker reports for the same tree) instead -// of an out-of-bounds write in `append`. use bun_paths::path_options::{CheckLength, Kind, PathSeparators}; use bun_paths::{AbsPath, OSPathChar, OSPathSlice, Path}; +// Length-checked: the walker opens each directory relative to its parent, so entry paths are unbounded. type OsAbsPath = AbsPath; type OsPath = Path; @@ -143,9 +138,7 @@ impl Hardlinker { dest_slice, ] }; - // `join_string_buf_w_same` and `add_nt_path_prefix_if_needed` - // write into the pooled buffers unchecked: the parts, a - // separator per part, the `\??\` prefix and the NUL must fit. + // The join and the NT prefix below write into the pooled buffers unchecked. let needed_len = dest_parts.iter().map(|part| part.len() + 1).sum::() + bun_paths::windows::NT_OBJECT_PREFIX.len(); diff --git a/src/paths/Path.rs b/src/paths/Path.rs index 2044dd477040..d19fc3b31f00 100644 --- a/src/paths/Path.rs +++ b/src/paths/Path.rs @@ -80,8 +80,7 @@ pub mod options { } impl CheckLength { pub(crate) const ASSUME: u8 = 0; - /// Pass as the `CHECK` const param (or convert with - /// [`Path::into_checked`]) when unbounded input gets appended. + /// For paths that get unbounded input appended; see also [`Path::into_checked`]. pub const CHECK: u8 = 1; #[inline(always)] pub(crate) const fn from_u8(v: u8) -> Self { @@ -857,12 +856,7 @@ impl self.reinterpret() } - /// Reinterpret this path as length-checked: from here on `append` and - /// friends return `Err(MaxPathExceeded)` for input that does not fit - /// instead of panicking. For paths built with the `ASSUME`-only helpers - /// (`PathLike`) that are about to receive unbounded input, e.g. the - /// entries of a directory walk. Like `SEP_OPT`, `CHECK` only selects how - /// later mutations behave, so this is a no-op move. + /// [`Self::into_sep`] for `CHECK`: from here on over-long input is `Err(MaxPathExceeded)`. #[inline] pub fn into_checked(self) -> Path { self.reinterpret()