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>

8 files changed, 43 insertions, 0 deletions

diff --git a/src/archive/tar/common.go b/src/archive/tar/common.go
index 5855b8e84f..7f8abdf989 100644
--- 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 bde6e1205c..6bb2c46e7b 100644
--- 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 bbabd96246..3ac81adb4d 100644
--- 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 e02963b74b..8bbd65cd1a 100644
--- 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 8d44f3bf65..61f52be31d 100644
--- 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
index 0000000000..bf1b2ec426
--- /dev/null
+++ b/src/archive/tar/testdata/trailing-slash.tar
diff --git a/src/archive/tar/writer.go b/src/archive/tar/writer.go
index 1e5b76f58f..f938dfbfde 100644
--- 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 ecac29a39e..e9bcad9374 100644
--- 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 {