Fix failing symlink tests (9.0)#130343
Conversation
|
Tagging subscribers to this area: @dotnet/area-system-formats-tar |
There was a problem hiding this comment.
Pull request overview
Backport to release/9.0-staging that fixes TarEntry.FilePathEscapesDirectory so extraction correctly detects symlink-based directory escapes without throwing, and re-enables previously quarantined symlink traversal tests.
Changes:
- Update
FilePathEscapesDirectoryto separate logical vs. physical path handling and add a prefix guard before computing a relative path. - Adjust symlink resolution to avoid treating non-links/dangling links incorrectly during path-walk checks.
- Remove
[ActiveIssue(#129227)]from three symlink-related extraction tests now that the underlying bug is addressed.
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/src/System/Formats/Tar/TarEntry.cs | Fixes symlink escape detection logic to avoid Substring OOR and correctly walk/resolve paths. |
| src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectory.File.Tests.cs | Re-enables symlink traversal tests by removing the ActiveIssue suppressions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… symlink tests When an entry resolves to the destination directory itself, the relative path computation did an out-of-range Substring (logicalPrefix includes a trailing separator). Return an empty relative path in that case. This also fixes the ArgumentOutOfRangeException seen on macOS where the physical destination path is longer than the logical one. Remove the [ActiveIssue] dotnet#129227 tags now that the affected tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Hi, the code complete date for 9.0.19 (the August 2026 release) is Monday 2026-07-13. Make sure to merge this PR on that date at the latest, or it won't make it into that release. As a reminder, to merge it, you need Tactics approval, the PR needs to be approved by another area co-owner and there can't be any CI failures that could be related to the fix. |
|
Got email approval to merge |
|
/ba-g test failures not related |
9f1e437
into
dotnet:release/9.0-staging
Backport of #130342 to .NET 9.
Fixes #129227
Customer Impact
The new tar symlink resolution is not working properly on some platforms.
Corrects a length mismatch that made the Tar traversal guard crash with ArgumentOutOfRangeException (subtracting a symlink-resolved path length from a logical path). Without it, extraction to any symlinked destination (macOS /var , /tmp ) is broken.
Regression
Testing
Reenabled failing CI tests.
Risk
Low.