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
14 changes: 6 additions & 8 deletions src/runtime/node/node_fs_stat_watcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,13 @@ use bun_jsc::{
self as jsc, CallFrame, JSGlobalObject, JSValue, JsCell, JsRef, JsResult, WorkPool,
WorkPoolTask,
};
use bun_paths::resolve_path::{self as Path, platform};
use bun_ptr::{BackRef, ParentRef, RefPtr, ThreadSafeRefCount};
use bun_resolver::fs;
use bun_sys::{self, PosixStat};
use bun_sys::{self, E, PosixStat};
use bun_threading::{Guarded, UnboundedQueue};

use crate::generated_classes::js_StatWatcher as js;
use crate::node::node_fs_watcher::absolute_watch_path_z;
use crate::node::stat::{StatsBig, StatsSmall};
use crate::node::types::PathLikeExt;
use crate::timer::{EventLoopTimer, EventLoopTimerState, EventLoopTimerTag};
Expand Down Expand Up @@ -828,15 +828,13 @@ impl StatWatcher {
slice = &slice[b"file://".len()..];
}

// SAFETY: `FileSystem::instance()` is initialized at process start
// (`FileSystem::init` runs before any JS module loads).
let top_level_dir = fs::FileSystem::get().top_level_dir;
let parts: [&[u8]; 1] = [slice];
let file_path =
Path::join_abs_string_buf::<platform::Auto>(top_level_dir, &mut buf[..], &parts);
let Some(file_path) = absolute_watch_path_z(top_level_dir, slice, &mut buf) else {
return Err(bun_sys::Error::from_code(E::ENAMETOOLONG, bun_sys::Tag::watch).into());
Comment thread
robobun marked this conversation as resolved.
Resolved
};

// allocSentinel + memcpy → owned NUL-terminated copy (ZBox)
let alloc_file_path = ZBox::from_bytes(file_path);
let alloc_file_path = ZBox::from_bytes(file_path.as_bytes());
// errdefer free → Drop handles it

// `args.global_this` is a `BackRef` (JSC_BORROW); safe Deref.
Expand Down
49 changes: 37 additions & 12 deletions src/runtime/node/node_fs_watcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ use bun_jsc::{
VirtualMachineRef as VirtualMachine,
};
use bun_jsc::{JsCell, JsCellRefExt as _};
use bun_paths::resolve_path::{self as Path, platform};
use bun_sys::{self, SystemErrno};
use bun_threading::Mutex;

Expand All @@ -33,6 +32,42 @@ use super::path_watcher;
#[cfg(windows)]
use super::win_watcher as path_watcher;

/// Absolute watch path; POSIX keeps the caller's bytes unnormalized. `None` if it does not fit.
pub(crate) fn absolute_watch_path_z<'a>(
cwd: &[u8],
path: &[u8],
buf: &'a mut bun_paths::PathBuffer,
) -> Option<&'a bun_core::ZStr> {
let cap = buf.len() - 1;
#[cfg(windows)]
let len = bun_paths::resolve_path::join_abs_string_buf_checked::<
bun_paths::resolve_path::platform::Windows,
>(cwd, &mut buf[..cap], &[path])?
.len();
#[cfg(not(windows))]
let len = {
let mut len = 0;
if !bun_paths::is_absolute(path) {
if cwd.len() + 1 > cap {
return None;
}
buf[..cwd.len()].copy_from_slice(cwd);
len = cwd.len();
if len > 0 && buf[len - 1] != b'/' {
buf[len] = b'/';
len += 1;
}
}
if len + path.len() > cap {
return None;
}
buf[len..len + path.len()].copy_from_slice(path);
len + path.len()
};
buf[len] = 0;
Some(bun_core::ZStr::from_buf(&buf[..], len))
}

// TODO: make this a top-level struct
// R-2 (host-fn re-entrancy): every JS-exposed method takes `&self`; per-field
// interior mutability via `Cell` (Copy) / `JsCell` (non-Copy). `&mut Self`
Expand Down Expand Up @@ -1115,25 +1150,15 @@ impl FSWatcher {
}
s
};
// SAFETY: `FileSystem::instance()` returns the process-global singleton
// initialized at startup; never null once init has run.
let cwd = bun_resolver::fs::FileSystem::get().top_level_dir;
let joined_buf_len = joined_buf.len();
let Some(joined) = Path::join_abs_string_buf_checked::<platform::Auto>(
cwd,
&mut joined_buf[..joined_buf_len - 1],
&[slice],
) else {
let Some(file_path) = absolute_watch_path_z(cwd, slice, &mut joined_buf) else {
return Err(bun_sys::Error {
errno: SystemErrno::ENAMETOOLONG as _,
syscall: bun_sys::Tag::watch,
path: args.path.slice().into(),
..Default::default()
});
};
let joined_len = joined.len();
joined_buf[joined_len] = 0;
let file_path: &bun_core::ZStr = bun_core::ZStr::from_buf(&joined_buf[..], joined_len);

let vm = args.global_this.bun_vm_ptr();
// `bun_vm()` is the audited safe `&'static VirtualMachine` accessor —
Expand Down
45 changes: 45 additions & 0 deletions test/js/node/watch/fs.watch.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,51 @@ describe("fs.watch", () => {
});
});

// On POSIX `a\b` is one filename, not `a/b`.
test.skipIf(isWindows)("a backslash in the path is not a separator", async () => {
const dir = tempDirWithFiles("watch-backslash", { "a/other.txt": "unrelated" });
const literal = path.join(dir, "a\\b");
fs.writeFileSync(literal, "literal");

// `a/b` does not exist, so a watcher that split on `\` throws ENOENT here.
const watcher = fs.watch(literal);
try {
const { promise, resolve, reject } = Promise.withResolvers<[string, string | Buffer | null]>();
watcher.once("error", reject);
watcher.once("change", (event, filename) => resolve([event, filename]));
const interval = repeat(() => fs.writeFileSync(literal, "changed"));
const [event, filename] = await promise.finally(() => clearInterval(interval));
expect([event, filename]).toEqual(["change", "a\\b"]);
} finally {
watcher.close();
}
});

// The kernel resolves `link/..` to the parent of the link target. A lexical
// join folds it to the directory that holds the link instead.
test.skipIf(isWindows)("`symlink/..` in the path resolves through the symlink", async () => {
const dir = tempDirWithFiles("watch-dotdot", { "d/keep.txt": "", "other/sub/keep.txt": "" });
fs.symlinkSync(path.join(dir, "other", "sub"), path.join(dir, "d", "link"));

// d/link/.. is other/, not d/. (path.join would fold the `..` away.)
const watcher = fs.watch(`${dir}/d/link/..`);
try {
const { promise, resolve, reject } = Promise.withResolvers<string | Buffer | null>();
watcher.once("error", reject);
watcher.on("change", (event, filename) => {
if (filename === "in-other.txt" || filename === "in-d.txt") resolve(filename);
});
const interval = repeat(() => {
fs.writeFileSync(path.join(dir, "other", "in-other.txt"), "x");
fs.writeFileSync(path.join(dir, "d", "in-d.txt"), "x");
});
const filename = await promise.finally(() => clearInterval(interval));
expect(filename).toBe("in-other.txt");
} finally {
Comment thread
robobun marked this conversation as resolved.
Resolved
watcher.close();
}
});

test("should emit 'change' event when file is modified", done => {
const filepath = path.join(testDir, "watch.txt");

Expand Down
28 changes: 28 additions & 0 deletions test/js/node/watch/fs.watchFile.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,34 @@ describe("fs.watchFile", () => {
expect(entries[0][0].mtimeMs).toBeGreaterThan(entries[0][1].mtimeMs);
});

// On POSIX `a\b` is one filename, not `a/b`.
test.skipIf(isWindows)("a backslash in the filename is not a separator", async () => {
const literal = path.join(testDir, "a\\b");
const nested = path.join(testDir, "a", "b");
fs.mkdirSync(path.join(testDir, "a"));
fs.writeFileSync(literal, "literal");
fs.writeFileSync(nested, "n");

let { promise, resolve } = Promise.withResolvers<[fs.Stats, fs.Stats]>();
fs.watchFile(literal, { interval: 50 }, (curr, prev) => resolve([curr, prev]));
let increment = 0;
// Touch both files so the listener fires whichever one the watcher stats.
const interval = repeat(() => {
increment++;
updateFile(literal, "literal" + increment);
updateFile(nested, "n" + (increment % 10));
});
const [curr, prev] = await promise;
clearInterval(interval);
fs.unwatchFile(literal);

// `a\b` holds "literal..." (7+ bytes), `a/b` holds "n." (1-2 bytes). The
// first update can land before the watcher's initial stat, so `prev` may
// already be past 7.
expect(prev.size).toBeGreaterThanOrEqual("literal".length);
expect(curr.size).toBeGreaterThanOrEqual("literal1".length);
});

test("bigint stats", async () => {
let entries: any = [];
let { promise, resolve } = Promise.withResolvers<void>();
Expand Down
Loading