From e21dcb0eeab88397abc5ec2a07b7e19eba9d8a22 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sat, 24 Oct 2020 18:26:30 +0200 Subject: [PATCH 1/7] dump: Additional ACL tests --- internal/dump/acl_test.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) 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) + } }) } } From 1e3c9a2c11552a995d1f42c336f5c1edb30a582c Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sat, 24 Oct 2020 18:38:30 +0200 Subject: [PATCH 2/7] dump: Fix file permission to tar mapping The file permissions included a go specific directory bit which accidentially forced the usage of the GNU header format. This leads to problems with 7zip on Windows or when extended attributes are used. --- internal/dump/tar.go | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/internal/dump/tar.go b/internal/dump/tar.go index 86786654a..fe08e8825 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,7 +84,7 @@ 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), ModTime: node.ModTime, @@ -83,6 +93,21 @@ func tarNode(ctx context.Context, tw *tar.Writer, node *restic.Node, repo restic 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 +115,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) From fe09e6f86589cabafac1913de9f8aacd0399c9e6 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sat, 24 Oct 2020 18:27:19 +0200 Subject: [PATCH 3/7] dump: test proper permissions and directory name --- internal/dump/tar_test.go | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/internal/dump/tar_test.go b/internal/dump/tar_test.go index a8ce336fd..e5b0385be 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,6 +149,10 @@ 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 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()) + } + if hdr.Typeflag == tar.TypeDir { // this is a folder if hdr.Name == "." { @@ -151,7 +164,17 @@ 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, "/") { + return fmt.Errorf("foldernames must end with separator got %v", hdr.Name) + } + } else if hdr.Typeflag == 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) + } } else { if match.Size() != hdr.Size { return fmt.Errorf("size does not match got %v want %v", hdr.Size, match.Size()) From 8d7d6ad2d5dead0896e3062409a24c0513977232 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sat, 24 Oct 2020 18:39:03 +0200 Subject: [PATCH 4/7] dump: include username in tar --- internal/dump/tar.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/internal/dump/tar.go b/internal/dump/tar.go index fe08e8825..816a0ffcf 100644 --- a/internal/dump/tar.go +++ b/internal/dump/tar.go @@ -87,6 +87,8 @@ func tarNode(ctx context.Context, tw *tar.Writer, node *restic.Node, repo restic 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, From 1aa61e6def3126e509d509be107385f59d21b252 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sat, 24 Oct 2020 22:49:29 +0200 Subject: [PATCH 5/7] Add changelog --- changelog/unreleased/issue-2319 | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 changelog/unreleased/issue-2319 diff --git a/changelog/unreleased/issue-2319 b/changelog/unreleased/issue-2319 new file mode 100644 index 000000000..d36858bc8 --- /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 open tar files containing directories. In addition it was not possible +to dump directories with extended attributes. These compatibility problems +were fixed by properly setting the attributes of a directory. + +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 From 04dfa19c7ee4504e4a2eeaae921df61257f67ab2 Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Mon, 2 Nov 2020 11:23:09 +0100 Subject: [PATCH 6/7] Improve changelog --- changelog/unreleased/issue-2319 | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/changelog/unreleased/issue-2319 b/changelog/unreleased/issue-2319 index d36858bc8..90f42a302 100644 --- a/changelog/unreleased/issue-2319 +++ b/changelog/unreleased/issue-2319 @@ -1,12 +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 open tar files containing directories. In addition it was not possible +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 -were fixed by properly setting the attributes of a directory. +are now corrected. -In addition a tar file now includes the name of the owner and group of a file. +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 From 5f0fa2129e509a5f0eb249db2b149b72311202a8 Mon Sep 17 00:00:00 2001 From: Alexander Neumann Date: Mon, 2 Nov 2020 11:24:26 +0100 Subject: [PATCH 7/7] Improve readability It's time to use a switch statement. --- internal/dump/tar_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/internal/dump/tar_test.go b/internal/dump/tar_test.go index e5b0385be..2ef98ffe0 100644 --- a/internal/dump/tar_test.go +++ b/internal/dump/tar_test.go @@ -153,7 +153,8 @@ func checkTar(t *testing.T, testDir string, srcTar *bytes.Buffer) error { return fmt.Errorf("mode does not match, got: %v, want: %v", os.FileMode(hdr.Mode), match.Mode()) } - if hdr.Typeflag == tar.TypeDir { + switch hdr.Typeflag { + case tar.TypeDir: // this is a folder if hdr.Name == "." { // we don't need to check the root folder @@ -167,7 +168,7 @@ func checkTar(t *testing.T, testDir string, srcTar *bytes.Buffer) error { if !strings.HasSuffix(hdr.Name, "/") { return fmt.Errorf("foldernames must end with separator got %v", hdr.Name) } - } else if hdr.Typeflag == tar.TypeSymlink { + case tar.TypeSymlink: target, err := fs.Readlink(matchPath) if err != nil { return err @@ -175,7 +176,7 @@ func checkTar(t *testing.T, testDir string, srcTar *bytes.Buffer) error { if target != hdr.Linkname { return fmt.Errorf("symlink target does not match, got %s want %s", target, hdr.Linkname) } - } else { + default: if match.Size() != hdr.Size { return fmt.Errorf("size does not match got %v want %v", hdr.Size, match.Size()) }