Merge pull request #3039 from MichaelEischer/tar-proper-dirs

Properly dump directories to tar

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

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)
+			}
 		})
 	}
 }

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)

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)
 			}
-
+			if !strings.HasSuffix(hdr.Name, "/") {
-		} else {
+				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())
 			}