Skip to content

Conversation

@tmds
Copy link
Member

@tmds tmds commented Apr 26, 2023

Fixes #78695.

@dotnet/area-system-io ptal.

@ghost ghost added community-contribution Indicates that the PR has been added by a community member area-System.Formats.Tar labels Apr 26, 2023
@ghost
Copy link

ghost commented Apr 26, 2023

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

Issue Details

Fixes #78695.

@dotnet/area-system-io ptal.

Author: tmds
Assignees: -
Labels:

community-contribution, area-System.Formats.Tar

Milestone: -

@tmds
Copy link
Member Author

tmds commented Jun 21, 2023

This PR includes changes for 3 issues: #78695, #87835 and #87463.

@tmds
Copy link
Member Author

tmds commented Jun 22, 2023

@carlossanlop @jeffhandley @dotnet/area-system-io-compression can we try to get this reviewed in the coming weeks?

@jeffhandley
Copy link
Member

@jozkee said he can take a look

}
else if (EntryType is TarEntryType.HardLink)
{
// LinkName is path relative to the destinationDirectoryPath.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How can we assume that LinkName will never be an absolute path for hard links?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're using Path.Join to interpret the LinkName as a relative path.

Copy link
Member

@jozkee jozkee Jun 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

if (linkTargetPath == null)
{
throw new IOException(SR.Format(SR.TarExtractingResultsLinkOutside, linkName, destinationDirectoryPath));
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@tmds
Copy link
Member Author

tmds commented Jun 26, 2023

@jozkee thanks for the review! I've updated the PR based on your feedback. ptal.

@jozkee jozkee closed this Jun 27, 2023
@jozkee jozkee reopened this Jun 27, 2023
Copy link
Member

@jozkee jozkee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-System.Formats.Tar community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TarEntry: link target path checks

4 participants