Skip to content

Prep filter for NtQueryDirectoryFile#123695

Merged
stephentoub merged 7 commits intodotnet:mainfrom
JeremyKuhne:passfsfilter
Feb 3, 2026
Merged

Prep filter for NtQueryDirectoryFile#123695
stephentoub merged 7 commits intodotnet:mainfrom
JeremyKuhne:passfsfilter

Conversation

@JeremyKuhne
Copy link
Member

NtQueryDirectoryFile should always be able to take filters as long as they are properly escaped for how we'll do our matching. The special directories . and .. get blocked, but anything outside of that should be fair game.

cc: @stephentoub

NtQueryDirectoryFile should always be able to take filters as long as they are properly escaped for how we'll do our matching. The special directories `.` and `..` get blocked, but anything outside of that should be fair game.

cc: @stephentoub
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors how filter expressions are prepared for NtQueryDirectoryFile on Windows. Instead of conservatively rejecting patterns that might be unsafe for OS-level filtering, it now escapes special DOS wildcard characters to ensure any expression (except . and ..) can be safely passed to the OS.

Changes:

  • Adds expression preprocessing logic to escape special characters (\, ", <, >) for MatchType.Simple patterns
  • Replaces the conservative IsSafePatternForOSFilter check with a simple null check, allowing more patterns to use OS-level filtering
  • Removes the IsSafePatternForOSFilter method and its associated SearchValues helper

@stephentoub
Copy link
Member

@JeremyKuhne there are 73 relevant test failures, e.g.

    System.IO.Tests.Directory_GetFiles_str_str.WindowsSearchPatternWhitespace [FAIL]
      System.IO.IOException : The filename, directory name, or volume label syntax is incorrect. : 'C:\h\w\B61E099C\t\#Directory_GetFiles_str_str_u2cqwqyk.mit'.
      Stack Trace:
        /_/src/libraries/System.Private.CoreLib/src/System/IO/Enumeration/FileSystemEnumerator.Windows.cs(282,0): at System.IO.Enumeration.FileSystemEnumerator`1.MoveNext()
        /_/src/libraries/System.Private.CoreLib/src/System/Collections/Generic/List.cs(85,0): at System.Collections.Generic.List`1..ctor(IEnumerable`1 collection)
        /_/src/libraries/System.Private.CoreLib/src/System/IO/Directory.cs(174,0): at System.IO.Directory.GetFiles(String path, String searchPattern, EnumerationOptions enumerationOptions)
        /_/src/libraries/System.Private.CoreLib/src/System/IO/Directory.cs(168,0): at System.IO.Directory.GetFiles(String path, String searchPattern)
        /_/src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/Directory/GetFiles.cs(96,0): at System.IO.Tests.Directory_GetFiles_str_str.GetEntries(String path, String searchPattern)
        /_/src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/Directory/GetFileSystemEntries_str_str.cs(695,0): at System.IO.Tests.Directory_GetFileSystemEntries_str_str.WindowsSearchPatternWhitespace()
           at System.RuntimeMethodHandle.InvokeMethod(ObjectHandleOnStack target, Void** arguments, ObjectHandleOnStack sig, BOOL isConstructor, ObjectHandleOnStack result)
           at System.RuntimeMethodHandle.InvokeMethod(ObjectHandleOnStack target, Void** arguments, ObjectHandleOnStack sig, BOOL isConstructor, ObjectHandleOnStack result)
        /_/src/coreclr/System.Private.CoreLib/src/System/Reflection/MethodBaseInvoker.CoreCLR.cs(36,0): at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
        /_/src/libraries/System.Private.CoreLib/src/System/Reflection/RuntimeMethodInfo.cs(134,0): at System.Reflection.RuntimeMethodInfo.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)

@jozkee
Copy link
Member

jozkee commented Jan 28, 2026

I'm still confused about the FileName param in NtQueryDirectoryFile. I can't find anything backing that it follows similar rules to MatchType.Win32. How do we know that's the case?

@JeremyKuhne
Copy link
Member Author

I'm still confused about the FileName param in NtQueryDirectoryFile. I can't find anything backing that it follows similar rules to MatchType.Win32. How do we know that's the case?

@jozkee Well, partially by looking at the sources when writing this all originally. :) https://learn.microsoft.com/openspecs/windows_protocols/ms-fsa/0b034646-4e23-4874-8488-2adac231ff23 talks about it. RtlIsNameInExpression is what everything calls (and we literally copy in FileSystemName), but it isn't particularly coherently documented. You can look at the implementation for FindFirstFile to see that it translates to these special characters.

Copilot AI review requested due to automatic review settings January 29, 2026 18:33
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Copilot AI review requested due to automatic review settings January 29, 2026 18:52
Copy link
Member

@stephentoub stephentoub left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Copy link
Member

@jozkee jozkee left a comment

Choose a reason for hiding this comment

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

Thanks, @JeremyKuhne.

Comment on lines +424 to +426
ReadOnlySpan<char> charsToEscape = ['\\', '"', '<', '>'];

int i = span.IndexOfAny(charsToEscape);
Copy link
Member

Choose a reason for hiding this comment

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

nit

Suggested change
ReadOnlySpan<char> charsToEscape = ['\\', '"', '<', '>'];
int i = span.IndexOfAny(charsToEscape);
int i = span.IndexOfAny("\\\"<>");

public void ReturnSpecialDirectories_WithExpression_ReturnsSpecialDirectory(MatchType matchType, string pattern)
{
string testDirectory = Directory.CreateDirectory(GetTestFilePath()).FullName;
string[] names = GetNames(testDirectory, pattern, matchType);
Copy link
Member

Choose a reason for hiding this comment

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

If this test doesn't fail, I guess we could remove if (!PlatformDetection.IsWindows10Version20348OrGreater) from the other tests too.

@jozkee
Copy link
Member

jozkee commented Feb 2, 2026

/ba-g infrastructure errors

@stephentoub stephentoub merged commit 63122f3 into dotnet:main Feb 3, 2026
146 of 153 checks passed
lewing pushed a commit to lewing/runtime that referenced this pull request Feb 9, 2026
Co-authored-by: Stephen Toub <stoub@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants