Fix TarWriter treating non-symlink reparse points as symbolic links on Windows#124753
Open
rzikm wants to merge 1 commit intodotnet:mainfrom
Open
Fix TarWriter treating non-symlink reparse points as symbolic links on Windows#124753rzikm wants to merge 1 commit intodotnet:mainfrom
rzikm wants to merge 1 commit intodotnet:mainfrom
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a critical bug where TarWriter on Windows incorrectly classified all reparse points (including data deduplication, OneDrive placeholders, and AppExecLinks) as symbolic links. The fix leverages the FileSystemInfo.LinkTarget property which only returns a non-null value for true symbolic links (IO_REPARSE_TAG_SYMLINK) and junctions (IO_REPARSE_TAG_MOUNT_POINT), allowing other reparse point types to be correctly classified as regular files or directories.
Changes:
- Modified
TarWriter.Windows.csto checkLinkTargetproperty instead of just the ReparsePoint attribute - Extracted
GetRegularFileEntryTypeForFormathelper toTarHelpers.csto eliminate duplication - Added comprehensive tests for junctions and non-symlink reparse points (using AppExecLinks)
- Moved
GetAppExecLinkPathutility to sharedReparsePointUtilities.csfor reuse across test projects
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| TarWriter.Windows.cs | Core fix: checks LinkTarget is not null to distinguish symlinks/junctions from other reparse points; caches LinkTarget value to avoid redundant I/O |
| TarHelpers.cs | Adds GetRegularFileEntryTypeForFormat helper method to centralize V7 vs regular file type selection logic |
| TarWriter.Unix.cs | Refactored to use new helper method; removed unused using directives |
| TarWriter.WriteEntry.File.Tests.Windows.cs | New test file with junction test and non-symlink reparse point test (sync version) |
| TarWriter.WriteEntryAsync.File.Tests.Windows.cs | New test file with junction test and non-symlink reparse point test (async version) |
| System.Formats.Tar.Tests.csproj | Registers new Windows-specific test files |
| TarTestsBase.cs | Adds GetRegularFileEntryTypeForFormat helper for test code reuse |
| Various test files | Replace inline ternary expressions with GetRegularFileEntryTypeForFormat helper for consistency |
| ReparsePointUtilities.cs | Adds GetAppExecLinkPath method migrated from FileSystem tests for cross-project reuse |
| File/FileInfo SymbolicLinks.cs | Updated to use MountHelper.GetAppExecLinkPath() instead of local method |
| BaseSymbolicLinks.FileSystem.cs | Removed GetAppExecLinkPath (migrated to shared utilities); removed unused using directive |
…n Windows Only treat reparse points as symbolic links when FileSystemInfo.LinkTarget returns a non-null value, indicating the reparse point is a true symlink or junction. Other reparse points (e.g., deduplication, OneDrive) now fall through to be handled as regular files or directories. Added GetRegularFileEntryTypeForFormat helper to reduce duplication of the V7/regular file entry type check pattern across source and test files. Moved GetAppExecLinkPath to shared ReparsePointUtilities so it can be used by both FileSystem and Tar tests. Added tests verifying junctions are correctly written as symbolic link entries and that non-symlink reparse points are not misidentified. Fixes dotnet#82949 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
a3c53f0 to
fff18c6
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix #82949
On Windows,
TarWriterincorrectly classified all reparse points (including data deduplication, OneDrive placeholders, and AppExecLinks) as symbolic links. This caused issues when archiving files on volumes with data deduplication enabled, or files managed by OneDrive.Changes
The fix uses
FileSystemInfo.LinkTargetto check whether a reparse point is a true symbolic link or junction before classifying it asTarEntryType.SymbolicLink.LinkTargetinternally usesDeviceIoControl(FSCTL_GET_REPARSE_POINT)and only returns a non-null value forIO_REPARSE_TAG_SYMLINKandIO_REPARSE_TAG_MOUNT_POINTreparse tags. All other reparse points (dedup, OneDrive, AppExecLinks, etc.) now fall through to be classified based on their other attributes (regular file or directory).This is a simpler alternative to the approach in #89102 which required adding multiple interop files for
FindFirstFile/WIN32_FIND_DATA.Source changes:
TarWriter.Windows.cs: Checkinfo.LinkTarget is not nullbefore classifying a reparse point as a symbolic link. Cache theLinkTargetvalue to avoid redundant I/O.TarHelpers.cs: ExtractGetRegularFileEntryTypeForFormathelper to reduce duplication between Windows and Unix implementations.TarWriter.Unix.cs: Use the new helper; remove unusedusingdirectives.Test changes:
Add_Junction_As_SymbolicLink) verifying that Windows junctions are correctly written as symbolic link entries.Add_Non_Symlink_ReparsePoint_Does_Not_Write_As_SymbolicLink) using AppExecLinks to verify non-symlink reparse points are not classified as symbolic links.GetAppExecLinkPathto sharedReparsePointUtilities.csfor reuse across test projects.format is TarEntryFormat.V7 ? ...ternary expressions withGetRegularFileEntryTypeForFormathelper in test files.