diff --git a/src/bundler/linker.rs b/src/bundler/linker.rs index 75d347b85062..cfaace9468c6 100644 --- a/src/bundler/linker.rs +++ b/src/bundler/linker.rs @@ -610,6 +610,7 @@ impl Linker { origin: &URL<'_>, import_path_format: ImportPathFormat, ) -> crate::Result> { + // `source_path` may be an unbounded `onResolve` result: no thread-local `relative`. match import_path_format { ImportPathFormat::AbsolutePath => { if namespace == b"node" { @@ -617,20 +618,19 @@ impl Linker { } if namespace == b"bun" || namespace == b"file" || namespace.is_empty() { - // `linker.fs.relative` is a thin wrapper over - // `bun.path.relative`; the inline `bun_resolver::fs` - // module doesn't expose it yet, so call the path layer - // directly. The threadlocal-buffer result must be - // dup'd to outlive this call. - let relative_name = - dupe(bun_paths::resolve_path::relative(source_dir, source_path)); + let relative_name = dupe(&bun_paths::resolve_path::relative_alloc( + source_dir, + source_path, + )?); Ok(PFs::Path::init_with_pretty(source_path, relative_name)) } else { Ok(PFs::Path::init_with_namespace(source_path, namespace)) } } ImportPathFormat::Relative => { - let relative_name = bun_paths::resolve_path::relative(source_dir, source_path); + let relative_path = + bun_paths::resolve_path::relative_alloc(source_dir, source_path)?; + let relative_name: &[u8] = &relative_path; let text: &'static [u8]; let pretty: &'static [u8]; @@ -685,8 +685,9 @@ impl Linker { } let top_level_dir = self.fs().top_level_dir; - let mut base: &[u8] = - bun_paths::resolve_path::relative(top_level_dir, source_path); + let relative_path = + bun_paths::resolve_path::relative_alloc(top_level_dir, source_path)?; + let mut base: &[u8] = &relative_path; if let Some(dot) = strings::last_index_of_char(base, b'.') { base = &base[0..dot]; } diff --git a/src/jsc/resolver_jsc.rs b/src/jsc/resolver_jsc.rs index 1a0a7f5e7617..a0684e2973e8 100644 --- a/src/jsc/resolver_jsc.rs +++ b/src/jsc/resolver_jsc.rs @@ -48,10 +48,13 @@ extern "C" fn node_module_paths_js_value( sliced.slice() }; let mut buf = bun_paths::path_buffer_pool::get(); + // Like Node's `_nodeModulePaths`, pure string manipulation: no length limit. + let mut spill = Vec::new(); - let mut full_path: &[u8] = resolve_path::join_abs_string_buf::( + let mut full_path: &[u8] = resolve_path::join_abs_string_buf_spill::( bun_paths::fs::FileSystem::instance().top_level_dir(), &mut **buf, + &mut spill, &[base_path], ); let root_index: usize = { diff --git a/src/paths/resolve_path.rs b/src/paths/resolve_path.rs index f6dfea1a045d..16e0d06c8fbe 100644 --- a/src/paths/resolve_path.rs +++ b/src/paths/resolve_path.rs @@ -658,7 +658,67 @@ pub fn relative_platform_buf<'a, P: PlatformT, const ALWAYS_COPY: bool>( // two `&mut` borrows below are disjoint. let relative_from_buf = RELATIVE_FROM_BUF.with(lazy_path_buf); let relative_to_buf = RELATIVE_TO_BUF.with(lazy_path_buf); + relative_platform_in::( + &mut relative_from_buf[..], + &mut relative_to_buf[..], + buf, + from, + to, + ) +} + +/// Scratch [`relative_platform_in`] needs for one input: a leading separator +/// plus the input normalized (which can grow by a byte, see +/// [`normalize_string_spill`]), or that joined onto the cwd when it is relative. +fn relative_scratch_needed(path: &[u8]) -> usize { + if P::P.is_absolute(path) { + path.len() + 2 + } else { + join_abs_needed(Fs::FileSystem::instance().top_level_dir().len(), &[path]) + 1 + } +} + +/// Result bound of [`relative_to_common_path`]: each component of `from` (two +/// bytes or more) becomes at most a three-byte `/..`, then a separator and `to`. +fn relative_out_needed(scratch: usize) -> usize { + (scratch + scratch / 2) + 1 + scratch +} +/// [`relative`] for inputs of any length: the thread-local `PathBuffer`s bound +/// `from`, `to` and the `../` chain alike, so when any of the three might not +/// fit, the same code runs in heap buffers sized to the inputs. +pub fn relative_alloc(from: &[u8], to: &[u8]) -> Result, bun_alloc::AllocError> { + // Either input may be normalized into either scratch buffer. + let scratch = relative_scratch_needed::(from) + .max(relative_scratch_needed::(to)); + let out_needed = relative_out_needed(scratch); + if scratch <= MAX_PATH_BYTES && out_needed <= MAX_PATH_BYTES { + return Ok(Box::from(relative_platform::( + from, to, + ))); + } + + let mut from_buf = vec![0u8; scratch]; + let mut to_buf = vec![0u8; scratch]; + let mut out = vec![0u8; out_needed]; + Ok(Box::from(relative_platform_in::( + &mut from_buf, + &mut to_buf, + &mut out, + from, + to, + ))) +} + +/// The result borrows whichever of the three buffers it was left in: `out`, or +/// `relative_to_buf` when `!ALWAYS_COPY` and the answer is a suffix of `to`. +fn relative_platform_in<'a, P: PlatformT, const ALWAYS_COPY: bool>( + relative_from_buf: &'a mut [u8], + relative_to_buf: &'a mut [u8], + buf: &'a mut [u8], + from: &[u8], + to: &[u8], +) -> &'a [u8] { let normalized_from: &[u8] = if P::P.is_absolute(from) { 'brk: { if P::P == Platform::Loose && cfg!(windows) { @@ -716,7 +776,7 @@ pub fn relative_platform_buf<'a, P: PlatformT, const ALWAYS_COPY: bool>( // Avoid aliasing relative_to_buf as both input (normalize result) // and output (join target): normalize into `buf` scratch (caller // output buffer, untouched until the final relative_normalized_buf call - // and disjoint from both threadlocals), then join into relative_to_buf. + // and disjoint from both scratch buffers), then join into relative_to_buf. let norm_len = normalize_string_buf::(to, buf).len(); join_abs_string_buf::

( Fs::FileSystem::instance().top_level_dir(), @@ -740,11 +800,6 @@ pub fn relative_platform( ) } -pub fn relative_alloc(from: &[u8], to: &[u8]) -> Result, bun_alloc::AllocError> { - let result = relative_platform::(from, to); - Ok(Box::<[u8]>::from(result)) -} - // This function is based on Go's volumeNameLen function // https://cs.opensource.google/go/go/+/refs/tags/go1.17.6:src/path/filepath/path_windows.go;l=57 // volumeNameLen returns length of the leading volume name on Windows. @@ -1614,13 +1669,18 @@ enum JoinScratch { Heap(Vec), } +/// Bound on what `_join_abs_string_buf` writes, scratch or output: the +/// concatenation, the separator Windows adds after a bare share root, and the +/// byte normalizing can add (see [`normalize_string_spill`]). +#[inline] +fn join_abs_needed(cwd_len: usize, parts: &[&[u8]]) -> usize { + parts.iter().map(|p| p.len() + 1).sum::() + cwd_len + 2 +} + impl JoinScratch { #[inline] fn init(base: usize, parts: &[&[u8]]) -> Self { - let mut total = base + 2; - for p in parts { - total += p.len() + 1; - } + let total = join_abs_needed(base, parts); if total <= MAX_PATH_BYTES { JoinScratch::Pooled(crate::path_buffer_pool::get()) } else { @@ -1658,10 +1718,7 @@ pub fn join_abs_string_buf_checked<'a, P: PlatformT>( debug_assert!(!matches!(P::P, Platform::Nt)); // Fast path: size check only — don't allocate a JoinScratch here since the // inner join_abs_string_buf already has its own (avoids doubling stack usage). - let mut total: usize = cwd.len() + 2; - for p in parts { - total += p.len() + 1; - } + let total = join_abs_needed(cwd.len(), parts); if total < buf.len() { return Some(join_abs_string_buf::

(cwd, buf, parts)); } @@ -1679,6 +1736,26 @@ pub fn join_abs_string_buf_checked<'a, P: PlatformT>( Some(&buf[..len]) } +/// [`join_abs_string_buf`] into `buf` when the result fits, otherwise into +/// `spill` (grown as needed, untouched in the common case); cf. [`join_z_buf_spill`]. +pub fn join_abs_string_buf_spill<'a, P: PlatformT>( + cwd: &'a [u8], + buf: &'a mut [u8], + spill: &'a mut Vec, + parts: &[&[u8]], +) -> &'a [u8] { + let needed = join_abs_needed(cwd.len(), parts); + let out: &'a mut [u8] = if needed <= buf.len() { + buf + } else { + if spill.len() < needed { + spill.resize(needed, 0); + } + &mut spill[..] + }; + join_abs_string_buf::

(cwd, out, parts) +} + pub fn join_abs_string_buf_z<'a, P: PlatformT>( cwd: &'a [u8], buf: &'a mut [u8], @@ -2510,6 +2587,45 @@ mod tests { .unwrap_or(text_len) } + /// Returns `haystack_len` when `needle` does not occur, like the kernel. + #[unsafe(no_mangle)] + unsafe extern "C" fn highway_last_index_of_char( + haystack: *const u8, + haystack_len: usize, + needle: u8, + ) -> usize { + // SAFETY: test stub; callers pass a valid (ptr, len) pair. + let haystack = unsafe { core::slice::from_raw_parts(haystack, haystack_len) }; + haystack + .iter() + .rposition(|&b| b == needle) + .unwrap_or(haystack_len) + } + + /// Returns `usize::MAX` when `needle` does not occur, like the kernel. + #[unsafe(no_mangle)] + unsafe extern "C" fn highway_memrmem16( + haystack: *const u16, + haystack_len: usize, + needle: *const u16, + needle_len: usize, + ) -> usize { + // SAFETY: test stub; callers pass valid (ptr, len) pairs. + let (haystack, needle) = unsafe { + ( + core::slice::from_raw_parts(haystack, haystack_len), + core::slice::from_raw_parts(needle, needle_len), + ) + }; + if needle_len > haystack_len { + return usize::MAX; + } + (0..=haystack_len - needle_len) + .rev() + .find(|&i| haystack[i..].starts_with(needle)) + .unwrap_or(usize::MAX) + } + #[test] fn normalize_string_spill_leaves_spill_untouched_when_the_input_fits() { let mut spill = Vec::new(); @@ -2631,4 +2747,111 @@ mod tests { &expected[..] ); } + + #[test] + fn join_abs_string_buf_spill_leaves_spill_untouched_when_the_result_fits() { + let mut buf = [0u8; 64]; + let mut spill = Vec::new(); + let out = join_abs_string_buf_spill::( + b"/cwd", + &mut buf, + &mut spill, + &[b"./lib/../x.js"], + ); + assert_eq!(out, b"/cwd/x.js"); + assert!(spill.is_empty()); + } + + #[test] + fn join_abs_string_buf_spill_spills_a_result_longer_than_the_buffer() { + let part = vec![b'p'; MAX_PATH_BYTES * 2]; + let mut expected = b"/cwd/".to_vec(); + expected.extend_from_slice(&part); + + let mut buf = [0u8; MAX_PATH_BYTES]; + let mut spill = Vec::new(); + let out = + join_abs_string_buf_spill::(b"/cwd", &mut buf, &mut spill, &[&part]); + assert_eq!(out, &expected[..]); + assert!(!spill.is_empty()); + } + + #[test] + fn join_abs_string_buf_spill_uses_the_buffer_up_to_its_bound() { + let mut buf = [0u8; 64]; + let cwd = b"/c"; + let part = vec![b'q'; buf.len() - join_abs_needed(cwd.len(), &[b""])]; + let mut spill = Vec::new(); + let out = join_abs_string_buf_spill::(cwd, &mut buf, &mut spill, &[&part]); + assert_eq!(out.len(), cwd.len() + 1 + part.len()); + assert!(spill.is_empty()); + + let mut longer = part; + longer.push(b'q'); + let mut spill = Vec::new(); + let out = + join_abs_string_buf_spill::(cwd, &mut buf, &mut spill, &[&longer]); + assert_eq!(out.len(), cwd.len() + 1 + longer.len()); + assert!(!spill.is_empty()); + } + + #[test] + fn join_abs_string_buf_spill_accounts_for_windows_results_that_grow() { + // A bare share root gains a separator; `buf` is sized exactly to the bound. + let cwd = b"\\\\server\\share"; + let parts: [&[u8]; 1] = [b"x"]; + let mut buf = vec![0u8; join_abs_needed(cwd.len(), &parts)]; + let mut spill = Vec::new(); + let out = join_abs_string_buf_spill::(cwd, &mut buf, &mut spill, &parts); + assert_eq!(out, b"\\\\server\\share\\x"); + assert!(spill.is_empty()); + } + + #[test] + fn relative_alloc_matches_relative_for_paths_that_fit() { + let rel = relative_alloc(b"/a/b/c", b"/a/d/e.js").unwrap(); + assert_eq!( + &rel[..], + relative_platform::(b"/a/b/c", b"/a/d/e.js") + ); + #[cfg(not(windows))] + assert_eq!(&rel[..], b"../../d/e.js"); + } + + #[test] + fn relative_alloc_handles_a_target_longer_than_a_path_buffer() { + let mut to = b"/a/".to_vec(); + to.resize(MAX_PATH_BYTES * 2, b't'); + let rel = relative_alloc(b"/a/b", &to).unwrap(); + let mut expected = b"../".to_vec(); + expected.extend_from_slice(&to[b"/a/".len()..]); + #[cfg(not(windows))] + assert_eq!(&rel[..], &expected[..]); + #[cfg(windows)] + assert_eq!(rel.len(), expected.len()); + } + + #[test] + fn relative_alloc_handles_a_result_longer_than_a_path_buffer() { + // Both inputs fit in a PathBuffer; the `../` chain for `from` does not. + let mut from = Vec::new(); + while from.len() + 2 < MAX_PATH_BYTES { + from.extend_from_slice(b"/d"); + } + // Two bytes: the Windows arm drops one-byte root-level targets (pre-existing). + let rel = relative_alloc(&from, b"/tt").unwrap(); + + let components = from.len() / 2; + let mut expected = Vec::new(); + for i in 0..components { + if i > 0 { + expected.push(SEP); + } + expected.extend_from_slice(b".."); + } + expected.push(SEP); + expected.extend_from_slice(b"tt"); + assert_eq!(&rel[..], &expected[..]); + assert!(rel.len() > MAX_PATH_BYTES); + } } diff --git a/test/js/bun/plugin/plugins.test.ts b/test/js/bun/plugin/plugins.test.ts index 5aea1966b6c0..7c11ffd4b448 100644 --- a/test/js/bun/plugin/plugins.test.ts +++ b/test/js/bun/plugin/plugins.test.ts @@ -936,3 +936,60 @@ describe.concurrent("Bun.plugin.clearAll()", () => { }); }); }); + +it.concurrent("an onResolve result longer than a path buffer is a resolve error, not a crash", async () => { + // The path a plugin returns for an import inside a module used to be run + // through the fixed-size relative-path buffers while that module was + // linked, which crashed the process. Long enough to exceed the buffers and + // the resolver's own specifier limit on every platform, Windows included. + const longPath = "/" + Buffer.alloc(150_000, "a").toString(); + using dir = tempDir("plugin-onresolve-long-path", { + "preload.js": ` + Bun.plugin({ + name: "redirect-to-long-path", + setup(build) { + build.onResolve({ filter: /^\\.\\/child\\.js$/ }, () => ({ path: "/" + Buffer.alloc(150_000, "a").toString() })); + }, + }); + `, + "parent-require.js": `module.exports = require("./child.js");`, + "parent-import.mjs": `export * from "./child.js";`, + "entry.js": ` + const attempt = async fn => { + try { + await fn(); + return "loaded"; + } catch ({ name, level, referrer, message }) { + return { name, level, referrer, message }; + } + }; + console.log( + JSON.stringify({ + requireParent: require.resolve("./parent-require.js"), + importParent: require.resolve("./parent-import.mjs"), + viaRequire: await attempt(() => require("./parent-require.js")), + viaImport: await attempt(() => import("./parent-import.mjs")), + }), + ); + `, + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "--preload", "./preload.js", "entry.js"], + env: bunEnv, + cwd: String(dir), + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + // The fixture catches its own failures, so empty stdout means it crashed. + const { requireParent, importParent, ...results } = stdout.trim() ? JSON.parse(stdout) : { crashed: stderr }; + const tooLong = (parent: string) => ({ + name: "ResolveMessage", + level: "error", + referrer: parent, + message: `ENAMETOOLONG while resolving '${longPath}' from '${parent}'`, + }); + expect(results).toEqual({ viaRequire: tooLong(requireParent), viaImport: tooLong(importParent) }); + expect(exitCode).toBe(0); +}); diff --git a/test/js/node/module/node-module-module.test.js b/test/js/node/module/node-module-module.test.js index 9014990b19f1..15c29d15851e 100644 --- a/test/js/node/module/node-module-module.test.js +++ b/test/js/node/module/node-module-module.test.js @@ -389,6 +389,55 @@ console.log("survived", require("./late.js"));`, expect(_nodeModulePaths(ospath("/a/b/c/d") + path.sep)).toEqual(_nodeModulePaths("/a/b/c/d")); }); + test("_nodeModulePaths() accepts paths longer than PATH_MAX", async () => { + // Like Node's, this is string manipulation: the input never has to exist + // or fit a path buffer. It used to be resolved into a fixed-size buffer, + // which crashed the process, so this runs in a child. The inputs are built + // on both sides rather than passed through argv (too long for Windows'). + function inputs() { + return { + single: Buffer.alloc(100_000, "a").toString(), + segments: Array.from({ length: 100 }, (_, i) => `seg${i}-` + Buffer.alloc(56, "s").toString()), + relative: Buffer.alloc(5000, "r").toString(), + }; + } + const { single, segments, relative } = inputs(); + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + `const m = require("module"); + ${inputs} + const { single, segments, relative } = inputs(); + process.stdout.write(JSON.stringify({ + single: m._nodeModulePaths("/" + single), + segments: m._nodeModulePaths("/" + segments.join("/")), + relative: m._nodeModulePaths(relative), + dot: m._nodeModulePaths("."), + }));`, + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + const { dot, ...results } = JSON.parse(stdout); + + const root = path.resolve("/"); + expect(results).toEqual({ + single: [path.join(root, single, "node_modules"), path.join(root, "node_modules")], + segments: [ + ...segments.map((_, i) => path.join(root, ...segments.slice(0, segments.length - i), "node_modules")), + path.join(root, "node_modules"), + ], + // A relative input resolves against the cwd, so its lookup chain is the + // cwd's own chain with one more entry in front. + relative: [path.join(path.dirname(dot[0]), relative, "node_modules"), ...dot], + }); + expect(exitCode).toBe(0); + }); + test("_nodeModulePaths() is stable across process.chdir()", async () => { // process.chdir() re-seeds the resolver's cached top-level dir with a // trailing separator; _nodeModulePaths("") then used to emit a duplicate