Skip to content
This repository has been archived by the owner on Nov 19, 2024. It is now read-only.

Add support for storing symlinks in tar and zip archives #92

Merged
merged 6 commits into from
Dec 4, 2018
Merged

Add support for storing symlinks in tar and zip archives #92

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
aee7070
Add support for storing symlinks in tar and zip archives
jandubois Nov 20, 2018
8c26246
Configure git symlink support on Appveyor
jandubois Dec 4, 2018
e178912
Move zip symlink writer from extractNext to extractFile
jandubois Dec 4, 2018
9c31240
Code rearrangement for codereview feedback
jandubois Dec 4, 2018
35c90ea
Slight refactor of symlink handling
mholt Dec 4, 2018
308684e
Set link target only if symlink
mholt Dec 4, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions appveyor.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
# set git config to clone with symlinks enabled on windows
init:
- git config --global core.symlinks true

version: "{build}"

clone_folder: c:\gopath\src\github.com\mholt\archiver
Expand Down
32 changes: 26 additions & 6 deletions archiver_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -343,19 +343,39 @@ func symmetricTest(t *testing.T, formatName, dest string) {
return nil
}

expectedFileInfo, err := os.Stat(origPath)
expectedFileInfo, err := os.Lstat(origPath)
if err != nil {
t.Fatalf("[%s] %s: Error obtaining original file info: %v", formatName, fpath, err)
}
expected, err := ioutil.ReadFile(origPath)
actualFileInfo, err := os.Lstat(fpath)
if err != nil {
t.Fatalf("[%s] %s: Couldn't open original file (%s) from disk: %v", formatName,
fpath, origPath, err)
t.Fatalf("[%s] %s: Error obtaining actual file info: %v", formatName, fpath, err)
}

actualFileInfo, err := os.Stat(fpath)
if actualFileInfo.Mode() != expectedFileInfo.Mode() {
t.Fatalf("[%s] %s: File mode differed between on disk and compressed", formatName,
expectedFileInfo.Mode().String()+" : "+actualFileInfo.Mode().String())
}

if (actualFileInfo.Mode() & os.ModeSymlink) != 0 {
expectedLinkTarget, err := os.Readlink(origPath)
if err != nil {
t.Fatalf("[%s] %s: Couldn't read original symlink target: %v", formatName, origPath, err)
}
actualLinkTarget, err := os.Readlink(fpath)
if err != nil {
t.Fatalf("[%s] %s: Couldn't read actual symlink target: %v", formatName, fpath, err)
}
if expectedLinkTarget != actualLinkTarget {
t.Fatalf("[%s] %s: Symlink targets differed between on disk and compressed", formatName, origPath)
}
return nil
}

expected, err := ioutil.ReadFile(origPath)
if err != nil {
t.Fatalf("[%s] %s: Error obtaining actual file info: %v", formatName, fpath, err)
t.Fatalf("[%s] %s: Couldn't open original file (%s) from disk: %v", formatName,
fpath, origPath, err)
}
actual, err := ioutil.ReadFile(fpath)
if err != nil {
Expand Down
22 changes: 16 additions & 6 deletions tar.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -285,12 +285,14 @@ func (t *Tar) writeWalk(source, topLevelFolder, destination string) error {
return handleErr(err)
}

file, err := os.Open(fpath)
if err != nil {
return handleErr(fmt.Errorf("%s: opening: %v", fpath, err))
var file io.ReadCloser
if info.Mode().IsRegular() {
file, err = os.Open(fpath)
if err != nil {
return handleErr(fmt.Errorf("%s: opening: %v", fpath, err))
}
defer file.Close()
}
defer file.Close()

err = t.Write(File{
FileInfo: FileInfo{
FileInfo: info,
Expand Down Expand Up @@ -338,7 +340,15 @@ func (t *Tar) Write(f File) error {
return fmt.Errorf("missing file name")
}

hdr, err := tar.FileInfoHeader(f, f.Name())
var linkTarget string
if (f.Mode() & os.ModeSymlink) != 0 {
var err error
linkTarget, err = os.Readlink(f.Name())
if err != nil {
return fmt.Errorf("%s: readlink: %v", f.Name(), err)
}
}
hdr, err := tar.FileInfoHeader(f, filepath.ToSlash(linkTarget))
if err != nil {
return fmt.Errorf("%s: making header: %v", f.Name(), err)
}
Expand Down
1 change: 1 addition & 0 deletions testdata/exist
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
/target/does/not/exist
1 change: 1 addition & 0 deletions testdata/proverb3.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
proverbs/extra/proverb3.txt
33 changes: 28 additions & 5 deletions zip.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,15 @@ func (z *Zip) extractNext(to string) error {
if !ok {
return fmt.Errorf("expected header to be zip.FileHeader but was %T", f.Header)
}
if (header.FileInfo().Mode() & os.ModeSymlink) != 0 {
buffer := make([]byte, header.FileInfo().Size())
Copy link
Owner

Choose a reason for hiding this comment

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

This seems to be very different from how it works in tar archives; while I know that this PR writes the target filepath as the symlink's contents, is it safe to assume that every zip archiver will do that? Is there risk here of reading in a potentially huge file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not 100% sure; I simply went by the symlink code in archive/zip test: https://github.com/golang/go/blob/b0a53d2/src/archive/zip/writer_test.go#L54-L59

I believe this is what Info-ZIP refers to as the "generic symlink support" in the later versions. Earlier versions (and PKZIP) had platform specific extensions for symlinks (so symlinks archived on one platform could not be extracted on another one), but I think those have been abandoned.

I'm not sure if this is necessary, but I can add a check for some reasonable maximum path length, if you prefer. If you want that, what should the limit be? 2^16 seems to be way beyond what I would expect to see for real pathnames, but small enough not to cause memory issues...

Copy link
Owner

@mholt mholt Dec 4, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Hmm, I suppose this is OK then (no need for size check at this time), but I would probably want to move this logic to the z.extractFile method, if possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I put the logic into extractNext because it already had done the type assertion on header, but you are right, it must be in extractFile, otherwise Extract will not be able to extract symlinks. I'll do this (and duplicate the header assertion into extractFile).

Unfortunately this points out that test coverage of the Extract method is lacking. 😦

size, err := f.Read(buffer)
if err != nil && err != io.EOF {
return fmt.Errorf("%s: reading symlink target: %v", header.Name, err)
}
linkTarget := string(buffer[:size])
return writeNewSymbolicLink(filepath.Join(to, header.Name), linkTarget)
}
return z.extractFile(f, filepath.Join(to, header.Name))
}

Expand Down Expand Up @@ -244,12 +253,14 @@ func (z *Zip) writeWalk(source, topLevelFolder, destination string) error {
return handleErr(err)
}

file, err := os.Open(fpath)
if err != nil {
return handleErr(fmt.Errorf("%s: opening: %v", fpath, err))
var file io.ReadCloser
if info.Mode().IsRegular() {
file, err = os.Open(fpath)
if err != nil {
return handleErr(fmt.Errorf("%s: opening: %v", fpath, err))
}
defer file.Close()
}
defer file.Close()

err = z.Write(File{
FileInfo: FileInfo{
FileInfo: info,
Expand Down Expand Up @@ -317,6 +328,18 @@ func (z *Zip) Write(f File) error {
return nil
}

if (header.Mode() & os.ModeSymlink) != 0 {
mholt marked this conversation as resolved.
Show resolved Hide resolved
linkTarget, err := os.Readlink(f.Name())
if err != nil {
return fmt.Errorf("%s: readlink: %v", f.Name(), err)
}
_, err = writer.Write([]byte(filepath.ToSlash(linkTarget)))
if err != nil {
return fmt.Errorf("%s: writing symlink target: %v", f.Name(), err)
}
return nil
}

if header.Mode().IsRegular() {
if f.ReadCloser == nil {
return fmt.Errorf("%s: no way to read file contents", f.Name())
Expand Down