]> Cypherpunks repositories - gostls13.git/commitdiff
archive/tar: disable prefix field in Writer
authorJoe Tsai <joetsai@digital-static.net>
Thu, 27 Oct 2016 23:23:53 +0000 (16:23 -0700)
committerJoe Tsai <thebrokentoaster@gmail.com>
Wed, 2 Nov 2016 20:18:38 +0000 (20:18 +0000)
The proper fix for the Writer is too involved to be done in time
for Go 1.8. Instead, we do a localized fix that simply disables the
prefix encoding logic. While this will prevent some legitimate uses
of prefix, it will ensure that we don't keep outputting invalid
GNU format files that have the prefix field populated.

For headers with long filenames that could have used the prefix field,
they will be promoted to use the PAX format, which ensures that we
will still be able to encode all headers that we were able to do before.

Updates #12594
Fixes #17630
Fixes #9683

Change-Id: Ia97b524ac69865390e2ae8bb0dfb664d40a05add
Reviewed-on: https://go-review.googlesource.com/32234
Reviewed-by: Russ Cox <rsc@golang.org>
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>

src/archive/tar/testdata/ustar.issue12594.tar [new file with mode: 0644]
src/archive/tar/testdata/writer-big-long.tar
src/archive/tar/writer.go
src/archive/tar/writer_test.go

diff --git a/src/archive/tar/testdata/ustar.issue12594.tar b/src/archive/tar/testdata/ustar.issue12594.tar
new file mode 100644 (file)
index 0000000..c7910ae
Binary files /dev/null and b/src/archive/tar/testdata/ustar.issue12594.tar differ
index 5960ee824784ffeacb976a9c648be41b0281508b..52bd748f3b286745455e815c63b6801c3fe06e91 100644 (file)
Binary files a/src/archive/tar/testdata/writer-big-long.tar and b/src/archive/tar/testdata/writer-big-long.tar differ
index bd6e7e5b58388c651a0cd305557271e05ad7ff74..596fb8b9e171f6386daa3792e46ff8359c73af05 100644 (file)
@@ -170,9 +170,41 @@ func (tw *Writer) writeHeader(hdr *Header, allowPax bool) error {
        formatNumeric(ustar.DevMajor(), hdr.Devmajor, paxNone)
        formatNumeric(ustar.DevMinor(), hdr.Devminor, paxNone)
 
+       // TODO(dsnet): The logic surrounding the prefix field is broken when trying
+       // to encode the header as GNU format. The challenge with the current logic
+       // is that we are unsure what format we are using at any given moment until
+       // we have processed *all* of the fields. The problem is that by the time
+       // all fields have been processed, some work has already been done to handle
+       // each field under the assumption that it is for one given format or
+       // another. In some situations, this causes the Writer to be confused and
+       // encode a prefix field when the format being used is GNU. Thus, producing
+       // an invalid tar file.
+       //
+       // As a short-term fix, we disable the logic to use the prefix field, which
+       // will force the badly generated GNU files to become encoded as being
+       // the PAX format.
+       //
+       // As an alternative fix, we could hard-code preferPax to be true. However,
+       // this is problematic for the following reasons:
+       //      * The preferPax functionality is not tested at all.
+       //      * This can result in headers that try to use both the GNU and PAX
+       //      features at the same time, which is also wrong.
+       //
+       // The proper fix for this is to use a two-pass method:
+       //      * The first pass simply determines what set of formats can possibly
+       //      encode the given header.
+       //      * The second pass actually encodes the header as that given format
+       //      without worrying about violating the format.
+       //
+       // See the following:
+       //      https://golang.org/issue/12594
+       //      https://golang.org/issue/17630
+       //      https://golang.org/issue/9683
+       const usePrefix = false
+
        // try to use a ustar header when only the name is too long
        _, paxPathUsed := paxHeaders[paxPath]
-       if !tw.preferPax && len(paxHeaders) == 1 && paxPathUsed {
+       if usePrefix && !tw.preferPax && len(paxHeaders) == 1 && paxPathUsed {
                prefix, suffix, ok := splitUSTARPath(hdr.Name)
                if ok {
                        // Since we can encode in USTAR format, disable PAX header.
index 678254dbc1bb8f22ff9b63ccf8bf51120fdaf159..d88b8f41ca8a8c49ff916dcba20a27b4ac3b17f8 100644 (file)
@@ -133,9 +133,13 @@ func TestWriter(t *testing.T) {
                        contents: strings.Repeat("\x00", 4<<10),
                }},
        }, {
-               // The truncated test file was produced using these commands:
-               //   dd if=/dev/zero bs=1048576 count=16384 > (longname/)*15 /16gig.txt
-               //   tar -b 1 -c -f- (longname/)*15 /16gig.txt | dd bs=512 count=8 > writer-big-long.tar
+               // This truncated file was produced using this library.
+               // It was verified to work with GNU tar 1.27.1 and BSD tar 3.1.2.
+               //  dd if=/dev/zero bs=1G count=16 >> writer-big-long.tar
+               //  gnutar -xvf writer-big-long.tar
+               //  bsdtar -xvf writer-big-long.tar
+               //
+               // This file is in PAX format.
                file: "testdata/writer-big-long.tar",
                entries: []*entry{{
                        header: &Header{
@@ -153,9 +157,15 @@ func TestWriter(t *testing.T) {
                        contents: strings.Repeat("\x00", 4<<10),
                }},
        }, {
-               // This file was produced using gnu tar 1.17
-               // gnutar  -b 4 --format=ustar (longname/)*15 + file.txt
-               file: "testdata/ustar.tar",
+               // TODO(dsnet): The Writer output should match the following file.
+               // To fix an issue (see https://golang.org/issue/12594), we disabled
+               // prefix support, which alters the generated output.
+               /*
+                       // This file was produced using gnu tar 1.17
+                       // gnutar  -b 4 --format=ustar (longname/)*15 + file.txt
+                       file: "testdata/ustar.tar"
+               */
+               file: "testdata/ustar.issue12594.tar", // This is a valid tar file, but not expected
                entries: []*entry{{
                        header: &Header{
                                Name:     strings.Repeat("longname/", 15) + "file.txt",
@@ -586,3 +596,52 @@ func TestSplitUSTARPath(t *testing.T) {
                }
        }
 }
+
+// TestIssue12594 tests that the Writer does not attempt to populate the prefix
+// field when encoding a header in the GNU format. The prefix field is valid
+// in USTAR and PAX, but not GNU.
+func TestIssue12594(t *testing.T) {
+       names := []string{
+               "0/1/2/3/4/5/6/7/8/9/10/11/12/13/14/15/16/17/18/19/20/21/22/23/24/25/26/27/28/29/30/file.txt",
+               "0/1/2/3/4/5/6/7/8/9/10/11/12/13/14/15/16/17/18/19/20/21/22/23/24/25/26/27/28/29/30/31/32/33/file.txt",
+               "0/1/2/3/4/5/6/7/8/9/10/11/12/13/14/15/16/17/18/19/20/21/22/23/24/25/26/27/28/29/30/31/32/333/file.txt",
+               "0/1/2/3/4/5/6/7/8/9/10/11/12/13/14/15/16/17/18/19/20/21/22/23/24/25/26/27/28/29/30/31/32/33/34/35/36/37/38/39/40/file.txt",
+               "0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000/file.txt",
+               "/home/support/.openoffice.org/3/user/uno_packages/cache/registry/com.sun.star.comp.deployment.executable.PackageRegistryBackend",
+       }
+
+       for i, name := range names {
+               var b bytes.Buffer
+
+               tw := NewWriter(&b)
+               if err := tw.WriteHeader(&Header{
+                       Name: name,
+                       Uid:  1 << 25, // Prevent USTAR format
+               }); err != nil {
+                       t.Errorf("test %d, unexpected WriteHeader error: %v", i, err)
+               }
+               if err := tw.Close(); err != nil {
+                       t.Errorf("test %d, unexpected Close error: %v", i, err)
+               }
+
+               // The prefix field should never appear in the GNU format.
+               var blk block
+               copy(blk[:], b.Bytes())
+               prefix := string(blk.USTAR().Prefix())
+               if i := strings.IndexByte(prefix, 0); i >= 0 {
+                       prefix = prefix[:i] // Truncate at the NUL terminator
+               }
+               if blk.GetFormat() == formatGNU && len(prefix) > 0 && strings.HasPrefix(name, prefix) {
+                       t.Errorf("test %d, found prefix in GNU format: %s", i, prefix)
+               }
+
+               tr := NewReader(&b)
+               hdr, err := tr.Next()
+               if err != nil {
+                       t.Errorf("test %d, unexpected Next error: %v", i, err)
+               }
+               if hdr.Name != name {
+                       t.Errorf("test %d, hdr.Name = %s, want %s", i, hdr.Name, name)
+               }
+       }
+}