Skip to content

glob: pass pattern component as NtQueryDirectoryFile FileName filter on Windows #28489

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Jarred-Sumner merged 3 commits into from
Mar 25, 2026
Merged

glob: pass pattern component as NtQueryDirectoryFile FileName filter on Windows #28489

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
429af18
Pass glob component as NtQueryDirectoryFile FileName filter on Windows
dylan-conway Mar 24, 2026
d786932
Add multi-level glob pattern to scan benchmark
dylan-conway Mar 24, 2026
766ce44
Handle STATUS_NO_SUCH_FILE from NtQueryDirectoryFile with filter
dylan-conway Mar 24, 2026
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
33 changes: 33 additions & 0 deletions bench/glob/scan.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { bench, group, run } from "../runner.mjs";
const normalPattern = "*.ts";
const recursivePattern = "**/*.ts";
const nodeModulesPattern = "**/node_modules/**/*.js";
const multiLevelPattern = "node_modules/*/lib/*.js";

const benchFdir = false;
const cwd = undefined;
Expand Down Expand Up @@ -104,6 +105,38 @@ group({ name: `node_modules pattern="${nodeModulesPattern}"`, summary: true }, (
});
});

group({ name: `multi-level pattern="${multiLevelPattern}"`, summary: true }, () => {
bench("fast-glob", async () => {
const entries = await fg.glob([multiLevelPattern], fgOpts);
});

if (Glob)
bench("Bun.Glob", async () => {
const entries = await Array.fromAsync(new Glob(multiLevelPattern).scan(bunOpts));
});

if (benchFdir)
bench("fdir", async () => {
const entries = await new fdir().withFullPaths().glob(multiLevelPattern).crawl(process.cwd()).withPromise();
});
});

group({ name: `sync multi-level pattern="${multiLevelPattern}"`, summary: true }, () => {
bench("fast-glob", () => {
const entries = fg.globSync([multiLevelPattern], fgOpts);
});

if (Glob)
bench("Bun.Glob", () => {
const entries = [...new Glob(multiLevelPattern).scanSync(bunOpts)];
});

if (benchFdir)
bench("fdir", () => {
const entries = new fdir().withFullPaths().glob(multiLevelPattern).crawl(process.cwd()).sync();
});
});

await run({
avg: true,
colors: false,
Expand Down
25 changes: 23 additions & 2 deletions src/bun.js/node/dir_iterator.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,10 @@ pub fn NewIterator(comptime use_windows_ospath: bool) type {
end_index: usize,
first: bool,
name_data: if (use_windows_ospath) [257]u16 else [513]u8,
/// Optional kernel-side wildcard filter passed to NtQueryDirectoryFile.
/// Evaluated by FsRtlIsNameInExpression (case-insensitive, supports `*` and `?`).
/// Only honored on the first call (RestartScan=TRUE); sticky for the handle lifetime.
name_filter: ?[]const u16 = null,

const Self = @This();

Expand All @@ -220,6 +224,16 @@ pub fn NewIterator(comptime use_windows_ospath: bool) type {
@memset(&self.buf, 0);
}

var filter_us: w.UNICODE_STRING = undefined;
const filter_ptr: ?*w.UNICODE_STRING = if (self.name_filter) |f| blk: {
filter_us = .{
.Length = @intCast(f.len * 2),

@Jarred-Sumner Jarred-Sumner Mar 24, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is byte length?

.MaximumLength = @intCast(f.len * 2),
.Buffer = @constCast(f.ptr),
};
break :blk &filter_us;
} else null;

const rc = w.ntdll.NtQueryDirectoryFile(
self.dir.cast(),
null,
Expand All @@ -230,7 +244,7 @@ pub fn NewIterator(comptime use_windows_ospath: bool) type {
self.buf.len,
.FileDirectoryInformation,
w.FALSE,
null,
filter_ptr,
if (self.first) @as(w.BOOLEAN, w.TRUE) else @as(w.BOOLEAN, w.FALSE),
);

Expand All @@ -252,7 +266,9 @@ pub fn NewIterator(comptime use_windows_ospath: bool) type {
};
}

if (rc == .NO_MORE_FILES) {
// NO_SUCH_FILE is returned on the first call when a FileName filter
// matches nothing; NO_MORE_FILES on subsequent calls. Both mean "done".
if (rc == .NO_MORE_FILES or rc == .NO_SUCH_FILE) {
bun.sys.syslog("NtQueryDirectoryFile({f}) = {s}", .{ self.dir, @tagName(rc) });
self.end_index = self.index;
return .{ .result = null };
Comment on lines 266 to 274

@claude claude Bot Mar 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 The new rc == .NO_SUCH_FILE branch (line 271) is dead code in its primary intended scenario because the if (io.Information == 0) early-return at line 252 fires first. When NtQueryDirectoryFile returns STATUS_NO_SUCH_FILE (filter matched nothing on first call), the kernel writes 0 bytes to the buffer so io.Information == 0, causing the earlier guard to return null with a misleading syslog message of = 0 rather than = NO_SUCH_FILE. The fix would be to reorder the checks so status codes are evaluated before the io.Information == 0 guard.

Extended reasoning...

When NtQueryDirectoryFile returns STATUS_NO_SUCH_FILE on the first call (because a FileName filter matched no entries), the NT kernel writes zero bytes to the output buffer, setting io.Information to 0. This is standard NT behavior for synchronous file handles as Bun uses.

The check order in the Windows next() function is: (1) if (io.Information == 0) fires at line 252 returning null with syslog = 0, then (2) self.index/end_index are set, (3) INVALID_PARAMETER check, and finally (4) the new NO_MORE_FILES or NO_SUCH_FILE check at line 271. When STATUS_NO_SUCH_FILE is returned, step 1 fires immediately because io.Information == 0, so the new explicit NO_SUCH_FILE check at step 4 is never reached.

The io.Information == 0 guard is a reasonable early-exit optimization, but it was placed before the status-code checks. Since STATUS_NO_SUCH_FILE always (in the primary case) results in io.Information == 0, the io.Information path absorbs the return before the newly added explicit status check can run.

Functional correctness is unaffected — both paths return null. However, the debug syslog emits NtQueryDirectoryFile(fd) = 0 instead of = NO_SUCH_FILE, which is misleading when diagnosing why a filtered glob found no results. The PR description specifically claims this PR handles STATUS_NO_SUCH_FILE, but the actual handling was already provided by the pre-existing io.Information == 0 path.

The refutation argues this is intentional defensive programming for the hypothetical case where a filesystem driver returns STATUS_NO_SUCH_FILE with io.Information > 0. While theoretically possible with non-standard drivers, it is undocumented and not observed in practice. By contrast, the analogous NO_MORE_FILES check IS reachable in normal operation when the last batch of entries partially fills the buffer. The NO_SUCH_FILE case lacks this justification since it only occurs on the very first call. The defensive-programming argument is weak here because the code already correctly handles NO_SUCH_FILE via the io.Information == 0 path.

Step-by-step proof: (1) Glob pattern *.target used; computeNtFilter returns .target as NT filter. (2) NtQueryDirectoryFile called with RestartScan=TRUE and FileName=.target. (3) No files match; kernel writes 0 bytes to buffer, sets io.Information = 0, returns STATUS_NO_SUCH_FILE. (4) self.first = false is set. (5) if (io.Information == 0) is true, so syslog prints NtQueryDirectoryFile(fd) = 0 and return .{ .result = null } is executed. (6) Lines 268-274 (the NO_SUCH_FILE check) are never reached. (7) Syslog shows = 0 rather than = NO_SUCH_FILE.

The fix is to move the status-code checks (NO_MORE_FILES, NO_SUCH_FILE, INVALID_PARAMETER) before the io.Information == 0 guard, or merge the NO_SUCH_FILE branch into the io.Information == 0 path so both the correct syslog message and the early return are emitted together.

Expand Down Expand Up @@ -449,6 +465,11 @@ pub fn NewWrappedIterator(comptime path_type: PathType) type {
},
};
}

pub fn setNameFilter(self: *Self, filter: ?[]const u16) void {
if (comptime !bun.Environment.isWindows) return;
self.iter.name_filter = filter;
}
};
}

Expand Down
44 changes: 43 additions & 1 deletion src/glob/GlobWalker.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,10 @@ pub const SyscallAccessor = struct {
pub inline fn iterate(dir: Handle) DirIter {
return .{ .value = DirIterator.WrappedIterator.init(dir.value) };
}

pub inline fn setNameFilter(self: *DirIter, filter: ?[]const u16) void {
self.value.setNameFilter(filter);
}
};

pub fn open(path: [:0]const u8) !Maybe(Handle) {
Expand Down Expand Up @@ -435,6 +439,8 @@ pub fn GlobWalker_(
/// directory being iterated on.
fds_open: if (count_fds) usize else u0 = 0,

nt_filter_buf: if (isWindows) [256]u16 else void = if (isWindows) undefined else {},

pub fn init(this: *Iterator) !Maybe(void) {
log("Iterator init pattern={s}", .{this.walker.pattern});
var was_absolute = false;
Expand Down Expand Up @@ -702,13 +708,49 @@ pub fn GlobWalker_(
log("Transition(dirpath={s}, component_idx={d})", .{ dir_path, component_idx });

this.iter_state.directory.fd = fd;
const iterator = Accessor.DirIter.iterate(fd);
var iterator = Accessor.DirIter.iterate(fd);
if (comptime isWindows) {
if (@hasDecl(Accessor.DirIter, "setNameFilter")) {
iterator.setNameFilter(this.computeNtFilter(component_idx));
}
}
this.iter_state.directory.iter = iterator;
this.iter_state.directory.iter_closed = false;

return .success;
}

/// Compute an optional NtQueryDirectoryFile FileName filter for the current
/// pattern component. The kernel filter is used purely as a pre-filter;
/// matchPatternImpl still runs on every returned entry for correctness
/// (case sensitivity, 8.3 aliases, etc). We only emit a filter when the
/// NT match is guaranteed to be a superset of the glob match.
fn computeNtFilter(this: *Iterator, component_idx: u32) ?[]const u16 {
if (comptime !isWindows) return null;

const comp = &this.walker.patternComponents.items[component_idx];
switch (comp.syntax_hint) {
// `*` and `**` match everything; a filter gains nothing and for `**`
// would incorrectly hide subdirectories we need to recurse into.
.Single, .Double, .Dot, .DotBack => return null,
else => {},
}

const slice = comp.patternSlice(this.walker.pattern);
if (slice.len == 0 or slice.len > this.nt_filter_buf.len) return null;

// Only `*` and literals are safe to lower. Reject anything NT cannot
// express (`[` `{` `\` `!`) or where NT semantics under-match glob
// (`?` matches one UTF-16 code unit, glob matches one codepoint).
// `<` `>` `"` are NT wildcards; treating them as literals would over-match,
// but they are invalid in Windows filenames so such a pattern never matches
// anyway.
Comment on lines +735 to +747

@claude claude Bot Mar 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 The size guard in computeNtFilter compares the UTF-8 byte length of the pattern slice against nt_filter_buf.len (256 u16 elements), causing the NT filter optimization to be silently skipped for non-ASCII patterns whose UTF-8 byte count exceeds 256 even though their UTF-16 representation would fit in the buffer. The fix is to compare against the actual UTF-16 code unit count (e.g., via bun.strings.utf16Len) rather than the raw byte length.

Extended reasoning...

The Bug

In computeNtFilter (src/glob/GlobWalker.zig, around line 745), the guard reads:

if (slice.len == 0 or slice.len > this.nt_filter_buf.len) return null;

slice is a UTF-8 byte slice returned by comp.patternSlice(this.walker.pattern), so slice.len is a count of UTF-8 bytes. this.nt_filter_buf is [256]u16, so .len is 256 — a count of UTF-16 code units. The two units are not the same for non-ASCII patterns.

Code Path

computeNtFilter is called from transitionToDirIterState on every directory transition on Windows. For each pattern component that passes the syntax check, it hits this guard before attempting the UTF-8→UTF-16 conversion via bun.strings.convertUTF8toUTF16InBuffer.

Why the existing guard is incorrect (but safe)

For all valid Unicode, the number of UTF-16 code units is always ≤ the number of UTF-8 bytes:

  • 1-byte UTF-8 → 1 UTF-16 code unit (ASCII)
  • 2-byte UTF-8 → 1 UTF-16 code unit (e.g., Cyrillic U+0400–U+04FF, Latin Extended)
  • 3-byte UTF-8 → 1 UTF-16 code unit (e.g., most CJK)
  • 4-byte UTF-8 → 2 UTF-16 code units (surrogate pair)

This means the guard can never cause a buffer overflow — if slice.len ≤ 256, the UTF-16 output is also ≤ 256 code units. However, it is overly conservative: it can reject patterns that would actually fit in the buffer.

Step-by-step Proof

Consider a glob pattern component made entirely of Cyrillic characters, each encoded as a 2-byte UTF-8 sequence (e.g., U+0410 = \xD0\x90):

  • 128 Cyrillic characters: slice.len = 256, UTF-16 length = 128. The guard evaluates 256 > 256 → false → passes (correctly allows the filter).
  • 129 Cyrillic characters: slice.len = 258, UTF-16 length = 129. The guard evaluates 258 > 256 → true → returns null, skipping the NT filter optimization even though the UTF-16 output (129 code units) fits comfortably in the 256-element buffer.

For a 200-character Cyrillic pattern: slice.len = 400 > 256, so the guard skips the filter. The UTF-16 output would be 200 code units — well within the buffer.

Impact

Purely a missed optimization. matchPatternImpl still runs on every entry returned by NtQueryDirectoryFile, so glob results are always correct. The NT filter is a pre-filter that reduces the number of entries the kernel serializes to userspace. For non-ASCII patterns on Windows that have multi-byte UTF-8 encodings and are longer than 128 characters (for 2-byte sequences) but shorter than 257 UTF-16 code units, the optimization is silently skipped, costing some performance.

Fix

Compare against the actual UTF-16 length before conversion. One approach:

const utf16_len = bun.strings.utf16Len(slice);
if (utf16_len == 0 or utf16_len > this.nt_filter_buf.len) return null;

Alternatively, run the conversion first and check the result length, discarding it if it exceeds the buffer.

if (bun.strings.indexOfAny(slice, "?[{\\!<>\"") != null) return null;

const wide = bun.strings.convertUTF8toUTF16InBuffer(&this.nt_filter_buf, slice);
return wide;
}

pub fn next(this: *Iterator) !Maybe(?MatchedPath) {
while (true) {
switch (this.iter_state) {
Expand Down
Loading