Skip to content

Commit 14167c2

Browse files
branchseerclaude
andauthored
fix(fspy): handle null ObjectName and Buffer in UNICODE_STRING on Windows (#91)
## Summary - Fix crash when running oxlint with `--type-aware` flag on Windows - The crash occurred in `fspy_preload_windows` at `winapi_utils.rs:33` due to null pointer in `slice::from_raw_parts` - Root cause: `ObjectName` or its `Buffer` field in `UNICODE_STRING` can be null when tsgolint is spawned ## Changes - `convert.rs`: Check for null `ObjectName` before dereferencing - `winapi_utils.rs`: Handle null/empty `Buffer` by returning empty slice, fix `Length` interpretation (bytes, not u16 count) - Added `oxlint_type_aware` test to verify the fix - Added `oxlint-tsgolint` to test_bins for the test ## Test plan - [x] `cargo test -p fspy oxlint_type_aware` passes - [x] Test verifies that `--type-aware` flag works without crashing 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
1 parent db0eaf1 commit 14167c2

16 files changed

Lines changed: 263 additions & 117 deletions

crates/fspy/tests/oxlint.rs

Lines changed: 59 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,47 @@
11
mod test_utils;
22

3-
use std::{env::vars_os, process::Stdio};
3+
use std::{env::vars_os, ffi::OsString};
44

55
use fspy::{AccessMode, PathAccessIterable};
66
use test_log::test;
77

8-
/// Find the oxlint executable in test_bins
9-
fn find_oxlint() -> std::path::PathBuf {
10-
let test_bins_dir = std::path::Path::new(env!("CARGO_MANIFEST_DIR"))
8+
/// Get the test_bins/.bin directory path
9+
fn test_bins_bin_dir() -> std::path::PathBuf {
10+
std::path::Path::new(env!("CARGO_MANIFEST_DIR"))
1111
.parent()
1212
.unwrap()
1313
.join("vite_task_bin")
1414
.join("test_bins")
1515
.join("node_modules")
16-
.join(".bin");
16+
.join(".bin")
17+
}
1718

19+
/// Find the oxlint executable in test_bins
20+
fn find_oxlint() -> std::path::PathBuf {
21+
let test_bins_dir = test_bins_bin_dir();
1822
which::which_in("oxlint", Some(&test_bins_dir), std::env::current_dir().unwrap())
1923
.expect("oxlint not found in test_bins/node_modules/.bin")
2024
}
2125

2226
async fn track_oxlint(dir: &std::path::Path, args: &[&str]) -> anyhow::Result<PathAccessIterable> {
2327
let oxlint_path = find_oxlint();
2428
let mut command = fspy::Command::new(&oxlint_path);
25-
command.args(args).stdout(Stdio::null()).stderr(Stdio::null()).envs(vars_os()).current_dir(dir);
29+
30+
// Build PATH with test_bins/.bin prepended so oxlint can find tsgolint
31+
let test_bins_dir = test_bins_bin_dir();
32+
let new_path = if let Some(existing_path) = std::env::var_os("PATH") {
33+
let mut paths = vec![test_bins_dir.as_os_str().to_owned()];
34+
paths.extend(std::env::split_paths(&existing_path).map(|p| p.into_os_string()));
35+
std::env::join_paths(paths)?
36+
} else {
37+
OsString::from(&test_bins_dir)
38+
};
39+
40+
command
41+
.args(args)
42+
.envs(vars_os().filter(|(k, _)| !k.eq_ignore_ascii_case("PATH")))
43+
.env("PATH", new_path)
44+
.current_dir(dir);
2645

2746
let child = command.spawn().await?;
2847
let termination = child.wait_handle.await?;
@@ -54,13 +73,44 @@ async fn oxlint_reads_directory() -> anyhow::Result<()> {
5473
// on macOS, tmpdir.path() may be a symlink, so we need to canonicalize it
5574
let tmpdir_path = std::fs::canonicalize(tmpdir.path())?;
5675

57-
let js_file = tmpdir.path().join("test.js");
58-
std::fs::write(&js_file, "console.log('hello');")?;
59-
6076
let accesses = track_oxlint(&tmpdir_path, &[]).await?;
6177

6278
// Check that oxlint read the directory to find JS files
6379
// This is the key check - if READ_DIR is not tracked, cache won't detect new files
6480
test_utils::assert_contains(&accesses, &tmpdir_path, AccessMode::READ_DIR);
6581
Ok(())
6682
}
83+
84+
#[test(tokio::test)]
85+
async fn oxlint_type_aware() -> anyhow::Result<()> {
86+
let tmpdir = tempfile::tempdir()?;
87+
// on macOS, tmpdir.path() may be a symlink, so we need to canonicalize it
88+
let tmpdir_path = std::fs::canonicalize(tmpdir.path())?;
89+
90+
// Create a simple TypeScript file
91+
let ts_file = tmpdir_path.join("index.ts");
92+
std::fs::write(
93+
&ts_file,
94+
r#"
95+
import type { Foo } from './types';
96+
declare const _foo: Foo;
97+
"#,
98+
)?;
99+
100+
// Run oxlint without --type-aware first
101+
let accesses = track_oxlint(&tmpdir_path, &[""]).await?;
102+
let access_to_types_ts = accesses.iter().find(|access| {
103+
let os_str = access.path.to_cow_os_str();
104+
os_str.as_encoded_bytes().ends_with(b"\\types.ts")
105+
|| os_str.as_encoded_bytes().ends_with(b"/types.ts")
106+
});
107+
assert_eq!(access_to_types_ts, None, "oxlint should not read types.ts without --type-aware");
108+
109+
// Run oxlint with --type-aware to enable type-aware linting
110+
let accesses = track_oxlint(&tmpdir_path, &["--type-aware"]).await?;
111+
112+
// Check that oxlint read types.ts
113+
test_utils::assert_contains(&accesses, &tmpdir_path.join("types.ts"), AccessMode::READ);
114+
115+
Ok(())
116+
}

crates/fspy/tests/rust_std.rs

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,25 @@ async fn open_write() -> anyhow::Result<()> {
3737

3838
#[test(tokio::test)]
3939
async fn readdir() -> anyhow::Result<()> {
40-
let accesses = track_child!((), |(): ()| {
40+
let tmpdir = tempfile::tempdir()?;
41+
let tmpdir_path = std::fs::canonicalize(tmpdir.path())?;
42+
// Reading a non-existent directory results in different tracked accesses on different platforms:
43+
// - Windows: READ, because the NT APIs open the directory as handle just like files (NtCreateFile/NtOpenFile),
44+
// and if that fails, not read dir call (NtQueryDirectoryFile/NtQueryDirectoryFileEx) is made.
45+
// - macOS/Linux:
46+
// - opendir results in a read_dir access. This call is directly made without trying to open the directory as a fd first.
47+
// - open + fopendir results in READ access, because open would fail with ENOENT, and fopendir is not called.
48+
//
49+
// This difference is acceptable because both will result in a "not found" fingerprint in vite-task.
50+
// To keep the test consistent across platforms, we create the directory first.
51+
std::fs::create_dir(tmpdir.path().join("hello_dir"))?;
52+
53+
let accesses = track_child!(tmpdir_path.to_str().unwrap().to_owned(), |tmpdir_path: String| {
54+
std::env::set_current_dir(tmpdir_path).unwrap();
4155
let _ = std::fs::read_dir("hello_dir");
4256
})
4357
.await?;
44-
assert_contains(&accesses, current_dir()?.join("hello_dir").as_path(), AccessMode::READ_DIR);
58+
assert_contains(&accesses, tmpdir_path.join("hello_dir").as_path(), AccessMode::READ_DIR);
4559

4660
Ok(())
4761
}

crates/fspy/tests/rust_tokio.rs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,15 +42,21 @@ async fn open_write() -> anyhow::Result<()> {
4242

4343
#[test(tokio::test)]
4444
async fn readdir() -> anyhow::Result<()> {
45-
let accesses = track_child!((), |(): ()| {
45+
let tmpdir = tempfile::tempdir()?;
46+
let tmpdir_path = std::fs::canonicalize(tmpdir.path())?;
47+
48+
std::fs::create_dir(tmpdir.path().join("hello_dir"))?;
49+
50+
let accesses = track_child!(tmpdir_path.to_str().unwrap().to_owned(), |tmpdir_path: String| {
51+
std::env::set_current_dir(tmpdir_path).unwrap();
4652
tokio::runtime::Builder::new_current_thread().enable_io().build().unwrap().block_on(
4753
async {
4854
let _ = tokio::fs::read_dir("hello_dir").await;
4955
},
5056
);
5157
})
5258
.await?;
53-
assert_contains(&accesses, current_dir()?.join("hello_dir").as_path(), AccessMode::READ_DIR);
59+
assert_contains(&accesses, tmpdir_path.join("hello_dir").as_path(), AccessMode::READ_DIR);
5460

5561
Ok(())
5662
}

crates/fspy_preload_windows/src/windows/convert.rs

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,16 +50,24 @@ impl ToAbsolutePath for POBJECT_ATTRIBUTES {
5050
self,
5151
f: F,
5252
) -> winsafe::SysResult<R> {
53-
let filename_str = unsafe { get_u16_str(&*(*self).ObjectName) };
53+
let filename_str = if let Some(object_name) = unsafe { (*self).ObjectName.as_ref() } {
54+
unsafe { get_u16_str(object_name) }
55+
} else {
56+
U16Str::from_slice(&[])
57+
};
5458
let filename_slice = filename_str.as_slice();
55-
let is_absolute = (filename_slice.get(0) == Some(&b'\\'.into())
56-
&& filename_slice.get(1) == Some(&b'\\'.into())) // \\...
59+
let is_absolute = filename_slice.get(0) == Some(&b'\\'.into()) // \...
5760
|| filename_slice.get(1) == Some(&b':'.into()); // C:...
5861

59-
if is_absolute {
62+
if !is_absolute {
6063
let Ok(mut root_dir) = (unsafe { get_path_name((*self).RootDirectory) }) else {
6164
return f(None);
6265
};
66+
// If filename is empty, just use root_dir directly
67+
if filename_str.is_empty() {
68+
let root_dir_str = U16Str::from_slice(&root_dir);
69+
return f(Some(root_dir_str));
70+
}
6371
let root_dir_cstr = {
6472
root_dir.push(0);
6573
unsafe { U16CStr::from_ptr_str(root_dir.as_ptr()) }

crates/fspy_preload_windows/src/windows/detour.rs

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ impl<T: Copy> Detour<T> {
2121
Detour { symbol_name, target: UnsafeCell::new(unsafe { transmute_copy(&target) }), new }
2222
}
2323

24-
#[expect(dead_code)]
2524
pub const unsafe fn dynamic(symbol_name: &'static CStr, new: T) -> Self {
2625
Detour { symbol_name, target: UnsafeCell::new(null_mut()), new }
2726
}
@@ -52,15 +51,18 @@ pub struct DetourAny {
5251
pub struct AttachContext {
5352
kernelbase: HMODULE,
5453
kernel32: HMODULE,
54+
ntdll: HMODULE,
5555
}
5656

5757
impl AttachContext {
5858
pub fn new() -> Self {
5959
let kernelbase = unsafe { LoadLibraryA(c"kernelbase".as_ptr()) };
6060
let kernel32 = unsafe { LoadLibraryA(c"kernel32".as_ptr()) };
61+
let ntdll = unsafe { LoadLibraryA(c"ntdll".as_ptr()) };
6162
assert_ne!(kernelbase, null_mut());
6263
assert_ne!(kernel32, null_mut());
63-
Self { kernelbase, kernel32 }
64+
assert_ne!(ntdll, null_mut());
65+
Self { kernelbase, kernel32, ntdll }
6466
}
6567
}
6668

@@ -74,9 +76,14 @@ impl DetourAny {
7476
unsafe { *self.target = symbol_in_kernelbase.cast() };
7577
} else {
7678
if unsafe { *self.target }.is_null() {
77-
// dynamic symbol
79+
// dynamic symbol - look up from kernel32 or ntdll
7880
let symbol_in_kernel32 = unsafe { GetProcAddress(ctx.kernel32, symbol_name) };
79-
unsafe { *self.target = symbol_in_kernel32.cast() };
81+
if !symbol_in_kernel32.is_null() {
82+
unsafe { *self.target = symbol_in_kernel32.cast() };
83+
} else {
84+
let symbol_in_ntdll = unsafe { GetProcAddress(ctx.ntdll, symbol_name) };
85+
unsafe { *self.target = symbol_in_ntdll.cast() };
86+
}
8087
}
8188
}
8289
if unsafe { *self.target }.is_null() {

crates/fspy_preload_windows/src/windows/detours/nt.rs

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,55 @@ static DETOUR_NT_QUERY_DIRECTORY_FILE: Detour<
286286
})
287287
};
288288

289+
// NtQueryDirectoryFileEx is not in ntapi crate, so we define it here.
290+
// https://learn.microsoft.com/en-us/windows-hardware/drivers/ddi/ntifs/nf-ntifs-ntquerydirectoryfileex
291+
type NtQueryDirectoryFileExFn = unsafe extern "system" fn(
292+
file_handle: HANDLE,
293+
event: HANDLE,
294+
apc_routine: PIO_APC_ROUTINE,
295+
apc_context: PVOID,
296+
io_status_block: PIO_STATUS_BLOCK,
297+
file_information: PVOID,
298+
length: ULONG,
299+
file_information_class: FILE_INFORMATION_CLASS,
300+
query_flags: ULONG,
301+
file_name: PUNICODE_STRING,
302+
) -> NTSTATUS;
303+
304+
static DETOUR_NT_QUERY_DIRECTORY_FILE_EX: Detour<NtQueryDirectoryFileExFn> = unsafe {
305+
Detour::dynamic(c"NtQueryDirectoryFileEx", {
306+
unsafe extern "system" fn new_fn(
307+
file_handle: HANDLE,
308+
event: HANDLE,
309+
apc_routine: PIO_APC_ROUTINE,
310+
apc_context: PVOID,
311+
io_status_block: PIO_STATUS_BLOCK,
312+
file_information: PVOID,
313+
length: ULONG,
314+
file_information_class: FILE_INFORMATION_CLASS,
315+
query_flags: ULONG,
316+
file_name: PUNICODE_STRING,
317+
) -> NTSTATUS {
318+
unsafe { handle_open(AccessMode::READ_DIR, file_handle) };
319+
unsafe {
320+
(DETOUR_NT_QUERY_DIRECTORY_FILE_EX.real())(
321+
file_handle,
322+
event,
323+
apc_routine,
324+
apc_context,
325+
io_status_block,
326+
file_information,
327+
length,
328+
file_information_class,
329+
query_flags,
330+
file_name,
331+
)
332+
}
333+
}
334+
new_fn
335+
})
336+
};
337+
289338
pub const DETOURS: &[DetourAny] = &[
290339
DETOUR_NT_CREATE_FILE.as_any(),
291340
DETOUR_NT_OPEN_FILE.as_any(),
@@ -294,4 +343,5 @@ pub const DETOURS: &[DetourAny] = &[
294343
DETOUR_NT_OPEN_SYMBOLIC_LINK_OBJECT.as_any(),
295344
DETOUR_NT_QUERY_INFORMATION_BY_NAME.as_any(),
296345
DETOUR_NT_QUERY_DIRECTORY_FILE.as_any(),
346+
DETOUR_NT_QUERY_DIRECTORY_FILE_EX.as_any(),
297347
];

crates/fspy_preload_windows/src/windows/winapi_utils.rs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,16 @@ pub fn ck_long(val: c_long) -> winsafe::SysResult<()> {
2929
}
3030

3131
pub unsafe fn get_u16_str(ustring: &UNICODE_STRING) -> &U16Str {
32-
let chars =
33-
unsafe { slice::from_raw_parts((*ustring).Buffer, (*ustring).Length.try_into().unwrap()) };
32+
// https://learn.microsoft.com/en-us/windows/win32/api/subauth/ns-subauth-unicode_string
33+
// UNICODE_STRING.Length is in bytes
34+
let u16_count = ustring.Length / 2;
35+
let chars: &[u16] = if u16_count == 0 {
36+
// If length is zero, we can't use slice::from_raw_parts as it requires a non-null pointer but
37+
// Buffer may be null in that case.
38+
&[]
39+
} else {
40+
unsafe { slice::from_raw_parts((*ustring).Buffer, u16_count.try_into().unwrap()) }
41+
};
3442
match U16CStr::from_slice_truncate(chars) {
3543
Ok(ok) => ok.as_ustr(),
3644
Err(_) => chars.into(),

crates/fspy_shared/src/ipc/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ impl Debug for AccessMode {
3232
}
3333
}
3434

35-
#[derive(Encode, BorrowDecode, Debug, Clone, Copy)]
35+
#[derive(Encode, BorrowDecode, Debug, Clone, Copy, PartialEq, Eq)]
3636
pub struct PathAccess<'a> {
3737
pub mode: AccessMode,
3838
pub path: &'a NativeStr,

crates/vite_task_bin/test_bins/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
"@yarnpkg/shell": "catalog:",
1313
"cross-env": "^10.1.0",
1414
"oxlint": "catalog:",
15+
"oxlint-tsgolint": "catalog:",
1516
"vite-task-test-bins": "link:"
1617
}
1718
}

crates/vite_task_bin/tests/test_snapshots/main.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ use std::{
1313
use copy_dir::copy_dir;
1414
use redact::{redact_e2e_output, redact_snapshot};
1515
use tokio::runtime::Runtime;
16-
use vite_path::{AbsolutePath, RelativePathBuf};
16+
use vite_path::{AbsolutePath, AbsolutePathBuf, RelativePathBuf};
1717
use vite_str::Str;
1818
use vite_task::{CLIArgs, Session};
1919
use vite_task_bin::CustomTaskSubcommand;
@@ -209,13 +209,13 @@ fn run_case(runtime: &Runtime, tmpdir: &AbsolutePath, fixture_path: &Path) {
209209
fn test_snapshots() {
210210
let tokio_runtime = Runtime::new().unwrap();
211211
let tmp_dir = tempfile::tempdir().unwrap();
212-
let tmp_dir_path = AbsolutePath::new(tmp_dir.path()).unwrap();
212+
let tmp_dir_path = AbsolutePathBuf::new(tmp_dir.path().canonicalize().unwrap()).unwrap();
213213

214214
let tests_dir = std::env::current_dir().unwrap().join("tests");
215215

216216
insta::glob!(tests_dir, "test_snapshots/fixtures/*", |case_path| run_case(
217217
&tokio_runtime,
218-
tmp_dir_path,
218+
&tmp_dir_path,
219219
case_path
220220
));
221221
}

0 commit comments

Comments
 (0)