diff --git a/changelog/unreleased/issue-2319 b/changelog/unreleased/issue-2319 new file mode 100644 index 000000000..90f42a302 --- /dev/null +++ b/changelog/unreleased/issue-2319 @@ -0,0 +1,12 @@ +Bugfix: Correctly dump directories into tar files + +The dump command previously wrote directories in a tar file in a way which +can cause compatibility problems. This caused, for example, 7zip on Windows +to not open tar files containing directories. In addition it was not possible +to dump directories with extended attributes. These compatibility problems +are now corrected. + +In addition, a tar file now includes the name of the owner and group of a file. + +https://github.com/restic/restic/issues/2319 +https://github.com/restic/restic/pull/3039 diff --git a/internal/dump/acl_test.go b/internal/dump/acl_test.go index fe930c986..bef11ad14 100644 --- a/internal/dump/acl_test.go +++ b/internal/dump/acl_test.go @@ -21,6 +21,13 @@ func Test_acl_decode(t *testing.T) { }, want: "user::rw-\nuser:0:rwx\nuser:65534:rwx\ngroup::rwx\nmask::rwx\nother::r--\n", }, + { + name: "decode group", + args: args{ + xattr: []byte{2, 0, 0, 0, 8, 0, 1, 0, 254, 255, 0, 0}, + }, + want: "group:65534:--x\n", + }, { name: "decode fail", args: args{ @@ -28,6 +35,13 @@ func Test_acl_decode(t *testing.T) { }, want: "", }, + { + name: "decode empty fail", + args: args{ + xattr: []byte(""), + }, + want: "", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -36,6 +50,10 @@ func Test_acl_decode(t *testing.T) { if tt.want != a.String() { t.Errorf("acl.decode() = %v, want: %v", a.String(), tt.want) } + a.decode(tt.args.xattr) + if tt.want != a.String() { + t.Errorf("second acl.decode() = %v, want: %v", a.String(), tt.want) + } }) } } diff --git a/internal/dump/tar.go b/internal/dump/tar.go index 86786654a..816a0ffcf 100644 --- a/internal/dump/tar.go +++ b/internal/dump/tar.go @@ -4,6 +4,7 @@ import ( "archive/tar" "context" "io" + "os" "path" "path/filepath" "strings" @@ -65,6 +66,15 @@ func tarTree(ctx context.Context, repo restic.Repository, rootNode *restic.Node, return err } +// copied from archive/tar.FileInfoHeader +const ( + // Mode constants from the USTAR spec: + // See http://pubs.opengroup.org/onlinepubs/9699919799/utilities/pax.html#tag_20_92_13_06 + c_ISUID = 04000 // Set uid + c_ISGID = 02000 // Set gid + c_ISVTX = 01000 // Save text (sticky bit) +) + func tarNode(ctx context.Context, tw *tar.Writer, node *restic.Node, repo restic.Repository) error { relPath, err := filepath.Rel("/", node.Path) if err != nil { @@ -74,15 +84,32 @@ func tarNode(ctx context.Context, tw *tar.Writer, node *restic.Node, repo restic header := &tar.Header{ Name: filepath.ToSlash(relPath), Size: int64(node.Size), - Mode: int64(node.Mode), + Mode: int64(node.Mode.Perm()), // c_IS* constants are added later Uid: int(node.UID), Gid: int(node.GID), + Uname: node.User, + Gname: node.Group, ModTime: node.ModTime, AccessTime: node.AccessTime, ChangeTime: node.ChangeTime, PAXRecords: parseXattrs(node.ExtendedAttributes), } + // adapted from archive/tar.FileInfoHeader + if node.Mode&os.ModeSetuid != 0 { + header.Mode |= c_ISUID + } + if node.Mode&os.ModeSetgid != 0 { + header.Mode |= c_ISGID + } + if node.Mode&os.ModeSticky != 0 { + header.Mode |= c_ISVTX + } + + if IsFile(node) { + header.Typeflag = tar.TypeReg + } + if IsLink(node) { header.Typeflag = tar.TypeSymlink header.Linkname = node.LinkTarget @@ -90,6 +117,7 @@ func tarNode(ctx context.Context, tw *tar.Writer, node *restic.Node, repo restic if IsDir(node) { header.Typeflag = tar.TypeDir + header.Name += "/" } err = tw.WriteHeader(header) diff --git a/internal/dump/tar_test.go b/internal/dump/tar_test.go index a8ce336fd..2ef98ffe0 100644 --- a/internal/dump/tar_test.go +++ b/internal/dump/tar_test.go @@ -9,6 +9,7 @@ import ( "io/ioutil" "os" "path/filepath" + "strings" "testing" "time" @@ -68,6 +69,14 @@ func TestWriteTar(t *testing.T) { }, target: "/", }, + { + name: "file and symlink in root", + args: archiver.TestDir{ + "file1": archiver.TestFile{Content: "string"}, + "file2": archiver.TestSymlink{Target: "file1"}, + }, + target: "/", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -128,7 +137,7 @@ func checkTar(t *testing.T, testDir string, srcTar *bytes.Buffer) error { } matchPath := filepath.Join(testDir, hdr.Name) - match, err := os.Stat(matchPath) + match, err := os.Lstat(matchPath) if err != nil { return err } @@ -140,7 +149,12 @@ func checkTar(t *testing.T, testDir string, srcTar *bytes.Buffer) error { return fmt.Errorf("modTime does not match, got: %s, want: %s", fileTime, tarTime) } - if hdr.Typeflag == tar.TypeDir { + if os.FileMode(hdr.Mode).Perm() != match.Mode().Perm() || os.FileMode(hdr.Mode)&^os.ModePerm != 0 { + return fmt.Errorf("mode does not match, got: %v, want: %v", os.FileMode(hdr.Mode), match.Mode()) + } + + switch hdr.Typeflag { + case tar.TypeDir: // this is a folder if hdr.Name == "." { // we don't need to check the root folder @@ -151,8 +165,18 @@ func checkTar(t *testing.T, testDir string, srcTar *bytes.Buffer) error { if filepath.Base(hdr.Name) != filebase { return fmt.Errorf("foldernames don't match got %v want %v", filepath.Base(hdr.Name), filebase) } - - } else { + if !strings.HasSuffix(hdr.Name, "/") { + return fmt.Errorf("foldernames must end with separator got %v", hdr.Name) + } + case tar.TypeSymlink: + target, err := fs.Readlink(matchPath) + if err != nil { + return err + } + if target != hdr.Linkname { + return fmt.Errorf("symlink target does not match, got %s want %s", target, hdr.Linkname) + } + default: if match.Size() != hdr.Size { return fmt.Errorf("size does not match got %v want %v", hdr.Size, match.Size()) }