Skip to content
97 changes: 84 additions & 13 deletions src/paths/resolve_path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,59 @@ use bun_core::{ZStr, strings};
// SAFETY invariant: each buffer has at most one live mutable borrow per thread;
// callers must not re-enter the accessor while a previous borrow is alive.
thread_local! {
static PARSER_JOIN_INPUT_BUFFER: UnsafeCell<[u8; 4096]> = const { UnsafeCell::new([0u8; 4096]) };
static PARSER_JOIN_INPUT_BUFFER: LazyJoinBuf = const { LazyJoinBuf::NEW };
static PARSER_BUFFER: UnsafeCell<[u8; 1024]> = const { UnsafeCell::new([0u8; 1024]) };
}

/// Output capacity of [`join_abs_string`] / [`join`]. Must hold any valid host
/// path: on Windows `MAX_PATH_BYTES` is 98 302, so a hard-coded 4096 rejects
/// paths in `[4096, MAX_PATH_BYTES)` with a slice-index panic. On POSIX
/// `MAX_PATH_BYTES <= 4096`; keep 4096 there so callers that pre-date per-part
/// length validation see no behaviour change.
Comment thread
robobun marked this conversation as resolved.
Outdated
pub(crate) const TL_JOIN_BUF_LEN: usize = if MAX_PATH_BYTES > 4096 {
MAX_PATH_BYTES
} else {
4096
};

/// Lazily heap-backed `[u8; TL_JOIN_BUF_LEN]` thread-local. 8 bytes in `.tls`
/// instead of `TL_JOIN_BUF_LEN` zeros (PE/COFF has no TLS-BSS; see
/// [`LazyPathBuf`]).
Comment thread
robobun marked this conversation as resolved.
struct LazyJoinBuf(core::cell::Cell<*mut [u8; TL_JOIN_BUF_LEN]>);

impl LazyJoinBuf {
const NEW: Self = Self(core::cell::Cell::new(core::ptr::null_mut()));

/// Borrow the thread-local buffer, allocating on first use. Same
/// single-live-borrow-per-thread contract as [`tl_buf_mut`].
Comment thread
robobun marked this conversation as resolved.
Outdated
#[inline]
fn get(&self) -> &'static mut [u8; TL_JOIN_BUF_LEN] {
let mut p = self.0.get();
if p.is_null() {
// SAFETY: `new_zeroed` writes every byte to `0`, a valid `u8`, so
// the value is fully initialized before `assume_init`.
p = bun_core::heap::into_raw(unsafe {
Box::<[u8; TL_JOIN_BUF_LEN]>::new_zeroed().assume_init()
});
self.0.set(p);
}
// SAFETY: non-null after the init branch; thread-local ⇒ sole accessor.
unsafe { &mut *p }
}
}

impl Drop for LazyJoinBuf {
fn drop(&mut self) {
let p = self.0.get();
if !p.is_null() {
// SAFETY: `p` came from `heap::into_raw` in `get()`; sole accessor.
unsafe { drop(bun_core::heap::take(p)) };
}
}
}

/// Project `&'static mut` into a thread-local `UnsafeCell<[u8; N]>` scratch
/// buffer. One `unsafe` site for all `PARSER_BUFFER` / `PARSER_JOIN_INPUT_BUFFER`
/// / `JOIN_BUF` accessors (nonnull-asref reduction: 6 sites → 1).
/// buffer (`PARSER_BUFFER`).
///
/// The `'static` output lifetime is the honest contract: the buffer is
/// thread-local storage that lives for the thread's lifetime, and the returned
Expand Down Expand Up @@ -1354,7 +1400,7 @@ pub fn join_abs<'a, P: PlatformT>(cwd: &'a [u8], part: &[u8]) -> &'a [u8] {
// result borrows the thread-local buffer ('static) OR returns `cwd`
// directly when `parts.is_empty()`. Return tied to `cwd`'s lifetime ('static: 'a).
pub fn join_abs_string<'a, P: PlatformT>(cwd: &'a [u8], parts: &[&[u8]]) -> &'a [u8] {
PARSER_JOIN_INPUT_BUFFER.with(|b| join_abs_string_buf::<P>(cwd, tl_buf_mut(b), parts))
PARSER_JOIN_INPUT_BUFFER.with(|b| join_abs_string_buf::<P>(cwd, b.get(), parts))
}

/// Convert parts of potentially invalid file paths into a single valid filpeath
Expand All @@ -1363,22 +1409,19 @@ pub fn join_abs_string<'a, P: PlatformT>(cwd: &'a [u8], parts: &[&[u8]]) -> &'a
///
/// Returned path is stored in a temporary buffer. It must be copied if it needs to be stored.
pub fn join_abs_string_z<'a, P: PlatformT>(cwd: &'a [u8], parts: &[&[u8]]) -> &'a ZStr {
PARSER_JOIN_INPUT_BUFFER.with(|b| join_abs_string_buf_z::<P>(cwd, tl_buf_mut(b), parts))
PARSER_JOIN_INPUT_BUFFER.with(|b| join_abs_string_buf_z::<P>(cwd, b.get(), parts))
}

const JOIN_BUF_LEN: usize = 4096;

thread_local! {
pub(crate) static JOIN_BUF: UnsafeCell<[u8; JOIN_BUF_LEN]> =
const { UnsafeCell::new([0u8; JOIN_BUF_LEN]) };
static JOIN_BUF: LazyJoinBuf = const { LazyJoinBuf::NEW };
}

pub fn join<P: PlatformT>(parts: &[&[u8]]) -> &'static [u8] {
JOIN_BUF.with(|b| join_string_buf::<P>(tl_buf_mut(b), parts))
JOIN_BUF.with(|b| join_string_buf::<P>(b.get(), parts))
}

pub fn join_z<P: PlatformT>(parts: &[&[u8]]) -> &'static ZStr {
JOIN_BUF.with(|b| join_z_buf::<P>(tl_buf_mut(b), parts))
JOIN_BUF.with(|b| join_z_buf::<P>(b.get(), parts))
}

#[inline]
Expand Down Expand Up @@ -1409,7 +1452,7 @@ pub fn join_z_buf_spill<'a, P: PlatformT>(
/// `spill` (grown as needed). `spill` is untouched in the common case.
pub fn join_spill<'a, P: PlatformT>(spill: &'a mut Vec<u8>, parts: &[&[u8]]) -> &'a [u8] {
let needed = join_needed(parts);
if needed <= JOIN_BUF_LEN {
if needed <= TL_JOIN_BUF_LEN {
return join::<P>(parts);
}
if spill.len() < needed {
Expand All @@ -1422,7 +1465,7 @@ pub fn join_spill<'a, P: PlatformT>(spill: &'a mut Vec<u8>, parts: &[&[u8]]) ->
/// `spill` (grown as needed). `spill` is untouched in the common case.
pub fn join_z_spill<'a, P: PlatformT>(spill: &'a mut Vec<u8>, parts: &[&[u8]]) -> &'a ZStr {
let needed = join_needed(parts);
if needed <= JOIN_BUF_LEN {
if needed <= TL_JOIN_BUF_LEN {
return join_z::<P>(parts);
}
if spill.len() < needed {
Expand Down Expand Up @@ -2434,3 +2477,31 @@ pub fn posix_to_platform_in_place<T: PathChar>(path_buffer: &mut [T]) {
// `PathChar` is now canonical at `crate::path_char`; re-export for callers
// that still path through `resolve_path::PathChar`.
pub use crate::PathChar;

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn tl_join_buf_len_holds_max_path_bytes() {
assert!(TL_JOIN_BUF_LEN >= MAX_PATH_BYTES);
assert!(TL_JOIN_BUF_LEN >= 4096);
}

#[test]
fn join_abs_string_accepts_path_buffer_length() {
// A part just under MAX_PATH_BYTES must round-trip through the
// thread-local output buffer without panicking. This is the Windows
// `[4096, MAX_PATH_BYTES)` window: valid host path, previously aborted.
Comment thread
robobun marked this conversation as resolved.
Outdated
let long = vec![b'a'; MAX_PATH_BYTES - 8];
let cwd: &[u8] = if cfg!(windows) { b"C:\\d" } else { b"/d" };
let abs: &[u8] = if cfg!(windows) { b"C:\\" } else { b"/" };
let r = join_abs_string::<platform::Auto>(cwd, &[abs, &long]);
assert_eq!(r.len(), abs.len() + long.len());
let r = join_abs_string_z::<platform::Auto>(cwd, &[abs, &long]);
assert_eq!(r.as_bytes().len(), abs.len() + long.len());
// And the non-absolute `join` sibling backed by `JOIN_BUF`.
let r = join::<platform::Auto>(&[abs, &long]);
assert_eq!(r.len(), abs.len() + long.len());
}
}
60 changes: 33 additions & 27 deletions src/runtime/ffi/ffi_body.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1493,36 +1493,42 @@

let dylib: bun_sys::DynLib = 'brk: {
// First try using the name directly
match bun_sys::DynLib::open(name) {
let mut last_err = match bun_sys::DynLib::open(name) {
Ok(d) => break 'brk d,
Err(_) => {
let backup_name = Fs::FileSystem::instance().abs(&[name]);
// if that fails, try resolving the filepath relative to the current working directory
match bun_sys::DynLib::open(backup_name) {
Ok(d) => break 'brk d,
Err(_) => {
// Then, if that fails, report an error with the library name and system error
let dlerror_msg = get_dl_error();

let mut msg = Vec::new();
write!(
&mut msg,
"Failed to open library \"{}\": {}",
BStr::new(name),
BStr::new(&dlerror_msg)
)
.ok();
let system_error = SystemError {
code: bun_core::String::clone_utf8(b"ERR_DLOPEN_FAILED").into(),
message: bun_core::String::clone_utf8(&msg).into(),
syscall: bun_core::String::clone_utf8(b"dlopen").into(),
..Default::default()
};
return system_error.to_error_instance(global);
}
}
Err(e) => e,
};
// if that fails, try resolving the filepath relative to the current working directory
if name.len() < bun_paths::MAX_PATH_BYTES {
let backup_name = Fs::FileSystem::instance().abs(&[name]);
match bun_sys::DynLib::open(backup_name) {
Ok(d) => break 'brk d,
Err(e) => last_err = e,
}
}
// Then, if that fails, report an error with the library name and
// system error. `DynLib::open` returns ENAMETOOLONG without calling
// the loader, so dlerror()/GetLastError() would be stale there.
Comment thread
robobun marked this conversation as resolved.
Outdated
let dlerror_msg = if last_err == bun_errno::SystemErrno::ENAMETOOLONG {
Box::<[u8]>::from(b"file name too long".as_slice())
} else {
get_dl_error()
};

Check warning on line 1515 in src/runtime/ffi/ffi_body.rs

View check run for this annotation

Claude / Claude Code Review

dlopen fallback overwrites last_err with ENAMETOOLONG, discarding valid dlerror() from first attempt

On macOS/BSD (`MAX_PATH_BYTES = 1024`, `TL_JOIN_BUF_LEN = 4096`), a short library name whose `abs()` result lands in `[1024, 4096)` makes the fallback `DynLib::open` short-circuit with `ENAMETOOLONG` without calling the loader — overwriting `last_err` and reporting `"file name too long"` for what the user typed as a ~20-byte name, discarding the still-fresh `dlerror()` from the first attempt. Pre-04a8f090 called `get_dl_error()` unconditionally and correctly returned that message, so this is a (
Comment thread
claude[bot] marked this conversation as resolved.
Outdated

let mut msg = Vec::new();
write!(
&mut msg,
"Failed to open library \"{}\": {}",
BStr::new(name),
BStr::new(&dlerror_msg)
)
.ok();
let system_error = SystemError {
code: bun_core::String::clone_utf8(b"ERR_DLOPEN_FAILED").into(),
message: bun_core::String::clone_utf8(&msg).into(),
syscall: bun_core::String::clone_utf8(b"dlopen").into(),
..Default::default()
};
return system_error.to_error_instance(global);
};

let mut size = symbols.values().len();
Expand Down
37 changes: 36 additions & 1 deletion test/js/bun/ffi/ffi-error-messages.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { dlopen, linkSymbols } from "bun:ffi";
import { describe, expect, test } from "bun:test";
import { isMusl } from "harness";
import { bunEnv, bunExe, isMusl } from "harness";

describe("FFI error messages", () => {
test("dlopen shows library name when library cannot be opened", () => {
Expand All @@ -20,6 +20,41 @@ describe("FFI error messages", () => {
}
});

// dlopen falls back to FileSystem::abs() when the direct open fails; abs()
// writes into a thread-local buffer that was 4096 bytes on every platform.
// A library path longer than that used to abort with
// panic: range end index 5003 out of range for slice of length 4095
// instead of reporting the ordinary dlopen error.
test.concurrent.each([5000, 100_000])(
"dlopen with a %d-byte library path reports an error instead of aborting",
async len => {
const prefix = process.platform === "win32" ? "C:\\" : "/";
await using proc = Bun.spawn({
cmd: [
bunExe(),
"-e",
`const { dlopen } = require("bun:ffi");` +
`const name = ${JSON.stringify(prefix)} + Buffer.alloc(${len}, "a").toString() + ".so";` +
`try { dlopen(name, { f: { args: [], returns: "void" } }); }` +
`catch (e) { const m = String(e.message);` +
` console.log("CAUGHT", e.code || e.name, m.slice(0, 30), "|", m.slice(-25)); }`,
],
env: bunEnv,
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect({ stdout: stdout.trim(), stderr }).toEqual({
stdout: expect.stringMatching(/^CAUGHT ERR_DLOPEN_FAILED Failed to open library/),
stderr: "",
});
// 100k exceeds every platform's MAX_PATH_BYTES: the fallback is skipped
// and the reported reason is the ENAMETOOLONG from DynLib::open, not a
// stale dlerror()/GetLastError() ("unknown error" / "error code 0").
if (len === 100_000) expect(stdout).toContain("file name too long");
expect(exitCode).toBe(0);
},
);

test("dlopen shows which symbol is missing when symbol not found", () => {
// Use appropriate system library for the platform
const libName =
Expand Down
31 changes: 31 additions & 0 deletions test/js/bun/http/bun-serve-html-manifest.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,37 @@ describe("Bun.serve HTML manifest", () => {
expect(out).toContain("SUCCESS: Manifest validation failed as expected");
});

it("rejects a manifest file path longer than the join buffer without aborting", async () => {
// Windows MAX_PATH_BYTES (98302) >> 4096, so a 5000-byte path passes the
// ENAMETOOLONG guard and reaches FileSystem::abs() whose output buffer was
// 4096 bytes: process used to abort with a slice-index panic. On POSIX
// MAX_PATH_BYTES <= 4096, so the guard rejects it with ENAMETOOLONG first.
await using proc = Bun.spawn({
cmd: [
bunExe(),
"-e",
`const long = (process.platform === "win32" ? "C:\\\\" : "/") + Buffer.alloc(5000, "a").toString();` +
`try {` +
` const s = Bun.serve({ port: 0, routes: { "/": {` +
` index: "./index.html",` +
` files: [{ input: "index.html", path: long, loader: "html", isEntry: true,` +
` headers: { etag: "x", "content-type": "text/html" } }],` +
` } } });` +
` s.stop();` +
` console.log("CAUGHT no-throw");` +
`} catch (e) { console.log("CAUGHT", e.code || e.name); }`,
],
env: bunEnv,
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
// ENAMETOOLONG on POSIX (the per-part guard fires); ERR_INVALID_ARG_TYPE
// on Windows (abs() succeeds; route setup then rejects the missing file).
if (!stdout.startsWith("CAUGHT")) console.error(stderr);
expect(stdout.trim()).toMatch(/^CAUGHT (ENAMETOOLONG|ERR_INVALID_ARG_TYPE)$/);
expect(exitCode).toBe(0);
});

it("serves manifest with proper headers", async () => {
await using dir = tempDir("serve-html-headers", {
"server.ts": `
Expand Down
Loading