System.IO: Make CanSeek independent of SafeFileHandle.Type

[jozkee]

Follow-up to #132077.

Summary

• Determine Windows SafeFileHandle.CanSeek directly from the raw GetFileType result instead of the cached, refined SafeFileHandle.Type value.
• Add symbolic-link regression tests covering Type access both before and after FileStream evaluates CanSeek.

Rationale

For handles opened on a symbolic link, Windows reports FILE_TYPE_DISK, while SafeFileHandle.Type refines that to SymbolicLink. As a result, #132077 could cache different CanSeek values depending on whether Type was queried before constructing the FileStream. Using the raw file type restores access-order-independent behavior while retaining the write-only named-pipe fix.

Validation

• .\build.cmd clr.corelib — succeeded with 0 warnings and 0 errors.
• FileStream_SymbolicLink_CanSeek_IsNotAffectedByType — passed.
• FileStream_WriteOnlyNamedPipe_CanSeek_IsAsync — 2 cases passed.

No baseline build was run.

Note

This pull request description was generated with GitHub Copilot.

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[Copilot]

Pull request overview

Updates Windows SafeFileHandle.CanSeek to be derived directly from the raw GetFileType result, avoiding access-order-dependent behavior when SafeFileHandle.Type (which refines disk handles into SymbolicLink, Directory, etc.) is queried before creating a FileStream. Adds Windows regression tests that exercise the symbolic-link scenario.

Changes:

• Compute SafeFileHandle seekability solely from Kernel32.GetFileType(handle) == FILE_TYPE_DISK.
• Add regression tests validating FileStream.CanSeek is unaffected by prior SafeFileHandle.Type access on symlink handles.
• Add a helper to open a symlink as a reparse point handle for the new tests.

Reviewed changes

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

File	Description
src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Windows.cs	Makes CanSeek determination independent of the refined cached handle Type by using raw GetFileType.
src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/SafeFileHandle/GetFileType.Windows.cs	Adds Windows symbolic-link regression tests covering Type access order relative to FileStream.CanSeek.

Suppressed comments (1)

src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/SafeFileHandle/GetFileType.Windows.cs:160

• This test verifies stability after CanSeek is evaluated, but it still only checks equality. Add an explicit assertion for the expected value to ensure the test fails if CanSeek regresses to a consistent-but-wrong result.

 bool canSeek = stream.CanSeek;
Assert.Equal(FileHandleType.SymbolicLink, handle.Type);
Assert.Equal(canSeek, stream.CanSeek);

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-io
See info in area-owners.md if you want to be subscribed.

[adamsitnik]

Overall it LGTM, but I think the tests can be improved to avoid code duplication.

Big thanks for catching the issue and providing the fix @jozkee !

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/SafeFileHandle/GetFileType.Windows.cs:133

• The PR description / validation section references a second regression test (FileStream_SymbolicLink_CanSeek_IsNotAffectedBySubsequentTypeAccess), but the current change only adds FileStream_SymbolicLink_CanSeek_IsNotAffectedByType (with an InlineData parameter covering both access orders). This can make it harder to correlate the stated validation with the actual tests in the diff. Consider updating the PR description (or splitting this into two separately-named tests if that’s the intent).

 [ConditionalTheory(typeof(MountHelper), nameof(MountHelper.CanCreateSymbolicLinks))]
[InlineData(true)]
[InlineData(false)]
public unsafe void FileStream_SymbolicLink_CanSeek_IsNotAffectedByType(bool accessTypeFirst)
{

[adamsitnik]

LGTM, big thanks for spotting the issue and providing the fix @jozkee !

[jozkee]

/ba-g #132030

[jozkee]

Follow-up to #132077.

Summary

• Determine Windows SafeFileHandle.CanSeek directly from the raw GetFileType result instead of the cached, refined SafeFileHandle.Type value.
• Add symbolic-link regression tests covering Type access both before and after FileStream evaluates CanSeek.

Rationale

For handles opened on a symbolic link, Windows reports FILE_TYPE_DISK, while SafeFileHandle.Type refines that to SymbolicLink. As a result, #132077 could cache different CanSeek values depending on whether Type was queried before constructing the FileStream. Using the raw file type restores access-order-independent behavior while retaining the write-only named-pipe fix.

Validation

• .\build.cmd clr.corelib — succeeded with 0 warnings and 0 errors.
• FileStream_SymbolicLink_CanSeek_IsNotAffectedByType — passed.
• FileStream_WriteOnlyNamedPipe_CanSeek_IsAsync — 2 cases passed.

No baseline build was run.

Note

This pull request description was generated with GitHub Copilot.

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[Copilot]

Pull request overview

Updates Windows SafeFileHandle.CanSeek to be derived directly from the raw GetFileType result, avoiding access-order-dependent behavior when SafeFileHandle.Type (which refines disk handles into SymbolicLink, Directory, etc.) is queried before creating a FileStream. Adds Windows regression tests that exercise the symbolic-link scenario.

Changes:

• Compute SafeFileHandle seekability solely from Kernel32.GetFileType(handle) == FILE_TYPE_DISK.
• Add regression tests validating FileStream.CanSeek is unaffected by prior SafeFileHandle.Type access on symlink handles.
• Add a helper to open a symlink as a reparse point handle for the new tests.

Reviewed changes

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

File	Description
src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Windows.cs	Makes CanSeek determination independent of the refined cached handle Type by using raw GetFileType.
src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/SafeFileHandle/GetFileType.Windows.cs	Adds Windows symbolic-link regression tests covering Type access order relative to FileStream.CanSeek.

Suppressed comments (1)

src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/SafeFileHandle/GetFileType.Windows.cs:160

• This test verifies stability after CanSeek is evaluated, but it still only checks equality. Add an explicit assertion for the expected value to ensure the test fails if CanSeek regresses to a consistent-but-wrong result.

 bool canSeek = stream.CanSeek;
Assert.Equal(FileHandleType.SymbolicLink, handle.Type);
Assert.Equal(canSeek, stream.CanSeek);

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-io
See info in area-owners.md if you want to be subscribed.

[adamsitnik]

Overall it LGTM, but I think the tests can be improved to avoid code duplication.

Big thanks for catching the issue and providing the fix @jozkee !

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/SafeFileHandle/GetFileType.Windows.cs:133

• The PR description / validation section references a second regression test (FileStream_SymbolicLink_CanSeek_IsNotAffectedBySubsequentTypeAccess), but the current change only adds FileStream_SymbolicLink_CanSeek_IsNotAffectedByType (with an InlineData parameter covering both access orders). This can make it harder to correlate the stated validation with the actual tests in the diff. Consider updating the PR description (or splitting this into two separately-named tests if that’s the intent).

 [ConditionalTheory(typeof(MountHelper), nameof(MountHelper.CanCreateSymbolicLinks))]
[InlineData(true)]
[InlineData(false)]
public unsafe void FileStream_SymbolicLink_CanSeek_IsNotAffectedByType(bool accessTypeFirst)
{

[adamsitnik]

LGTM, big thanks for spotting the issue and providing the fix @jozkee !

[jozkee]

/ba-g #132030

[jozkee]

Follow-up to #132077.

Summary

• Determine Windows SafeFileHandle.CanSeek directly from the raw GetFileType result instead of the cached, refined SafeFileHandle.Type value.
• Add symbolic-link regression tests covering Type access both before and after FileStream evaluates CanSeek.

Rationale

For handles opened on a symbolic link, Windows reports FILE_TYPE_DISK, while SafeFileHandle.Type refines that to SymbolicLink. As a result, #132077 could cache different CanSeek values depending on whether Type was queried before constructing the FileStream. Using the raw file type restores access-order-independent behavior while retaining the write-only named-pipe fix.

Validation

• .\build.cmd clr.corelib — succeeded with 0 warnings and 0 errors.
• FileStream_SymbolicLink_CanSeek_IsNotAffectedByType — passed.
• FileStream_WriteOnlyNamedPipe_CanSeek_IsAsync — 2 cases passed.

No baseline build was run.

Note

This pull request description was generated with GitHub Copilot.

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[Copilot]

Pull request overview

Updates Windows SafeFileHandle.CanSeek to be derived directly from the raw GetFileType result, avoiding access-order-dependent behavior when SafeFileHandle.Type (which refines disk handles into SymbolicLink, Directory, etc.) is queried before creating a FileStream. Adds Windows regression tests that exercise the symbolic-link scenario.

Changes:

• Compute SafeFileHandle seekability solely from Kernel32.GetFileType(handle) == FILE_TYPE_DISK.
• Add regression tests validating FileStream.CanSeek is unaffected by prior SafeFileHandle.Type access on symlink handles.
• Add a helper to open a symlink as a reparse point handle for the new tests.

Reviewed changes

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

File	Description
src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Windows.cs	Makes CanSeek determination independent of the refined cached handle Type by using raw GetFileType.
src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/SafeFileHandle/GetFileType.Windows.cs	Adds Windows symbolic-link regression tests covering Type access order relative to FileStream.CanSeek.

Suppressed comments (1)

src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/SafeFileHandle/GetFileType.Windows.cs:160

• This test verifies stability after CanSeek is evaluated, but it still only checks equality. Add an explicit assertion for the expected value to ensure the test fails if CanSeek regresses to a consistent-but-wrong result.

 bool canSeek = stream.CanSeek;
Assert.Equal(FileHandleType.SymbolicLink, handle.Type);
Assert.Equal(canSeek, stream.CanSeek);

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-io
See info in area-owners.md if you want to be subscribed.

[adamsitnik]

Overall it LGTM, but I think the tests can be improved to avoid code duplication.

Big thanks for catching the issue and providing the fix @jozkee !

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/SafeFileHandle/GetFileType.Windows.cs:133

• The PR description / validation section references a second regression test (FileStream_SymbolicLink_CanSeek_IsNotAffectedBySubsequentTypeAccess), but the current change only adds FileStream_SymbolicLink_CanSeek_IsNotAffectedByType (with an InlineData parameter covering both access orders). This can make it harder to correlate the stated validation with the actual tests in the diff. Consider updating the PR description (or splitting this into two separately-named tests if that’s the intent).

 [ConditionalTheory(typeof(MountHelper), nameof(MountHelper.CanCreateSymbolicLinks))]
[InlineData(true)]
[InlineData(false)]
public unsafe void FileStream_SymbolicLink_CanSeek_IsNotAffectedByType(bool accessTypeFirst)
{

[adamsitnik]

LGTM, big thanks for spotting the issue and providing the fix @jozkee !

[jozkee]

/ba-g #132030

[jozkee]

Follow-up to #132077.

Summary

• Determine Windows SafeFileHandle.CanSeek directly from the raw GetFileType result instead of the cached, refined SafeFileHandle.Type value.
• Add symbolic-link regression tests covering Type access both before and after FileStream evaluates CanSeek.

Rationale

For handles opened on a symbolic link, Windows reports FILE_TYPE_DISK, while SafeFileHandle.Type refines that to SymbolicLink. As a result, #132077 could cache different CanSeek values depending on whether Type was queried before constructing the FileStream. Using the raw file type restores access-order-independent behavior while retaining the write-only named-pipe fix.

Validation

• .\build.cmd clr.corelib — succeeded with 0 warnings and 0 errors.
• FileStream_SymbolicLink_CanSeek_IsNotAffectedByType — passed.
• FileStream_WriteOnlyNamedPipe_CanSeek_IsAsync — 2 cases passed.

No baseline build was run.

Note

This pull request description was generated with GitHub Copilot.

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[Copilot]

Pull request overview

Updates Windows SafeFileHandle.CanSeek to be derived directly from the raw GetFileType result, avoiding access-order-dependent behavior when SafeFileHandle.Type (which refines disk handles into SymbolicLink, Directory, etc.) is queried before creating a FileStream. Adds Windows regression tests that exercise the symbolic-link scenario.

Changes:

• Compute SafeFileHandle seekability solely from Kernel32.GetFileType(handle) == FILE_TYPE_DISK.
• Add regression tests validating FileStream.CanSeek is unaffected by prior SafeFileHandle.Type access on symlink handles.
• Add a helper to open a symlink as a reparse point handle for the new tests.

Reviewed changes

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

File	Description
src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Windows.cs	Makes CanSeek determination independent of the refined cached handle Type by using raw GetFileType.
src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/SafeFileHandle/GetFileType.Windows.cs	Adds Windows symbolic-link regression tests covering Type access order relative to FileStream.CanSeek.

Suppressed comments (1)

src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/SafeFileHandle/GetFileType.Windows.cs:160

• This test verifies stability after CanSeek is evaluated, but it still only checks equality. Add an explicit assertion for the expected value to ensure the test fails if CanSeek regresses to a consistent-but-wrong result.

 bool canSeek = stream.CanSeek;
Assert.Equal(FileHandleType.SymbolicLink, handle.Type);
Assert.Equal(canSeek, stream.CanSeek);

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-io
See info in area-owners.md if you want to be subscribed.

[adamsitnik]

Overall it LGTM, but I think the tests can be improved to avoid code duplication.

Big thanks for catching the issue and providing the fix @jozkee !

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/SafeFileHandle/GetFileType.Windows.cs:133

• The PR description / validation section references a second regression test (FileStream_SymbolicLink_CanSeek_IsNotAffectedBySubsequentTypeAccess), but the current change only adds FileStream_SymbolicLink_CanSeek_IsNotAffectedByType (with an InlineData parameter covering both access orders). This can make it harder to correlate the stated validation with the actual tests in the diff. Consider updating the PR description (or splitting this into two separately-named tests if that’s the intent).

 [ConditionalTheory(typeof(MountHelper), nameof(MountHelper.CanCreateSymbolicLinks))]
[InlineData(true)]
[InlineData(false)]
public unsafe void FileStream_SymbolicLink_CanSeek_IsNotAffectedByType(bool accessTypeFirst)
{

[adamsitnik]

LGTM, big thanks for spotting the issue and providing the fix @jozkee !

[jozkee]

/ba-g #132030

[jozkee]

Follow-up to #132077.

Summary

• Determine Windows SafeFileHandle.CanSeek directly from the raw GetFileType result instead of the cached, refined SafeFileHandle.Type value.
• Add symbolic-link regression tests covering Type access both before and after FileStream evaluates CanSeek.

Rationale

For handles opened on a symbolic link, Windows reports FILE_TYPE_DISK, while SafeFileHandle.Type refines that to SymbolicLink. As a result, #132077 could cache different CanSeek values depending on whether Type was queried before constructing the FileStream. Using the raw file type restores access-order-independent behavior while retaining the write-only named-pipe fix.

Validation

• .\build.cmd clr.corelib — succeeded with 0 warnings and 0 errors.
• FileStream_SymbolicLink_CanSeek_IsNotAffectedByType — passed.
• FileStream_WriteOnlyNamedPipe_CanSeek_IsAsync — 2 cases passed.

No baseline build was run.

Note

This pull request description was generated with GitHub Copilot.

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[Copilot]

Pull request overview

Updates Windows SafeFileHandle.CanSeek to be derived directly from the raw GetFileType result, avoiding access-order-dependent behavior when SafeFileHandle.Type (which refines disk handles into SymbolicLink, Directory, etc.) is queried before creating a FileStream. Adds Windows regression tests that exercise the symbolic-link scenario.

Changes:

• Compute SafeFileHandle seekability solely from Kernel32.GetFileType(handle) == FILE_TYPE_DISK.
• Add regression tests validating FileStream.CanSeek is unaffected by prior SafeFileHandle.Type access on symlink handles.
• Add a helper to open a symlink as a reparse point handle for the new tests.

Reviewed changes

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

File	Description
src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Windows.cs	Makes CanSeek determination independent of the refined cached handle Type by using raw GetFileType.
src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/SafeFileHandle/GetFileType.Windows.cs	Adds Windows symbolic-link regression tests covering Type access order relative to FileStream.CanSeek.

Suppressed comments (1)

src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/SafeFileHandle/GetFileType.Windows.cs:160

• This test verifies stability after CanSeek is evaluated, but it still only checks equality. Add an explicit assertion for the expected value to ensure the test fails if CanSeek regresses to a consistent-but-wrong result.

 bool canSeek = stream.CanSeek;
Assert.Equal(FileHandleType.SymbolicLink, handle.Type);
Assert.Equal(canSeek, stream.CanSeek);

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-io
See info in area-owners.md if you want to be subscribed.

[adamsitnik]

Overall it LGTM, but I think the tests can be improved to avoid code duplication.

Big thanks for catching the issue and providing the fix @jozkee !

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/SafeFileHandle/GetFileType.Windows.cs:133

• The PR description / validation section references a second regression test (FileStream_SymbolicLink_CanSeek_IsNotAffectedBySubsequentTypeAccess), but the current change only adds FileStream_SymbolicLink_CanSeek_IsNotAffectedByType (with an InlineData parameter covering both access orders). This can make it harder to correlate the stated validation with the actual tests in the diff. Consider updating the PR description (or splitting this into two separately-named tests if that’s the intent).

 [ConditionalTheory(typeof(MountHelper), nameof(MountHelper.CanCreateSymbolicLinks))]
[InlineData(true)]
[InlineData(false)]
public unsafe void FileStream_SymbolicLink_CanSeek_IsNotAffectedByType(bool accessTypeFirst)
{

[adamsitnik]

LGTM, big thanks for spotting the issue and providing the fix @jozkee !

[jozkee]

/ba-g #132030

[jozkee]

Follow-up to #132077.

Summary

• Determine Windows SafeFileHandle.CanSeek directly from the raw GetFileType result instead of the cached, refined SafeFileHandle.Type value.
• Add symbolic-link regression tests covering Type access both before and after FileStream evaluates CanSeek.

Rationale

For handles opened on a symbolic link, Windows reports FILE_TYPE_DISK, while SafeFileHandle.Type refines that to SymbolicLink. As a result, #132077 could cache different CanSeek values depending on whether Type was queried before constructing the FileStream. Using the raw file type restores access-order-independent behavior while retaining the write-only named-pipe fix.

Validation

• .\build.cmd clr.corelib — succeeded with 0 warnings and 0 errors.
• FileStream_SymbolicLink_CanSeek_IsNotAffectedByType — passed.
• FileStream_WriteOnlyNamedPipe_CanSeek_IsAsync — 2 cases passed.

No baseline build was run.

Note

This pull request description was generated with GitHub Copilot.

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[Copilot]

Pull request overview

Updates Windows SafeFileHandle.CanSeek to be derived directly from the raw GetFileType result, avoiding access-order-dependent behavior when SafeFileHandle.Type (which refines disk handles into SymbolicLink, Directory, etc.) is queried before creating a FileStream. Adds Windows regression tests that exercise the symbolic-link scenario.

Changes:

• Compute SafeFileHandle seekability solely from Kernel32.GetFileType(handle) == FILE_TYPE_DISK.
• Add regression tests validating FileStream.CanSeek is unaffected by prior SafeFileHandle.Type access on symlink handles.
• Add a helper to open a symlink as a reparse point handle for the new tests.

Reviewed changes

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

File	Description
src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Windows.cs	Makes CanSeek determination independent of the refined cached handle Type by using raw GetFileType.
src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/SafeFileHandle/GetFileType.Windows.cs	Adds Windows symbolic-link regression tests covering Type access order relative to FileStream.CanSeek.

Suppressed comments (1)

src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/SafeFileHandle/GetFileType.Windows.cs:160

• This test verifies stability after CanSeek is evaluated, but it still only checks equality. Add an explicit assertion for the expected value to ensure the test fails if CanSeek regresses to a consistent-but-wrong result.

 bool canSeek = stream.CanSeek;
Assert.Equal(FileHandleType.SymbolicLink, handle.Type);
Assert.Equal(canSeek, stream.CanSeek);

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-io
See info in area-owners.md if you want to be subscribed.

[adamsitnik]

Overall it LGTM, but I think the tests can be improved to avoid code duplication.

Big thanks for catching the issue and providing the fix @jozkee !

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/SafeFileHandle/GetFileType.Windows.cs:133

• The PR description / validation section references a second regression test (FileStream_SymbolicLink_CanSeek_IsNotAffectedBySubsequentTypeAccess), but the current change only adds FileStream_SymbolicLink_CanSeek_IsNotAffectedByType (with an InlineData parameter covering both access orders). This can make it harder to correlate the stated validation with the actual tests in the diff. Consider updating the PR description (or splitting this into two separately-named tests if that’s the intent).

 [ConditionalTheory(typeof(MountHelper), nameof(MountHelper.CanCreateSymbolicLinks))]
[InlineData(true)]
[InlineData(false)]
public unsafe void FileStream_SymbolicLink_CanSeek_IsNotAffectedByType(bool accessTypeFirst)
{

[adamsitnik]

LGTM, big thanks for spotting the issue and providing the fix @jozkee !

[jozkee]

/ba-g #132030

[jozkee]

Follow-up to #132077.

Summary

• Determine Windows SafeFileHandle.CanSeek directly from the raw GetFileType result instead of the cached, refined SafeFileHandle.Type value.
• Add symbolic-link regression tests covering Type access both before and after FileStream evaluates CanSeek.

Rationale

For handles opened on a symbolic link, Windows reports FILE_TYPE_DISK, while SafeFileHandle.Type refines that to SymbolicLink. As a result, #132077 could cache different CanSeek values depending on whether Type was queried before constructing the FileStream. Using the raw file type restores access-order-independent behavior while retaining the write-only named-pipe fix.

Validation

• .\build.cmd clr.corelib — succeeded with 0 warnings and 0 errors.
• FileStream_SymbolicLink_CanSeek_IsNotAffectedByType — passed.
• FileStream_WriteOnlyNamedPipe_CanSeek_IsAsync — 2 cases passed.

No baseline build was run.

Note

This pull request description was generated with GitHub Copilot.

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[Copilot]

Pull request overview

Updates Windows SafeFileHandle.CanSeek to be derived directly from the raw GetFileType result, avoiding access-order-dependent behavior when SafeFileHandle.Type (which refines disk handles into SymbolicLink, Directory, etc.) is queried before creating a FileStream. Adds Windows regression tests that exercise the symbolic-link scenario.

Changes:

• Compute SafeFileHandle seekability solely from Kernel32.GetFileType(handle) == FILE_TYPE_DISK.
• Add regression tests validating FileStream.CanSeek is unaffected by prior SafeFileHandle.Type access on symlink handles.
• Add a helper to open a symlink as a reparse point handle for the new tests.

Reviewed changes

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

File	Description
src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Windows.cs	Makes CanSeek determination independent of the refined cached handle Type by using raw GetFileType.
src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/SafeFileHandle/GetFileType.Windows.cs	Adds Windows symbolic-link regression tests covering Type access order relative to FileStream.CanSeek.

Suppressed comments (1)

src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/SafeFileHandle/GetFileType.Windows.cs:160

• This test verifies stability after CanSeek is evaluated, but it still only checks equality. Add an explicit assertion for the expected value to ensure the test fails if CanSeek regresses to a consistent-but-wrong result.

 bool canSeek = stream.CanSeek;
Assert.Equal(FileHandleType.SymbolicLink, handle.Type);
Assert.Equal(canSeek, stream.CanSeek);

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-io
See info in area-owners.md if you want to be subscribed.

[adamsitnik]

Overall it LGTM, but I think the tests can be improved to avoid code duplication.

Big thanks for catching the issue and providing the fix @jozkee !

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/SafeFileHandle/GetFileType.Windows.cs:133

• The PR description / validation section references a second regression test (FileStream_SymbolicLink_CanSeek_IsNotAffectedBySubsequentTypeAccess), but the current change only adds FileStream_SymbolicLink_CanSeek_IsNotAffectedByType (with an InlineData parameter covering both access orders). This can make it harder to correlate the stated validation with the actual tests in the diff. Consider updating the PR description (or splitting this into two separately-named tests if that’s the intent).

 [ConditionalTheory(typeof(MountHelper), nameof(MountHelper.CanCreateSymbolicLinks))]
[InlineData(true)]
[InlineData(false)]
public unsafe void FileStream_SymbolicLink_CanSeek_IsNotAffectedByType(bool accessTypeFirst)
{

[adamsitnik]

LGTM, big thanks for spotting the issue and providing the fix @jozkee !

[jozkee]

/ba-g #132030

[jozkee]

Follow-up to #132077.

Summary

• Determine Windows SafeFileHandle.CanSeek directly from the raw GetFileType result instead of the cached, refined SafeFileHandle.Type value.
• Add symbolic-link regression tests covering Type access both before and after FileStream evaluates CanSeek.

Rationale

For handles opened on a symbolic link, Windows reports FILE_TYPE_DISK, while SafeFileHandle.Type refines that to SymbolicLink. As a result, #132077 could cache different CanSeek values depending on whether Type was queried before constructing the FileStream. Using the raw file type restores access-order-independent behavior while retaining the write-only named-pipe fix.

Validation

• .\build.cmd clr.corelib — succeeded with 0 warnings and 0 errors.
• FileStream_SymbolicLink_CanSeek_IsNotAffectedByType — passed.
• FileStream_WriteOnlyNamedPipe_CanSeek_IsAsync — 2 cases passed.

No baseline build was run.

Note

This pull request description was generated with GitHub Copilot.

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[Copilot]

Pull request overview

Updates Windows SafeFileHandle.CanSeek to be derived directly from the raw GetFileType result, avoiding access-order-dependent behavior when SafeFileHandle.Type (which refines disk handles into SymbolicLink, Directory, etc.) is queried before creating a FileStream. Adds Windows regression tests that exercise the symbolic-link scenario.

Changes:

• Compute SafeFileHandle seekability solely from Kernel32.GetFileType(handle) == FILE_TYPE_DISK.
• Add regression tests validating FileStream.CanSeek is unaffected by prior SafeFileHandle.Type access on symlink handles.
• Add a helper to open a symlink as a reparse point handle for the new tests.

Reviewed changes

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

File	Description
src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Windows.cs	Makes CanSeek determination independent of the refined cached handle Type by using raw GetFileType.
src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/SafeFileHandle/GetFileType.Windows.cs	Adds Windows symbolic-link regression tests covering Type access order relative to FileStream.CanSeek.

Suppressed comments (1)

src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/SafeFileHandle/GetFileType.Windows.cs:160

• This test verifies stability after CanSeek is evaluated, but it still only checks equality. Add an explicit assertion for the expected value to ensure the test fails if CanSeek regresses to a consistent-but-wrong result.

 bool canSeek = stream.CanSeek;
Assert.Equal(FileHandleType.SymbolicLink, handle.Type);
Assert.Equal(canSeek, stream.CanSeek);

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-io
See info in area-owners.md if you want to be subscribed.

[adamsitnik]

Overall it LGTM, but I think the tests can be improved to avoid code duplication.

Big thanks for catching the issue and providing the fix @jozkee !

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/libraries/System.Runtime/tests/System.IO.FileSystem.Tests/SafeFileHandle/GetFileType.Windows.cs:133

• The PR description / validation section references a second regression test (FileStream_SymbolicLink_CanSeek_IsNotAffectedBySubsequentTypeAccess), but the current change only adds FileStream_SymbolicLink_CanSeek_IsNotAffectedByType (with an InlineData parameter covering both access orders). This can make it harder to correlate the stated validation with the actual tests in the diff. Consider updating the PR description (or splitting this into two separately-named tests if that’s the intent).

 [ConditionalTheory(typeof(MountHelper), nameof(MountHelper.CanCreateSymbolicLinks))]
[InlineData(true)]
[InlineData(false)]
public unsafe void FileStream_SymbolicLink_CanSeek_IsNotAffectedByType(bool accessTypeFirst)
{

[adamsitnik]

LGTM, big thanks for spotting the issue and providing the fix @jozkee !

[jozkee]

/ba-g #132030