Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tar: behavior for symbolic links to directories #74214

Closed
tmds opened this issue Aug 19, 2022 · 11 comments · Fixed by #74376
Closed

Tar: behavior for symbolic links to directories #74214

tmds opened this issue Aug 19, 2022 · 11 comments · Fixed by #74376
Assignees
Milestone

Comments

@tmds
Copy link
Member

tmds commented Aug 19, 2022

Unlike tar, currently these links are being followed.
If the file linked to is in the archive it will be duplicated. If it is outside the archived directory it will be included as if it were.
Symbolic links to directories can also cause loops which will cause the archive creation to hang.

Can we behave like tar and not follow symlinks by default?
tar does provide an option to follow the links (--dereference).

cc @carlossanlop @dotnet/area-system-io

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Aug 19, 2022
@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@ghost
Copy link

ghost commented Aug 19, 2022

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

Issue Details

Unlike tar, currently these links are being followed.
If the file linked to is in the archive it will be duplicated. If it is outside the archived directory it will be included as if it were.
Symbolic links to directories can also cause loops which will cause the archive creation to hang.

Can we behave like tar and not follow symlinks by default?
tar does provide an option to follow the links (--dereference).

cc @carlossanlop @dotnet/area-system-io

Author: tmds
Assignees: -
Labels:

area-System.IO, untriaged

Milestone: -

@jozkee
Copy link
Member

jozkee commented Aug 19, 2022

@carlossanlop did we deliberately choose to follow, and if so, what was the reason for it?

@jozkee jozkee added this to the 7.0.0 milestone Aug 19, 2022
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Aug 19, 2022
@carlossanlop carlossanlop self-assigned this Aug 22, 2022
@carlossanlop
Copy link
Member

carlossanlop commented Aug 22, 2022

The TarFile.ExtractToDirectory TarFile.CreateFromDirectory methods should treat symlinks without following them. I'll submit a fix.

Edit: Corrected below.

@tmds
Copy link
Member Author

tmds commented Aug 22, 2022

@carlossanlop the issue about creation of the archive.

@carlossanlop
Copy link
Member

carlossanlop commented Aug 22, 2022

the issue about creation of the archive.

Oh I see. Re-reading the issue description, I made the assumption it was about extraction. Looking at the code again, extraction won't have a problem with following a symlink. The symlink will get created pointing at a destination, regardless if it exists or not.

We can fix for archive creation.

@carlossanlop
Copy link
Member

Fixing this would require implementing the logic proposed in #52666

Due to the amount of work and the API proposal dependency, this won't make it to 7.0, so I'll move it to 8.0.

We may have to also fix this in ZipFile.CreateFromDirectory: https://github.com/dotnet/runtime/blob/main/src/libraries/System.IO.Compression.ZipFile/src/System/IO/Compression/ZipFile.Create.cs#L378

@carlossanlop carlossanlop modified the milestones: 7.0.0, 8.0.0 Aug 22, 2022
@tmds
Copy link
Member Author

tmds commented Aug 22, 2022

It should be possible to implement this using RecursePredicate or ShouldRecurseIntoEntry.

If this is pushed to 8.0 it will be a breaking change.

@carlossanlop
Copy link
Member

OK let me try.

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Aug 22, 2022
@carlossanlop carlossanlop modified the milestones: 8.0.0, 7.0.0 Aug 22, 2022
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Aug 23, 2022
@lewing
Copy link
Member

lewing commented Aug 23, 2022

/backport to release/7.0-rc1

@carlossanlop
Copy link
Member

carlossanlop commented Aug 23, 2022

@lewing I'm including this in this draft: #74414

We can close the automated one.

Edit: Ah, the command won't work on issues 😄 .

@ghost ghost locked as resolved and limited conversation to collaborators Sep 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants