-
Notifications
You must be signed in to change notification settings - Fork 239
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
decompressor_tgz: handle symlinks and links (hardlinks) #37
Conversation
This adds support for re-creating symlinks after untaring tgz archives.
This adds support for re-creating links after untaring tgz archives.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pearkes Looks like you are generally on the right path. I left a few comments which address 1) and 3). For 2), maybe we implement some internal helpers, like makeLinks(map[string]string) error
to start. Then you just accumulate the mapping for hard links and pass it in after you've decompressed everything, and it can be applied to all the decompressors. It would make the tests easier as well since the linker could be tested in isolation.
if (hdr.Typeflag == tar.TypeLink) { | ||
// If the type is a link ("hardlink") we re-write it and | ||
// continue instead of attempting to copy the contents | ||
if err := os.Link(hdr.Linkname, path); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you may need to accumulate the hard links and iterate over them after unpacking everything. If the hard link comes before the target lexically, this might not work. It's fine for symlinks since they are allowed to be broken. It would be worth it to try that in the test fixtures as well.
@@ -61,6 +61,22 @@ func TestTarGzipDecompressor(t *testing.T) { | |||
multiplePaths, | |||
"", | |||
}, | |||
|
|||
{ | |||
"with-symlinks.tar.gz", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test is still useful but we should definitely augment and check that the symlink actually got written as a symlink (instead of just contents being duplicated).
👋 I started playing around with symlink, hardlink, and device node (:grimacing:) support in a fork because an approach we were taking for LXC in nomad required it. At first I was afraid supporting links posed a security issue for nomad* -- however it's no different from the existing security issue of absolute paths in archives. So! Here's my thinking:
* eg tar contains symlink |
Closing because this pr has been replaced by #192 |
This adds support for symlinks and hardlinks being unpacked from
tar.gz
archives go-getter processes. Previously, these files would be copied from their empty contents from the targettar.gz
to the destination file system.I have a few questions:
TestDecompressCase
test case to accept multiple MD5s for comparison, or maybe just write an individual test case outside of the table driven style to test this?