TarEntry: remove some unneeded checks when extracting symbolic and hard links. #85378
Conversation
ghost
added
the
community-contribution
|
Tagging subscribers to this area: @dotnet/area-system-formats-tar Issue DetailsFixes #78695. @dotnet/area-system-io ptal.
|
src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarEntry.cs
Outdated
Show resolved
Hide resolved
The LinkName for hard links is a path relative to the archive root.
|
@carlossanlop @jeffhandley @dotnet/area-system-io-compression can we try to get this reviewed in the coming weeks? |
|
@jozkee said he can take a look |
| } | ||
| else if (EntryType is TarEntryType.HardLink) | ||
| { | ||
| // LinkName is path relative to the destinationDirectoryPath. |
There was a problem hiding this comment.
How can we assume that LinkName will never be an absolute path for hard links?
There was a problem hiding this comment.
We're using Path.Join to interpret the LinkName as a relative path.
There was a problem hiding this comment.
Someone could use a TarWriter to manually write a Hard Link entry with LinkName = "C:\my\absolute\path". I don't think is something worth validating because likely no tools would produce it but is something to have in mind.
src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarEntry.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectory.Stream.Tests.cs
Show resolved
Hide resolved
| if (linkTargetPath == null) | ||
| { | ||
| throw new IOException(SR.Format(SR.TarExtractingResultsLinkOutside, linkName, destinationDirectoryPath)); | ||
| } |
There was a problem hiding this comment.
What do we do if the hard link target does not exist at the moment of extraction i.e: on CreateNonRegularFile? I expect that the respective syscall fails.
There was a problem hiding this comment.
Yes it fails, and that is the proper thing to do.
Hard links are detected and handled by the tar writer. It won't include a hard link before the target file.
note: .NET's tar writer doesn't detect hard links and treats them like regular files. There is an open issue to handle them: #74404.
|
@jozkee thanks for the review! I've updated the PR based on your feedback. ptal. |
ghost
locked as resolved and limited conversation to collaborators
Fixes #78695.
@dotnet/area-system-io ptal.