archive/tar: improve handling of directory paths

The USTAR format says:
<<<
Implementors should be aware that the previous file format did not include
a mechanism to archive directory type files.
For this reason, the convention of using a filename ending with
<slash> was adopted to specify a directory on the archive.
>>>

In light of this suggestion, make the following changes:
* Writer.WriteHeader refuses to encode a header where a file that
is obviously a file-type has a trailing slash in the name.
* formatter.formatString avoids encoding a trailing slash in the event
that the string is truncated (the full string will be encoded elsewhere,
so stripping the slash is safe).
* Reader.Next treats a TypeRegA (which is the zero value of Typeflag)
as a TypeDir if the name has a trailing slash.

Change-Id: Ibf27aa8234cce2032d92e5e5b28546c2f2ae5ef6
Reviewed-on: https://go-review.googlesource.com/69293
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

diff --git a/src/archive/tar/common.go b/src/archive/tar/common.go
index 5855b8e84f512feb0f3d7a9bc9f2b832026e13ea..7f8abdf989e108dc5c59f9894c20655af258991a 100644 (file)
--- a/src/archive/tar/common.go
+++ b/src/archive/tar/common.go
@@ -444,6 +444,11 @@ func (h *Header) allowedFormats() (format Format, paxHdrs map[string]string, err
        // Check for header-only types.
        var whyOnlyPAX, whyOnlyGNU string
        switch h.Typeflag {
+       case TypeReg, TypeChar, TypeBlock, TypeFifo, TypeGNUSparse:
+               // Exclude TypeLink and TypeSymlink, since they may reference directories.
+               if strings.HasSuffix(h.Name, "/") {
+                       return FormatUnknown, nil, headerError{"filename may not have trailing slash"}
+               }
        case TypeXHeader, TypeGNULongName, TypeGNULongLink:
                return FormatUnknown, nil, headerError{"cannot manually encode TypeXHeader, TypeGNULongName, or TypeGNULongLink headers"}
        case TypeXGlobalHeader:

diff --git a/src/archive/tar/reader.go b/src/archive/tar/reader.go
index bde6e1205ce37d6a0886266432ed432e38240e81..6bb2c46e7b818191f0cb5980286c10670586033a 100644 (file)
--- a/src/archive/tar/reader.go
+++ b/src/archive/tar/reader.go
@@ -130,6 +130,9 @@ loop:
                        if gnuLongLink != "" {
                                hdr.Linkname = gnuLongLink
                        }
+                       if hdr.Typeflag == TypeRegA && strings.HasSuffix(hdr.Name, "/") {
+                               hdr.Typeflag = TypeDir // Legacy archives use trailing slash for directories
+                       }
 
                        // The extended headers may have updated the size.
                        // Thus, setup the regFileReader again after merging PAX headers.

diff --git a/src/archive/tar/reader_test.go b/src/archive/tar/reader_test.go
index bbabd96246b685e7d396956fcaed580381cb218d..3ac81adb4dd8339a679cec075e863603b6ac71cb 100644 (file)
--- a/src/archive/tar/reader_test.go
+++ b/src/archive/tar/reader_test.go
@@ -675,6 +675,17 @@ func TestReader(t *testing.T) {
                        },
                        Format: FormatPAX,
                }},
+       }, {
+               file: "testdata/trailing-slash.tar",
+               headers: []*Header{{
+                       Typeflag: TypeDir,
+                       Name:     strings.Repeat("123456789/", 30),
+                       ModTime:  time.Unix(0, 0),
+                       PAXRecords: map[string]string{
+                               "path": strings.Repeat("123456789/", 30),
+                       },
+                       Format: FormatPAX,
+               }},
        }}
 
        for _, v := range vectors {

diff --git a/src/archive/tar/strconv.go b/src/archive/tar/strconv.go
index e02963b74ba7ea1e5e5323ecbaae5a06106b6011..8bbd65cd1acaf8866a226dcb2fd9a422d1fc1304 100644 (file)
--- a/src/archive/tar/strconv.go
+++ b/src/archive/tar/strconv.go
@@ -68,6 +68,14 @@ func (f *formatter) formatString(b []byte, s string) {
        if len(s) < len(b) {
                b[len(s)] = 0
        }
+
+       // Some buggy readers treat regular files with a trailing slash
+       // in the V7 path field as a directory even though the full path
+       // recorded elsewhere (e.g., via PAX record) contains no trailing slash.
+       if len(s) > len(b) && b[len(b)-1] == '/' {
+               n := len(strings.TrimRight(s[:len(b)], "/"))
+               b[n] = 0 // Replace trailing slash with NUL terminator
+       }
 }
 
 // fitsInBase256 reports whether x can be encoded into n bytes using base-256

diff --git a/src/archive/tar/tar_test.go b/src/archive/tar/tar_test.go
index 8d44f3bf65ab247c49eb8f1fb5a82446ef509ab7..61f52be31d38f0a5fe672deb0db5402950527081 100644 (file)
--- a/src/archive/tar/tar_test.go
+++ b/src/archive/tar/tar_test.go
@@ -748,6 +748,15 @@ func TestHeaderAllowedFormats(t *testing.T) {
        }, {
                header:  &Header{Name: "sparse.db", Size: 1000, SparseHoles: []SparseEntry{{0, 500}}, Format: FormatUSTAR},
                formats: FormatUnknown,
+       }, {
+               header:  &Header{Name: "foo/", Typeflag: TypeDir},
+               formats: FormatUSTAR | FormatPAX | FormatGNU,
+       }, {
+               header:  &Header{Name: "foo/", Typeflag: TypeReg},
+               formats: FormatUnknown,
+       }, {
+               header:  &Header{Name: "foo/", Typeflag: TypeSymlink},
+               formats: FormatUSTAR | FormatPAX | FormatGNU,
        }}
 
        for i, v := range vectors {

diff --git a/src/archive/tar/testdata/trailing-slash.tar b/src/archive/tar/testdata/trailing-slash.tar
new file mode 100644 (file)
index 0000000..bf1b2ec
Binary files /dev/null and b/src/archive/tar/testdata/trailing-slash.tar differ

diff --git a/src/archive/tar/writer.go b/src/archive/tar/writer.go
index 1e5b76f58f5cdddb38549f9442a0e97bacc392dc..f938dfbfde4955d0613560f4ec95eddabf014893 100644 (file)
--- a/src/archive/tar/writer.go
+++ b/src/archive/tar/writer.go
@@ -323,6 +323,7 @@ func (tw *Writer) writeRawFile(name, data string, flag byte, format Format) erro
        if len(name) > nameSize {
                name = name[:nameSize]
        }
+       name = strings.TrimRight(name, "/")
 
        var f formatter
        v7 := tw.blk.V7()

diff --git a/src/archive/tar/writer_test.go b/src/archive/tar/writer_test.go
index ecac29a39e79a1f3d01a17fb4d8f8acc1bf293b2..e9bcad9374da8f0f99662e1d3956c20dc03401c1 100644 (file)
--- a/src/archive/tar/writer_test.go
+++ b/src/archive/tar/writer_test.go
@@ -451,6 +451,12 @@ func TestWriter(t *testing.T) {
                        }, 6e10, nil},
                        testClose{nil},
                },
+       }, {
+               file: "testdata/trailing-slash.tar",
+               tests: []testFnc{
+                       testHeader{Header{Name: strings.Repeat("123456789/", 30)}, nil},
+                       testClose{nil},
+               },
        }}
 
        equalError := func(x, y error) bool {