Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 23 additions & 8 deletions src/install/PackageInstaller.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1568,16 +1568,31 @@ impl<'a> PackageInstaller<'a> {
installer.cache_dir = Fd::cwd();
} else {
let global_link_dir = package_manager::global_link_dir_path(self.manager_mut());
let sep_len = (global_link_dir[global_link_dir.len() - 1] != SEP) as usize;
let len = global_link_dir.len() + sep_len + folder.len();
// `folder` is the `link:` specifier as written in package.json.
if len >= self.folder_path_buf.len() {
if log_level != Options::LogLevel::Silent {
Output::err(
"ENAMETOOLONG",
"link path for package <b>{}<r> is too long",
(bstr::BStr::new(pkg_name.slice(string_buf!())),),
);
}
self.summary.fail += 1;
self.increment_tree_install_count(
!IS_PENDING_PACKAGE_INSTALL,
self.current_tree_id,
log_level,
);
return;
}
let buf = self.folder_path_buf.as_mut_slice();
let mut len = 0usize;
buf[len..len + global_link_dir.len()].copy_from_slice(global_link_dir);
len += global_link_dir.len();
if global_link_dir[global_link_dir.len() - 1] != SEP {
buf[len] = SEP;
len += 1;
buf[..global_link_dir.len()].copy_from_slice(global_link_dir);
if sep_len != 0 {
buf[global_link_dir.len()] = SEP;
}
buf[len..len + folder.len()].copy_from_slice(folder);
len += folder.len();
buf[global_link_dir.len() + sep_len..len].copy_from_slice(folder);
buf[len] = 0;
// SAFETY: buf[len] == 0 written above
installer.cache_dir_subpath = ZStr::from_buf(&self.folder_path_buf, len);
Expand Down
14 changes: 8 additions & 6 deletions src/install/patch_install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -394,9 +394,11 @@ impl PatchTask {
let patchfile_path = &patch.patchfilepath;

let mut absolute_patchfile_path_buf = PathBuffer::uninit();
let mut absolute_patchfile_path_spill = Vec::new();
// 1. Parse the patch file
let absolute_patchfile_path = path::resolve_path::join_z_buf::<path::platform::Auto>(
let absolute_patchfile_path = path::resolve_path::join_z_buf_spill::<path::platform::Auto>(
&mut absolute_patchfile_path_buf.0,
&mut absolute_patchfile_path_spill,
&[dir, patchfile_path],
);
// TODO: can the patch file be anything other than utf-8?
Expand Down Expand Up @@ -609,9 +611,11 @@ impl PatchTask {
let patchfile_path = &calc_hash.patchfile_path;

let mut absolute_patchfile_path_buf = PathBuffer::uninit();
let mut absolute_patchfile_path_spill = Vec::new();
// parse the patch file
let absolute_patchfile_path = path::resolve_path::join_z_buf::<path::platform::Auto>(
let absolute_patchfile_path = path::resolve_path::join_z_buf_spill::<path::platform::Auto>(
&mut absolute_patchfile_path_buf.0,
&mut absolute_patchfile_path_spill,
&[dir, patchfile_path],
);

Expand Down Expand Up @@ -639,12 +643,10 @@ impl PatchTask {
);
return None;
}
bun_ast::add_warning_pretty!(
log,
log.add_error_fmt(
None,
Loc::EMPTY,
"patchfile <b>{}<r> is empty, please restore or delete it.",
BStr::new(absolute_patchfile_path.as_bytes()),
format_args!("failed to read patch file: {}", e),
);
return None;
}
Expand Down
171 changes: 85 additions & 86 deletions src/install/resolvers/folder_resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use core::fmt;

use bun_core::fmt::QuotedFormatter;
use bun_core::{ZStr, strings};
use bun_paths::{self, MAX_PATH_BYTES, PathBuffer, SEP, SEP_STR};
use bun_paths::{self, PathBuffer, SEP, SEP_STR, resolve_path};
use bun_resolver::fs::FileSystem;
use bun_semver::{self as semver, String as SemverString};
use bun_sys::{self, Fd, File, O};
Expand Down Expand Up @@ -35,7 +35,6 @@ pub(crate) struct PackageWorkspaceSearchPathFormatter<'a> {

impl<'a> fmt::Display for PackageWorkspaceSearchPathFormatter<'a> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let mut joined = [0u8; MAX_PATH_BYTES + 2];
// Caller constructs this formatter only when
// `self.version.tag == .workspace`.
let workspace = self.version.workspace();
Expand All @@ -48,34 +47,38 @@ impl<'a> fmt::Display for PackageWorkspaceSearchPathFormatter<'a> {
))
.unwrap_or(workspace);

// SAFETY: joined[2..] is exactly MAX_PATH_BYTES bytes long.
let joined_path: &mut PathBuffer =
unsafe { &mut *joined.as_mut_ptr().add(2).cast::<PathBuffer>() };
let mut paths = normalize_package_json_path(
GlobalOrRelative::Relative(dependency::version::Tag::Workspace),
joined_path,
self.manager.lockfile.str(str_to_use),
);
let search_path = self.manager.lockfile.str(str_to_use);

if !strings::starts_with_char(paths.rel, b'.') && !strings::starts_with_char(paths.rel, SEP)
{
joined[0] = b'.';
joined[1] = SEP;
// `paths.rel` points into `joined[2..]`; extend the view backward
// by the two bytes just written via safe slicing of `joined`.
let n = paths.rel.len() + 2;
paths.rel = &joined[..n];
}
let mut joined = PathBuffer::uninit();
let mut dot_slash_rel = Vec::new();
let rel: &[u8] = match normalize_package_json_path(
GlobalOrRelative::Relative(dependency::version::Tag::Workspace),
&mut joined,
search_path,
) {
Some(paths)
if !strings::starts_with_char(paths.rel, b'.')
&& !strings::starts_with_char(paths.rel, SEP) =>
{
dot_slash_rel.push(b'.');
dot_slash_rel.push(SEP);
dot_slash_rel.extend_from_slice(paths.rel);
dot_slash_rel.as_slice()
}
Some(paths) => paths.rel,
// Too long to be a path; show it as written.
None => search_path,
};
Comment thread
coderabbitai[bot] marked this conversation as resolved.

if self.quoted {
let quoted = QuotedFormatter { text: paths.rel };
let quoted = QuotedFormatter { text: rel };
fmt::Display::fmt(&quoted, f)
} else {
// `fmt::Formatter` only accepts `&str`, so non-UTF-8 path bytes are emitted lossily
// (U+FFFD) via `bstr::BStr`'s Display. Both current callers pass
// `quoted = true`, so this branch is unreached today; if a future
// caller needs byte-exact output it must use an `io::Write` sink.
write!(f, "{}", bstr::BStr::new(paths.rel))
write!(f, "{}", bstr::BStr::new(rel))
}
}
}
Expand All @@ -92,10 +95,6 @@ pub struct Entry {
// bun_collections::HashMap currently ignores the context/load-factor
// type params (backed by std HashMap); identity hashing is a TODO(perf).

fn normalize(path: &[u8]) -> &[u8] {
FileSystem::instance().normalize(path)
}

pub(crate) fn hash(normalized_path: &[u8]) -> u64 {
bun_wyhash::hash(normalized_path)
}
Expand Down Expand Up @@ -177,84 +176,80 @@ struct Paths<'a> {
rel: &'a [u8],
}

/// Returns `None` when the `package.json` path does not fit `joined`.
fn normalize_package_json_path<'a>(
global_or_relative: GlobalOrRelative<'_>,
joined: &'a mut PathBuffer,
non_normalized_path: &[u8],
) -> Paths<'a> {
let abs: &[u8];

) -> Option<Paths<'a>> {
let mut normalize_spill = Vec::new();
// We consider it valid if there is a package.json in the folder
let normalized: &[u8] = if non_normalized_path.len() == 1 && non_normalized_path[0] == b'.' {
let normalized: &[u8] = if non_normalized_path == b"." {
non_normalized_path
} else if bun_paths::is_absolute(non_normalized_path) {
strings::trim_right(non_normalized_path, SEP_STR.as_bytes())
} else {
strings::trim_right(normalize(non_normalized_path), SEP_STR.as_bytes())
strings::trim_right(
resolve_path::normalize_string_spill::<true, bun_paths::platform::Auto>(
&mut normalize_spill,
non_normalized_path,
),
SEP_STR.as_bytes(),
)
};

const PACKAGE_JSON_LEN: usize = "/package.json".len();

let rel: &[u8] = if strings::starts_with_char(normalized, b'.') {
let mut tempcat = PathBuffer::uninit();

tempcat[..normalized.len()].copy_from_slice(normalized);
tempcat[normalized.len()] = SEP;
tempcat[normalized.len() + 1..normalized.len() + PACKAGE_JSON_LEN]
.copy_from_slice(b"package.json");
let parts: [&[u8]; 2] = [
FileSystem::instance().top_level_dir(),
&tempcat[0..normalized.len() + PACKAGE_JSON_LEN],
];
abs = FileSystem::instance().abs_buf(&parts, joined);
FileSystem::instance().relative(
FileSystem::instance().top_level_dir(),
&abs[0..abs.len() - PACKAGE_JSON_LEN],
)
// The last byte of `joined` is reserved for the NUL terminator.
let capacity = joined.len() - 1;

let abs_len = if strings::starts_with_char(normalized, b'.') {
let parts: [&[u8]; 2] = [normalized, b"package.json"];
FileSystem::instance()
.abs_buf_checked(&parts, &mut joined[..capacity])?
.len()
} else {
let joined_len = joined.len();
let mut remain: &mut [u8] = &mut joined[..];
match &global_or_relative {
GlobalOrRelative::Global(path) | GlobalOrRelative::CacheFolder(path) => {
if !path.is_empty() {
let offset = path
.len()
.saturating_sub((path[path.len().saturating_sub(1)] == SEP) as usize);
if offset > 0 {
remain[0..offset].copy_from_slice(&path[0..offset]);
}
remain = &mut remain[offset..];
if !normalized.is_empty() {
if (path[path.len() - 1] != SEP) && (normalized[0] != SEP) {
remain[0] = SEP;
remain = &mut remain[1..];
}
}
}
let (prefix, needs_sep): (&[u8], bool) = match global_or_relative {
GlobalOrRelative::Global(path) | GlobalOrRelative::CacheFolder(path)
if !path.is_empty() =>
{
let ends_with_sep = path[path.len() - 1] == SEP;
(
&path[..path.len() - ends_with_sep as usize],
!normalized.is_empty() && !ends_with_sep && normalized[0] != SEP,
)
}
GlobalOrRelative::Relative(_) => {}
_ => (b"", false),
};
let abs_len = prefix.len() + needs_sep as usize + normalized.len() + PACKAGE_JSON_LEN;
if abs_len > capacity {
return None;
}
remain[..normalized.len()].copy_from_slice(normalized);
remain[normalized.len()] = SEP;
remain[normalized.len() + 1..normalized.len() + PACKAGE_JSON_LEN]
.copy_from_slice(b"package.json");
let remain_after = remain.len() - (normalized.len() + PACKAGE_JSON_LEN);
// Compute abs len from remaining capacity.
let abs_len = joined_len - remain_after;
abs = &joined[0..abs_len];
// We store the folder name without package.json
FileSystem::instance().relative(
FileSystem::instance().top_level_dir(),
&abs[0..abs.len() - PACKAGE_JSON_LEN],
)

let mut len = prefix.len();
joined[..len].copy_from_slice(prefix);
if needs_sep {
joined[len] = SEP;
len += 1;
}
joined[len..len + normalized.len()].copy_from_slice(normalized);
len += normalized.len();
joined[len] = SEP;
joined[len + 1..len + PACKAGE_JSON_LEN].copy_from_slice(b"package.json");
abs_len
};
let abs_len = abs.len();

// We store the folder name without package.json
let rel = FileSystem::instance().relative(
FileSystem::instance().top_level_dir(),
&joined[..abs_len - PACKAGE_JSON_LEN],
);
joined[abs_len] = 0;

Paths {
Some(Paths {
abs: ZStr::from_buf(joined, abs_len),
rel,
}
})
}

fn read_package_json_from_disk<R: FolderResolverImpl>(
Expand Down Expand Up @@ -386,7 +381,11 @@ pub(crate) fn get_or_put(
let mut joined = PathBuffer::uninit();
#[cfg(windows)]
let mut rel_buf = PathBuffer::uninit();
let paths = normalize_package_json_path(global_or_relative, &mut joined, non_normalized_path);
let Some(paths) =
normalize_package_json_path(global_or_relative, &mut joined, non_normalized_path)
else {
return FolderResolution::Err(crate::Error::Sys(bun_errno::SystemErrno::ENAMETOOLONG));
};

#[cfg(not(windows))]
let abs = paths.abs;
Expand Down Expand Up @@ -435,10 +434,10 @@ pub(crate) fn get_or_put(

let result: crate::Result<LockfilePackage> = match global_or_relative {
GlobalOrRelative::Global(_) => 'global: {
let mut path = PathBuffer::uninit();
path[..non_normalized_path.len()].copy_from_slice(non_normalized_path);
// `non_normalized_path` may alias the lockfile string buffer, which grows below.
let folder_path: Box<[u8]> = Box::from(non_normalized_path);
let mut resolver: SymlinkResolver = NewResolver {
folder_path: &path[0..non_normalized_path.len()],
folder_path: &folder_path,
};
break 'global read_package_json_from_disk(
manager,
Expand Down
8 changes: 0 additions & 8 deletions src/resolver/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -428,14 +428,6 @@ pub mod fs {
}
}

/// Normalizes `str` in the shared scratch space, returning the input
/// unchanged when already normalized.
#[inline]
pub fn normalize<'a>(&self, str: &'a [u8]) -> &'a [u8] {
use bun_paths::resolve_path::{normalize_string, platform};
normalize_string::<true, platform::Auto>(str)
}

/// The process-global directory-name interning store.
#[inline]
pub fn dirname_store(&self) -> &'static DirnameStore {
Expand Down
35 changes: 34 additions & 1 deletion test/cli/install/bun-install-patch.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { $ } from "bun";
import { describe, expect, it, setDefaultTimeout, test } from "bun:test";
import { rmSync } from "fs";
import { bunEnv, bunExe, normalizeBunSnapshot as normalizeBunSnapshot_, tempDir } from "harness";
import { bunEnv, bunExe, isWindows, normalizeBunSnapshot as normalizeBunSnapshot_, tempDir } from "harness";
import { join } from "path";

const normalizeBunSnapshot = (str: string) => {
Expand Down Expand Up @@ -1119,3 +1119,36 @@ describe("patchedDependencies contents_hash", () => {
expect({ hasB: mB.includes("TAIL_BBBB"), hasA: mB.includes("TAIL_AAAA") }).toEqual({ hasB: true, hasA: false });
});
});

describe("patchedDependencies path longer than the path buffer", () => {
// Hashing a patch joins its path onto the project directory in a path buffer
// (4096 bytes on Linux, 1024 on macOS, ~96 KiB on Windows). The join used to
// write past the buffer for a path that did not fit, aborting the install.
const longName = Buffer.alloc(100_000, "p").toString();

test("install reports the path instead of crashing", async () => {
using dir = tempDir("patch-path-too-long", {
"package.json": JSON.stringify({
name: "patch-path-too-long",
patchedDependencies: { "is-odd@3.0.1": `patches/${longName}.patch` },
}),
});

await using proc = Bun.spawn({
cmd: [bunExe(), "install"],
cwd: String(dir),
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});
const [, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);

// The path is handed to the OS as is. POSIX rejects anything longer than
// PATH_MAX with ENAMETOOLONG; Windows may report it as missing instead.
if (!isWindows) {
expect(stderr).toContain("error: failed to read patch file: ENAMETOOLONG: ");
}
expect(stderr).toContain(`${longName}.patch`);
expect(exitCode).toBe(1);
});
});
Loading
Loading