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
1 change: 1 addition & 0 deletions src/codegen/generate-js2native.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ const rustIdentifierPaths: Record<string, string> = {
"shell.rs": "runtime/shell/shell.rs",
"sourcemap/InternalSourceMap.rs": "sourcemap/InternalSourceMap.rs",
"string/immutable/unicode.rs": "bun_core/string/immutable/unicode.rs",
"string_paths.rs": "paths/string_paths.rs",
"subprocess.rs": "runtime/api/bun/subprocess.rs",
"sys.rs": "sys/sys.rs",
"sys/Error.rs": "sys/Error.rs",
Expand Down
7 changes: 7 additions & 0 deletions src/js/internal-for-testing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -737,6 +737,13 @@ export const stringsInternals = {
) => string,
};

export const pathsInternals = {
/** The Windows arm of `without_trailing_slash_windows_path` (src/paths/string_paths.rs), callable on every host. */
withoutTrailingSlashWindows: $newRustFunction("string_paths.rs", "TestingAPIs.withoutTrailingSlashWindows", 1) as (
path: string,
) => string,
};

/** Seed the connect-path DNS cache for `hostname` via the real `process_results` interleave; returns family order. */
export const dnsCacheSeed = $newRustFunction("runtime/dns_jsc/dns.rs", "internal.seedCacheForTesting", 2) as (
hostname: string,
Expand Down
26 changes: 26 additions & 0 deletions src/jsc/resolve_path_jsc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,29 @@ extern "C" fn ResolvePath__joinAbsStringBufCurrentPlatformBunString(

BunString::clone_utf8(out_slice)
}

pub mod testing_apis {
use crate::bun_string_jsc::to_js;
use crate::{CallFrame, JSGlobalObject, JSValue, JsResult};
use bun_core::String as BunString;

/// `pathsInternals.withoutTrailingSlashWindows` in `internal-for-testing.ts`.
#[bun_jsc::host_fn]
pub fn without_trailing_slash_windows(
global: &JSGlobalObject,
call_frame: &CallFrame,
) -> JsResult<JSValue> {
let path_value = call_frame.argument(0);
if !path_value.is_string() {
return Err(global.throw(format_args!("expected a string path")));
}
let path = path_value.to_slice(global)?;

let output = BunString::clone_utf8(
bun_paths::string_paths::without_trailing_slash_windows(path.slice()),
);
let js = to_js(&output, global);
output.deref();
js
}
}
72 changes: 69 additions & 3 deletions src/paths/string_paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -452,13 +452,21 @@ pub fn starts_with_windows_drive_letter_t<T: Ch>(s: &[T]) -> bool {

pub use crate::strings::without_trailing_slash;

/// Does not strip the device root (C:\ or \\Server\Share\ portion off of the path)
/// [`without_trailing_slash_windows`] on Windows, [`without_trailing_slash`] elsewhere.
pub fn without_trailing_slash_windows_path(input: &[u8]) -> &[u8] {
if cfg!(unix) || input.len() < 3 || input[1] != b':' {
if cfg!(unix) {
return without_trailing_slash(input);
}
without_trailing_slash_windows(input)
}

/// Windows rules on any host: trailing separators go, a drive root's own stays (`C:\\` -> `C:\`).
pub fn without_trailing_slash_windows(input: &[u8]) -> &[u8] {
if input.len() < 3 || input[1] != b':' {
return without_trailing_slash(input);
}

let root_len = resolve_path::windows_filesystem_root(input).len() + 1;
let root_len = resolve_path::windows_filesystem_root(input).len();

let mut path = input;
while path.len() > root_len && matches!(path[path.len() - 1], b'/' | b'\\') {
Expand Down Expand Up @@ -721,4 +729,62 @@ mod tests {
assert!(fits_in_wide_path_buffer(&vec![0x80u8; 32758]));
assert!(fits_in_wide_path_buffer(&vec![0x80u8; 32757]));
}

fn check_without_trailing_slash_windows(cases: &[(&str, &str)]) {
for (input, expected) in cases {
assert_eq!(
bstr::BStr::new(without_trailing_slash_windows(input.as_bytes())),
bstr::BStr::new(expected.as_bytes()),
"input {input:?}"
);
}
}

#[test]
fn without_trailing_slash_windows_strips_below_a_drive() {
check_without_trailing_slash_windows(&[
("C:\\foo\\", "C:\\foo"),
("C:\\foo\\\\", "C:\\foo"),
("C:/foo/", "C:/foo"),
("C:\\foo/\\", "C:\\foo"),
("C:\\a\\", "C:\\a"),
("C:\\foo", "C:\\foo"),
// Separators before the last component are not the helper's job.
("C:\\\\foo\\", "C:\\\\foo"),
// Drive-relative: `C:` is the root and `foo` is relative to it.
("C:foo\\", "C:foo"),
("C:foo", "C:foo"),
]);
}

#[test]
fn without_trailing_slash_windows_keeps_exactly_the_drive_root() {
check_without_trailing_slash_windows(&[
("C:\\", "C:\\"),
("C:/", "C:/"),
("c:\\", "c:\\"),
// These used to keep one extra separator.
("C:\\\\", "C:\\"),
("C://", "C:/"),
("C:\\/", "C:\\"),
("C:\\\\\\\\", "C:\\"),
]);
}

#[test]
fn without_trailing_slash_windows_strips_paths_without_a_drive_like_posix() {
check_without_trailing_slash_windows(&[
("\\\\server\\share\\dir\\", "\\\\server\\share\\dir"),
// UNC roots are keyed without the slash (Resolver::assert_valid_cache_key).
("\\\\server\\share\\", "\\\\server\\share"),
("\\dir\\", "\\dir"),
("dir\\", "dir"),
("dir/", "dir"),
("\\", "\\"),
("/", "/"),
("//", "/"),
("C:", "C:"),
("", ""),
]);
}
}
1 change: 1 addition & 0 deletions src/runtime/dispatch_js2native.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ pub use bun_jsc::virtual_machine_exports::Bun__setSyntheticAllocationLimitForTes
pub use bun_jsc::bun_string_jsc::js_escape_reg_exp as string_escape_reg_exp_js_escape_reg_exp;
pub use bun_jsc::bun_string_jsc::js_escape_reg_exp_for_package_name_matching as string_escape_reg_exp_js_escape_reg_exp_for_package_name_matching;
pub use bun_jsc::bun_string_jsc::unicode_testing_apis::to_utf16_alloc_sentinel as bun_core_string_immutable_unicode_testing_ap_is_to_utf16_alloc_sentinel;
pub use bun_jsc::resolve_path_jsc::testing_apis::without_trailing_slash_windows as paths_string_paths_testing_ap_is_without_trailing_slash_windows;

pub use bun_patch_jsc::testing::patch_apply as patch_patch_testing_ap_is_apply;
pub use bun_patch_jsc::testing::patch_make_diff as patch_patch_testing_ap_is_make_diff;
Expand Down
68 changes: 68 additions & 0 deletions test/internal/string-paths.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
// `without_trailing_slash_windows_path` (src/paths/string_paths.rs) produces
// the resolver's directory cache keys: no trailing separator, except that a
// drive root keeps its one separator (`C:\`), see
// `Resolver::assert_valid_cache_key`. Its drive-letter arm only runs on
// Windows hosts, and the only thing that observes its result there is that
// assertion (debug and ASAN builds; see "directory cache key computation at
// the drive root" in test/js/bun/resolve/resolve.test.ts), so the arm is bound
// directly through `bun:internal-for-testing` and its results pinned here on
// every platform. The same table runs under `cargo test -p bun_paths`.
//
// It used to stop stripping one byte past `windows_filesystem_root`, so `C:\\`
// (the root plus one more separator) came back unchanged and failed the
// assertion.
//
// Namespace import: on a build without the binding, `pathsInternals` is
// undefined and each test below fails, instead of the file failing to link.
import * as internals from "bun:internal-for-testing";
import { describe, expect, test } from "bun:test";

function withoutTrailingSlashWindows(path: string): string {
return internals.pathsInternals.withoutTrailingSlashWindows(path);
}

describe("without_trailing_slash_windows_path", () => {
test.each([
["C:\\", "C:\\"],
["C:/", "C:/"],
["c:\\", "c:\\"],
["C:\\\\", "C:\\"],
["C://", "C:/"],
["C:\\/", "C:\\"],
["C:\\\\\\\\", "C:\\"],
])("%s is the drive root %s", (input, expected) => {
expect(withoutTrailingSlashWindows(input)).toBe(expected);
});

test.each([
["C:\\foo\\", "C:\\foo"],
["C:\\foo\\\\", "C:\\foo"],
["C:/foo/", "C:/foo"],
["C:\\foo/\\", "C:\\foo"],
["C:\\a\\", "C:\\a"],
["C:\\foo", "C:\\foo"],
// Only trailing separators are stripped.
["C:\\\\foo\\", "C:\\\\foo"],
// Drive-relative: `C:` is the root.
["C:foo\\", "C:foo"],
["C:foo", "C:foo"],
])("%s below a drive root becomes %s", (input, expected) => {
expect(withoutTrailingSlashWindows(input)).toBe(expected);
});

test.each([
["\\\\server\\share\\dir\\", "\\\\server\\share\\dir"],
// UNC roots are keyed without the separator.
["\\\\server\\share\\", "\\\\server\\share"],
["\\dir\\", "\\dir"],
["dir\\", "dir"],
["dir/", "dir"],
["\\", "\\"],
["/", "/"],
["//", "/"],
["C:", "C:"],
["", ""],
])("%s without a drive letter becomes %s", (input, expected) => {
expect(withoutTrailingSlashWindows(input)).toBe(expected);
});
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.
11 changes: 11 additions & 0 deletions test/js/bun/resolve/resolve.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,17 @@ it.if(isWindows)("directory cache key computation", () => {
expect(import(`\\\\\\Test\\doesnotexist.ts\\\\` as any)).rejects.toThrow();
});

// The failed resolution busts the directory cache for the dirname, which for
// `C:\\\x` is `C:\\`. That used to reach the cache as-is and fail the cache key
// assertion (debug and ASAN builds) instead of the import rejecting.
it.if(isWindows)("directory cache key computation at the drive root", async () => {
const drive = process.cwd().slice(0, 2);
for (const separators of [2, 3, 4]) {
await expect(import(drive + "\\".repeat(separators) + "doesnotexist.ts")).rejects.toThrow();
}
await expect(import(`${drive}///doesnotexist.ts`)).rejects.toThrow();
});

describe("NODE_PATH test", () => {
const prepareTest = () => {
const tempDir = tempDirWithFiles("node_path", {
Expand Down
Loading