diff --git a/src/install/lockfile/Package/WorkspaceMap.rs b/src/install/lockfile/Package/WorkspaceMap.rs index e85671fe247b..07052f36bafd 100644 --- a/src/install/lockfile/Package/WorkspaceMap.rs +++ b/src/install/lockfile/Package/WorkspaceMap.rs @@ -2,7 +2,7 @@ use bstr::BStr; use bun_alloc::Arena; // bumpalo::Bump re-export use bun_ast as js_ast; use bun_collections::StringArrayHashMap; -use bun_core::{ZStr, strings}; +use bun_core::strings; use bun_glob as glob; use bun_paths as path; use bun_paths::resolve_path; @@ -117,13 +117,13 @@ impl<'a> NamesArray<'a> { fn process_workspace_name( json_cache: &mut WorkspacePackageJSONCache, - abs_package_json_path: &ZStr, + abs_package_json_path: &[u8], log: &mut bun_ast::Log, ) -> crate::Result { let workspace_json = json_cache .get_with_path( log, - abs_package_json_path.as_bytes(), + abs_package_json_path, GetJSONOptions { init_reset_store: false, guess_indentation: true, @@ -159,7 +159,7 @@ fn process_workspace_name( bun_output::scoped_log!( Lockfile, "processWorkspaceName({}) = {}", - BStr::new(abs_package_json_path.as_bytes()), + BStr::new(abs_package_json_path), BStr::new(&entry.name) ); @@ -215,48 +215,52 @@ impl WorkspaceMap { continue; } - let abs_package_json_path: &ZStr = - resolve_path::join_abs_string_buf_z::( - source.path.name().dir, - filepath_buf, - &[input_path, b"package.json"], - ); - - // skip root package.json - if strings::eql_long( - resolve_path::dirname::(abs_package_json_path.as_bytes()), + let processed = match resolve_path::join_abs_string_buf_checked::( source.path.name().dir, - true, + filepath_buf, + &[input_path, b"package.json"], ) { - continue; - } + Some(abs_package_json_path) => { + // skip root package.json + if strings::eql_long( + resolve_path::dirname::(abs_package_json_path), + source.path.name().dir, + true, + ) { + continue; + } - let workspace_entry = - match process_workspace_name(json_cache, abs_package_json_path, log) { - Ok(e) => e, - Err(err) => { - if err == crate::Error::Sys(bun_errno::SystemErrno::EISDIR) - || err == crate::Error::Sys(bun_errno::SystemErrno::EPERM) - || err == crate::Error::Sys(bun_errno::SystemErrno::ENOENT) - { - let _ = log.add_error_fmt( - Some(source), - arr.item_loc(source, i), - format_args!("Workspace not found \"{}\"", BStr::new(input_path)), - ); - } else if err == crate::Error::MissingPackageName { - let _ = log.add_error_fmt( - Some(source), - loc, - format_args!( - "Missing \"name\" from package.json in {}", - BStr::new(input_path) - ), - ); - } else { - let mut cwd_buf = vec![0u8; MAX_PATH_BYTES]; - let cwd_len = bun_sys::getcwd(&mut cwd_buf).expect("unreachable"); - let _ = log.add_error_fmt( + process_workspace_name(json_cache, abs_package_json_path, log) + .map(|entry| (abs_package_json_path, entry)) + } + None => Err(crate::Error::Sys(bun_errno::SystemErrno::ENAMETOOLONG)), + }; + + let (abs_package_json_path, workspace_entry) = match processed { + Ok(processed) => processed, + Err(err) => { + if err == crate::Error::Sys(bun_errno::SystemErrno::EISDIR) + || err == crate::Error::Sys(bun_errno::SystemErrno::EPERM) + || err == crate::Error::Sys(bun_errno::SystemErrno::ENOENT) + { + let _ = log.add_error_fmt( + Some(source), + arr.item_loc(source, i), + format_args!("Workspace not found \"{}\"", BStr::new(input_path)), + ); + } else if err == crate::Error::MissingPackageName { + let _ = log.add_error_fmt( + Some(source), + loc, + format_args!( + "Missing \"name\" from package.json in {}", + BStr::new(input_path) + ), + ); + } else { + let mut cwd_buf = vec![0u8; MAX_PATH_BYTES]; + let cwd_len = bun_sys::getcwd(&mut cwd_buf).expect("unreachable"); + let _ = log.add_error_fmt( Some(source), arr.item_loc(source, i), format_args!( @@ -266,10 +270,10 @@ impl WorkspaceMap { BStr::new(&cwd_buf[..cwd_len]), ), ); - } - continue; } - }; + continue; + } + }; if workspace_entry.name.len() == 0 { continue; @@ -278,7 +282,7 @@ impl WorkspaceMap { let rel_input_path = resolve_path::relative_platform::( source.path.name().dir, strings::without_suffix_comptime( - abs_package_json_path.as_bytes(), + abs_package_json_path, const_format::concatcp!(SEP_STR, "package.json").as_bytes(), ), ); @@ -333,7 +337,10 @@ impl WorkspaceMap { b"package.json" } else { let parts: [&[u8]; 2] = [user_pattern, b"package.json"]; - arena.alloc_slice_copy(resolve_path::join::(&parts)) + let mut spill: Vec = Vec::new(); + arena.alloc_slice_copy(resolve_path::join_spill::( + &mut spill, &parts, + )) }; let mut cwd = resolve_path::dirname::(source.path.text); @@ -436,22 +443,20 @@ impl WorkspaceMap { BStr::new(entry_dir) ); - let abs_package_json_path = resolve_path::join_abs_string_buf_z::< + let processed = match resolve_path::join_abs_string_buf_checked::< path::platform::Auto, >( cwd, filepath_buf, &[entry_dir, b"package.json"] - ); - let abs_workspace_dir_path: &[u8] = strings::without_suffix_comptime( - abs_package_json_path.as_bytes(), - b"package.json", - ); - - let workspace_entry = match process_workspace_name( - json_cache, - abs_package_json_path, - log, ) { - Ok(e) => e, + Some(abs_package_json_path) => { + process_workspace_name(json_cache, abs_package_json_path, log) + .map(|entry| (abs_package_json_path, entry)) + } + None => Err(crate::Error::Sys(bun_errno::SystemErrno::ENAMETOOLONG)), + }; + + let (abs_package_json_path, workspace_entry) = match processed { + Ok(processed) => processed, Err(err) => { let entry_base: &[u8] = path::basename(matched_path); if err == crate::Error::Sys(bun_errno::SystemErrno::ENOENT) { @@ -488,6 +493,8 @@ impl WorkspaceMap { continue; } + let abs_workspace_dir_path: &[u8] = + strings::without_suffix_comptime(abs_package_json_path, b"package.json"); let workspace_path: &[u8] = resolve_path::relative_platform::( source.path.name().dir, diff --git a/test/cli/install/bad-workspace.test.ts b/test/cli/install/bad-workspace.test.ts index e2101f0a3fe3..df235114c4d5 100644 --- a/test/cli/install/bad-workspace.test.ts +++ b/test/cli/install/bad-workspace.test.ts @@ -1,7 +1,9 @@ -import { spawnSync } from "bun"; -import { beforeEach, expect, setDefaultTimeout, test } from "bun:test"; +import { spawn, spawnSync } from "bun"; +import { install_test_helpers } from "bun:internal-for-testing"; +import { beforeEach, describe, expect, setDefaultTimeout, test } from "bun:test"; import { mkdirSync, writeFileSync } from "fs"; -import { bunEnv, bunExe, tempDir, tmpdirSync } from "harness"; +import { bunEnv, bunExe, isLinux, isWindows, tempDir, tmpdirSync } from "harness"; +import { dirname, join } from "path"; let cwd: string; @@ -143,3 +145,156 @@ test("workspace with .\\ should not crash", () => { expect(text).not.toContain("panic"); expect(text).not.toContain("Internal assertion failure"); }); + +// Each `workspaces` entry is joined onto the project directory in a path buffer of +// MAX_PATH_BYTES: 4096 bytes on Linux, 1024 on macOS (on Windows 32767 * 3 + 1, more than +// any path the OS accepts, so the tests around the buffer size are POSIX only). Glob +// entries are first joined onto "package.json" in a 4096 byte buffer on every platform. +const POSIX_PATH_BUFFER_BYTES = isLinux ? 4096 : 1024; +// Longer than either buffer on every platform. +const LONG_ENTRY_BYTES = 100_000; + +const PKG1 = { "pkgs/pkg1/package.json": JSON.stringify({ name: "pkg1" }) }; + +function rootPackageJson(workspaces: string[]) { + return JSON.stringify({ name: "root", workspaces }); +} + +async function runInstall(cwd: string) { + await using proc = spawn({ + cmd: [bunExe(), "install"], + cwd, + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + return { stdout, stderr, exitCode }; +} + +// Installs and asserts that pkgs/pkg1 is the only workspace package that was found. +async function expectOnlyPkg1Found(dir: string) { + const { stderr, exitCode } = await runInstall(dir); + expect(stderr).not.toContain("error:"); + expect(exitCode).toBe(0); + expect(Object.values(install_test_helpers.parseLockfile(dir).workspace_paths)).toEqual(["pkgs/pkg1"]); +} + +describe.concurrent("workspaces entries longer than the path buffer", () => { + test("path entry fails with ENAMETOOLONG", async () => { + const entry = Buffer.alloc(LONG_ENTRY_BYTES, "a").toString(); + using dir = tempDir("bad-workspace-long-path", { "package.json": rootPackageJson([entry]) }); + + const { stderr, exitCode } = await runInstall(String(dir)); + + expect(stderr).toContain(`error: ENAMETOOLONG reading package.json for workspace package "${entry}"`); + expect(exitCode).toBe(1); + }); + + // A relative path of exactly `bytes` bytes made of one letter directory names, so that a + // path which fits the buffer is looked up by the OS (ENOENT) instead of exceeding its + // limit on the length of a single name. + function pathOfLength(bytes: number) { + const tail = bytes % 2 === 0 ? "dd" : "d"; + return Buffer.alloc(bytes - tail.length, "d/").toString() + tail; + } + + // The entry is read from `${dir}/${entry}/package.json`. One byte below the buffer size + // that path still reaches the OS; from the buffer size on it is rejected before that. + test.skipIf(isWindows).each([ + ["one byte below", -1, "Workspace not found"], + ["exactly", 0, "ENAMETOOLONG reading package.json for workspace package"], + ["one byte above", 1, "ENAMETOOLONG reading package.json for workspace package"], + ])("path entry whose package.json path is %s the path buffer size", async (_, offset, message) => { + using dir = tempDir("bad-workspace-path-buffer-edge", {}); + const prefixBytes = Buffer.byteLength(String(dir)) + "/".length; + const entry = pathOfLength(POSIX_PATH_BUFFER_BYTES + offset - prefixBytes - "/package.json".length); + writeFileSync(join(String(dir), "package.json"), rootPackageJson([entry])); + + const { stderr, exitCode } = await runInstall(String(dir)); + + expect(stderr).toContain(`error: ${message} "${entry}"`); + expect(exitCode).toBe(1); + }); + + test("glob entry is still matched", async () => { + // A brace group padded far past the buffer size; its first alternative matches `pkgs`. + const entry = "{pkgs," + Buffer.alloc(LONG_ENTRY_BYTES, "x,").toString() + "x}/*"; + using dir = tempDir("bad-workspace-long-glob", { "package.json": rootPackageJson([entry]), ...PKG1 }); + + await expectOnlyPkg1Found(String(dir)); + }); + + test("glob entry matching nothing is skipped like any other glob", async () => { + const entry = Buffer.alloc(LONG_ENTRY_BYTES, "a").toString() + "/*"; + using dir = tempDir("bad-workspace-long-glob-no-match", { + "package.json": rootPackageJson(["pkgs/*", entry]), + ...PKG1, + }); + + await expectOnlyPkg1Found(String(dir)); + }); + + // What has to fit the buffer is the normalized path, not the entry as written. + test.each([ + ["path", "pkgs/pkg1"], + ["glob", "pkgs/*"], + ])("%s entry that only fits the path buffer once normalized resolves", async (_, suffix) => { + const entry = Buffer.alloc(LONG_ENTRY_BYTES, "x/../").toString() + suffix; + using dir = tempDir("bad-workspace-long-normalized", { "package.json": rootPackageJson([entry]), ...PKG1 }); + + await expectOnlyPkg1Found(String(dir)); + }); + + // Directory names for a relative path of exactly `bytes` bytes: "deep" followed by names + // of at most NAME_MAX (255) bytes. The last name is at least 128 bytes long so that its + // parent directory, used as a cwd below, stays well within PATH_MAX. + function deepDirectoryNames(bytes: number) { + const names = ["deep"]; + let remaining = bytes - names[0].length; + while (remaining > 256) { + const length = remaining - 256 >= 129 ? 255 : remaining - 130; + names.push(Buffer.alloc(length, "a").toString()); + remaining -= "/".length + length; + } + names.push(Buffer.alloc(remaining - "/".length, "b").toString()); + return names; + } + + // Globs are walked relative to the project directory, so a match can lie deeper than the + // buffer holds once the project directory is put in front of it. Windows has no such + // depth: the buffer there holds more than the longest path the OS accepts. + test.skipIf(isWindows)( + "glob match whose absolute package.json path does not fit fails with ENAMETOOLONG", + async () => { + using dir = tempDir("bad-workspace-deep-glob-match", { "package.json": rootPackageJson(["deep/**"]) }); + // The directory itself fits PATH_MAX (so it can be created), `/package.json` + // does not fit the buffer. + const absoluteDirBytes = POSIX_PATH_BUFFER_BYTES - 6; + const names = deepDirectoryNames(absoluteDirBytes - Buffer.byteLength(String(dir)) - "/".length); + const workspaceDir = join(String(dir), ...names); + expect(Buffer.byteLength(workspaceDir)).toBe(absoluteDirBytes); + mkdirSync(workspaceDir, { recursive: true }); + // Too long to be written by its absolute path: write it relative to the parent directory. + await using writer = spawn({ + cmd: [ + bunExe(), + "-e", + `require("fs").writeFileSync(process.argv.at(-1) + "/package.json", JSON.stringify({ name: "deep" }))`, + names.at(-1)!, + ], + cwd: dirname(workspaceDir), + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + expect(await writer.stderr.text()).toBe(""); + expect(await writer.exited).toBe(0); + + const { stderr, exitCode } = await runInstall(String(dir)); + + expect(stderr).toContain(`error: ENAMETOOLONG reading package.json for workspace package "${names.join("/")}"`); + expect(exitCode).toBe(1); + }, + ); +});