Fix failing symlink tests#130342
Conversation
|
Tagging subscribers to this area: @dotnet/area-system-formats-tar |
There was a problem hiding this comment.
Pull request overview
This PR re-enables previously skipped symlink-related TarFile.ExtractToDirectory tests and updates tar extraction path validation to better detect directory traversal via symlinks created by earlier archive entries.
Changes:
- Removed
ActiveIssueskips for symlink traversal tests so they run again on macOS (and Windows for the chained traversal test). - Updated
TarEntry.FilePathEscapesDirectoryto compute a logical relative path first and to resolve symlinks during component-by-component traversal (including dangling links).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectory.File.Tests.cs | Re-enables symlink traversal tests by removing ActiveIssue skips. |
| src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarEntry.cs | Adjusts path-escape detection logic to better account for symlink resolution during extraction. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Walk relative components, resolving symlinks at each step | ||
| string relative = normalizedFile.Substring(resolvedDest.Length) | ||
| string relative = normalizedFile.Substring(logicalPrefix.Length) | ||
| .TrimStart(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar); |
| [Theory] | ||
| [InlineData(true)] | ||
| [InlineData(false)] | ||
| [ActiveIssue("https://github.com/dotnet/runtime/issues/129227", TestPlatforms.OSX)] |
There was a problem hiding this comment.
There are still several tests with this ActiveIssue, in this file and several others. These should be re-enabled as well.
I think the same applies to the other branches as well, but I didn't check those.
No description provided.