diff --git a/src/paths/resolve_path.rs b/src/paths/resolve_path.rs index d2dfcb4a7b1f..e476b60d1ef1 100644 --- a/src/paths/resolve_path.rs +++ b/src/paths/resolve_path.rs @@ -13,21 +13,55 @@ 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; PARSER_JOIN_INPUT_BUFFER_LEN]> = - const { UnsafeCell::new([0u8; PARSER_JOIN_INPUT_BUFFER_LEN]) }; + static PARSER_JOIN_INPUT_BUFFER: LazyJoinBuf = const { LazyJoinBuf::NEW }; static PARSER_BUFFER: UnsafeCell<[u8; PARSER_BUFFER_LEN]> = const { UnsafeCell::new([0u8; PARSER_BUFFER_LEN]) }; } -/// Output capacity of [`join_abs_string`] / [`join_abs_string_z`]. -const PARSER_JOIN_INPUT_BUFFER_LEN: usize = 4096; - /// Output capacity of [`normalize_string`]. const PARSER_BUFFER_LEN: usize = 1024; +/// Output capacity of [`join_abs_string`] / [`join`]: at least `MAX_PATH_BYTES` +/// (98 302 on Windows) so any valid host path fits; floored at 4096 on POSIX. +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`]). +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())); + + /// Same single-live-borrow-per-thread contract as [`tl_buf_mut`]. + #[inline] + fn get(&self) -> &'static mut [u8; TL_JOIN_BUF_LEN] { + let mut p = self.0.get(); + if p.is_null() { + p = bun_core::heap::into_raw(bun_core::boxed_zeroed::<[u8; TL_JOIN_BUF_LEN]>()); + self.0.set(p); + } + // SAFETY: non-null after init; 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 @@ -1381,7 +1415,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::

(cwd, tl_buf_mut(b), parts)) + PARSER_JOIN_INPUT_BUFFER.with(|b| join_abs_string_buf::

(cwd, b.get(), parts)) } /// [`join_abs_string`] (thread-local buffer) when the result fits, otherwise @@ -1393,7 +1427,7 @@ pub fn join_abs_string_spill<'a, P: PlatformT>( ) -> &'a [u8] { debug_assert!(!matches!(P::P, Platform::Nt)); let needed = join_abs_needed(cwd.len(), parts); - if needed <= PARSER_JOIN_INPUT_BUFFER_LEN { + if needed <= TL_JOIN_BUF_LEN { return join_abs_string::

(cwd, parts); } if spill.len() < needed { @@ -1408,22 +1442,19 @@ pub fn join_abs_string_spill<'a, P: PlatformT>( /// /// 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::

(cwd, tl_buf_mut(b), parts)) + PARSER_JOIN_INPUT_BUFFER.with(|b| join_abs_string_buf_z::

(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(parts: &[&[u8]]) -> &'static [u8] { - JOIN_BUF.with(|b| join_string_buf::

(tl_buf_mut(b), parts)) + JOIN_BUF.with(|b| join_string_buf::

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

(tl_buf_mut(b), parts)) + JOIN_BUF.with(|b| join_z_buf::

(b.get(), parts)) } #[inline] @@ -1454,7 +1485,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, parts: &[&[u8]]) -> &'a [u8] { let needed = join_needed(parts); - if needed <= JOIN_BUF_LEN { + if needed <= TL_JOIN_BUF_LEN { return join::

(parts); } if spill.len() < needed { @@ -1467,7 +1498,7 @@ pub fn join_spill<'a, P: PlatformT>(spill: &'a mut Vec, 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, parts: &[&[u8]]) -> &'a ZStr { let needed = join_needed(parts); - if needed <= JOIN_BUF_LEN { + if needed <= TL_JOIN_BUF_LEN { return join_z::

(parts); } if spill.len() < needed { @@ -2583,7 +2614,7 @@ mod tests { #[test] fn join_abs_string_spill_spills_a_part_longer_than_the_thread_local_buffer() { - let name = vec![b'a'; PARSER_JOIN_INPUT_BUFFER_LEN + 1]; + let name = vec![b'a'; TL_JOIN_BUF_LEN + 1]; let mut expected = b"/work/".to_vec(); expected.extend_from_slice(&name); @@ -2596,7 +2627,7 @@ mod tests { #[test] fn join_abs_string_spill_spills_an_absolute_part_and_a_long_cwd_alike() { let mut abs = b"/".to_vec(); - abs.resize(PARSER_JOIN_INPUT_BUFFER_LEN * 2, b'a'); + abs.resize(TL_JOIN_BUF_LEN * 2, b'a'); let mut spill = Vec::new(); assert_eq!( join_abs_string_spill::(b"/", &mut spill, &[&abs]), @@ -2604,7 +2635,7 @@ mod tests { ); let mut cwd = b"/".to_vec(); - cwd.resize(PARSER_JOIN_INPUT_BUFFER_LEN, b'c'); + cwd.resize(TL_JOIN_BUF_LEN, b'c'); let mut expected = cwd.clone(); expected.extend_from_slice(b"/x"); let mut spill = Vec::new(); @@ -2618,7 +2649,7 @@ mod tests { fn join_abs_string_spill_normalizes_a_long_part_that_collapses() { // `sub/../` repeated past the buffer size resolves back to the cwd. let mut part = Vec::new(); - while part.len() <= PARSER_JOIN_INPUT_BUFFER_LEN { + while part.len() <= TL_JOIN_BUF_LEN { part.extend_from_slice(b"sub/../"); } part.extend_from_slice(b"sub"); @@ -2704,4 +2735,25 @@ mod tests { &expected[..] ); } + + #[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 valid host path just under MAX_PATH_BYTES round-trips through the TL buffer. + 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::(cwd, &[abs, &long]); + assert_eq!(r.len(), abs.len() + long.len()); + let r = join_abs_string_z::(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::(&[abs, &long]); + assert_eq!(r.len(), abs.len() + long.len()); + } } diff --git a/src/runtime/ffi/ffi_body.rs b/src/runtime/ffi/ffi_body.rs index b2ff2174976b..2841227d2e89 100644 --- a/src/runtime/ffi/ffi_body.rs +++ b/src/runtime/ffi/ffi_body.rs @@ -1469,35 +1469,39 @@ impl FFI { let dylib: bun_sys::DynLib = 'brk: { // First try using the name directly - 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(); - let _ = write!( - &mut msg, - "Failed to open library \"{}\": {}", - BStr::new(name), - BStr::new(&dlerror_msg) - ); - let system_error = SystemError { - code: bun_core::String::clone_utf8(b"ERR_DLOPEN_FAILED"), - message: bun_core::String::clone_utf8(&msg), - syscall: bun_core::String::clone_utf8(b"dlopen"), - ..Default::default() - }; - return Ok(system_error.to_error_instance(global)); - } - } - } + if let Ok(d) = bun_sys::DynLib::open(name) { + break 'brk d; + } + // if that fails, try resolving the filepath relative to the current working directory + let mut backup_buf = bun_paths::path_buffer_pool::get(); + if let Some(backup_name) = + Fs::FileSystem::instance().abs_buf_checked(&[name], &mut backup_buf[..]) + && let Ok(d) = bun_sys::DynLib::open(backup_name) + { + break 'brk d; } + // `DynLib::open` short-circuits ENAMETOOLONG without calling the + // loader, so dlerror()/GetLastError() is stale iff `name` was too long. + let dlerror_msg = if name.len() >= bun_paths::MAX_PATH_BYTES { + Box::<[u8]>::from(b"file name too long".as_slice()) + } else { + get_dl_error() + }; + + let mut msg = Vec::new(); + let _ = write!( + &mut msg, + "Failed to open library \"{}\": {}", + BStr::new(name), + BStr::new(&dlerror_msg) + ); + let system_error = SystemError { + code: bun_core::String::clone_utf8(b"ERR_DLOPEN_FAILED"), + message: bun_core::String::clone_utf8(&msg), + syscall: bun_core::String::clone_utf8(b"dlopen"), + ..Default::default() + }; + return Ok(system_error.to_error_instance(global)); }; let mut size = symbols.values().len(); diff --git a/src/runtime/node/types.rs b/src/runtime/node/types.rs index 7ce362789056..9f423aca5f47 100644 --- a/src/runtime/node/types.rs +++ b/src/runtime/node/types.rs @@ -1245,12 +1245,17 @@ impl Valid { if path.len() < MAX_PATH_BYTES { return None; } + Some(Self::name_too_long(path)) + } + + /// ENAMETOOLONG for `path`, with no length check. + pub(crate) fn name_too_long(path: &[u8]) -> bun_sys::SystemError { let mut system_error = bun_sys::Error::from_code(bun_sys::E::ENAMETOOLONG, bun_sys::Tag::open) .with_path(path) .to_system_error(); system_error.syscall = bun_core::String::DEAD; - Some(system_error) + system_error } /// Sync bindings throw; async ones get it as `arguments.deferred_error` and a placeholder path. diff --git a/src/runtime/server/server_body.rs b/src/runtime/server/server_body.rs index 600bf58a69c0..9e1f13d84c6d 100644 --- a/src/runtime/server/server_body.rs +++ b/src/runtime/server/server_body.rs @@ -612,8 +612,25 @@ impl AnyRoute { FileSystem::instance().top_level_dir }; - let abs_path = FileSystem::instance().abs(&[path_slice]); - let mut relative_path = FileSystem::instance().relative(cwd, abs_path); + // Same bound as `Valid::path_too_long`: the joined path needs room for its NUL. + let mut abs_buf = paths::path_buffer_pool::get(); + let abs_path = match FileSystem::instance().abs_buf_checked(&[path_slice], &mut abs_buf[..]) + { + Some(abs_path) if abs_path.len() < paths::MAX_PATH_BYTES => abs_path, + _ => { + use bun_sys_jsc::SystemErrorJsc as _; + let err = crate::node::types::Valid::name_too_long(path_slice); + return Err(init_ctx + .global + .throw_value(err.to_error_instance(init_ctx.global))); + } + }; + // Worst case: one "/.." per `cwd` segment, then a separator and `abs_path`. + let mut relative_buf = vec![0u8; abs_path.len() + 3 * cwd.len() + 4]; + let mut relative_path = paths::resolve_path::relative_platform_buf::< + paths::resolve_path::platform::Auto, + false, + >(&mut relative_buf, cwd, abs_path); if relative_path.starts_with(b"./") { relative_path = &relative_path[2..]; diff --git a/test/js/bun/ffi/ffi-error-messages.test.ts b/test/js/bun/ffi/ffi-error-messages.test.ts index d886d6e0de68..dfa60861a506 100644 --- a/test/js/bun/ffi/ffi-error-messages.test.ts +++ b/test/js/bun/ffi/ffi-error-messages.test.ts @@ -1,6 +1,6 @@ import { CString, dlopen, linkSymbols, ptr, toArrayBuffer, toBuffer } from "bun:ffi"; import { describe, expect, test } from "bun:test"; -import { isMusl } from "harness"; +import { bunEnv, bunExe, isMusl } from "harness"; // Not `toThrow()`: it also accepts an Error that the function returns, which is // what `toBuffer()` and `toArrayBuffer()` did with their TypeError before they @@ -103,6 +103,46 @@ 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. The "relative" row joins + // against cwd, so the overflow point is roughly MAX_PATH_BYTES - cwd.len(). + describe.each([ + ["absolute", 5000], + ["absolute", 100_000], + ["relative", 4090], + ["relative", 100_000], + ] as const)("dlopen with a %s %d-byte library path", (kind, len) => { + test.concurrent("reports an error instead of aborting", async () => { + const prefix = kind === "relative" ? "" : process.platform === "win32" ? "C:\\" : "/"; + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + `import { dlopen } from "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 = diff --git a/test/js/bun/http/bun-serve-html-manifest.test.ts b/test/js/bun/http/bun-serve-html-manifest.test.ts index 8013039cb0dc..b3f7b7e34f4b 100644 --- a/test/js/bun/http/bun-serve-html-manifest.test.ts +++ b/test/js/bun/http/bun-serve-html-manifest.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "bun:test"; -import { bunEnv, bunExe, tempDir } from "harness"; +import { bunEnv, bunExe, isWindows, tempDir } from "harness"; import { join } from "node:path"; import { StringDecoder } from "node:string_decoder"; @@ -266,6 +266,81 @@ describe("Bun.serve HTML manifest", () => { expect(out).toContain("SUCCESS: Manifest validation failed as expected"); }); + const maxPathBytes = isWindows ? 98302 : process.platform === "darwin" ? 1024 : 4096; + + // Spawns bun with a manifest whose one file has `path` and prints the + // error code the Bun.serve call throws, or "no-throw". + async function serveWithManifestPath(pathExpr: string, cwd?: string) { + await using proc = Bun.spawn({ + cwd, + cmd: [ + bunExe(), + "-e", + `const long = ${pathExpr};` + + `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]); + if (!stdout.startsWith("CAUGHT")) console.error(stderr); + return { stdout: stdout.trim(), exitCode }; + } + + it("rejects an absolute 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. + const { stdout, exitCode } = await serveWithManifestPath( + `(process.platform === "win32" ? "C:\\\\" : "/") + Buffer.alloc(5000, "a").toString()`, + ); + // On Windows abs() succeeds and route setup then rejects the missing file. + expect(stdout).toBe(isWindows ? "CAUGHT ERR_INVALID_ARG_TYPE" : "CAUGHT ENAMETOOLONG"); + expect(exitCode).toBe(0); + }); + + it("rejects a relative manifest file path that overflows the join buffer once joined onto the cwd", async () => { + // One byte under MAX_PATH_BYTES passes the per-part guard, but cwd + "/" + + // part is longer than any host path, and on Linux and Windows longer than + // the join buffer: process used to abort with a slice-index panic. + const { stdout, exitCode } = await serveWithManifestPath(`Buffer.alloc(${maxPathBytes - 1}, "a").toString()`); + expect(stdout).toBe("CAUGHT ENAMETOOLONG"); + expect(exitCode).toBe(0); + }); + + it("rejects a relative manifest file path that joins onto the cwd to exactly MAX_PATH_BYTES", async () => { + // The joined path fills a PathBuffer with no room for its NUL. The OS + // rejects it, and on Windows relative() used to overflow its own buffer. + const { stdout, exitCode } = await serveWithManifestPath( + `Buffer.alloc(${maxPathBytes} - process.cwd().length - 1, "a").toString()`, + ); + expect(stdout).toBe("CAUGHT ENAMETOOLONG"); + expect(exitCode).toBe(0); + }); + + it("rejects an absolute manifest file path outside the cwd that overflows the relative buffer", async () => { + // The path fits the join buffer, but relative(cwd, path) prepends one + // "/.." per cwd segment, and that output did not fit its own buffer: + // process used to abort with a slice-index panic. + using dir = tempDir("serve-html-deep", {}); + const { stdout, exitCode } = await serveWithManifestPath( + `(process.platform === "win32" ? "C:\\\\" : "/") + Buffer.alloc(${maxPathBytes - 4}, "a").toString()`, + String(dir), + ); + // abs() succeeds, so route setup then rejects the missing file. + expect(stdout).toBe("CAUGHT ERR_INVALID_ARG_TYPE"); + expect(exitCode).toBe(0); + }); + it("serves manifest with proper headers", async () => { await using dir = tempDir("serve-html-headers", { "server.ts": `