From 4cd58c2f2687fc5930a3da2581da09e2e96f69f5 Mon Sep 17 00:00:00 2001 From: Joe Tsai <joetsai@digital-static.net> Date: Fri, 6 Oct 2017 01:40:58 -0700 Subject: [PATCH] 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> --- src/archive/tar/common.go | 5 +++++ src/archive/tar/reader.go | 3 +++ src/archive/tar/reader_test.go | 11 +++++++++++ src/archive/tar/strconv.go | 8 ++++++++ src/archive/tar/tar_test.go | 9 +++++++++ src/archive/tar/testdata/trailing-slash.tar | Bin 0 -> 2560 bytes src/archive/tar/writer.go | 1 + src/archive/tar/writer_test.go | 6 ++++++ 8 files changed, 43 insertions(+) create mode 100644 src/archive/tar/testdata/trailing-slash.tar 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 0000000000000000000000000000000000000000..bf1b2ec426b4999b111c7f0914fb1f2d52b91728 GIT binary patch literal 2560 zcmXpsGBz<aGq<qRH>4Gd!2kkq(FP`FW-vLBN(KWX14A<d289X+I)KvRlEfmQ^>nam zFj`@3XrNG#Sdw8&v*@Q?!WuA>xdvlQlIJEmL^~{R<DwyX9#Td?^S_~yAu#`w=OYqz bQ7ivL-4FL9iOv{RJ{kg}Aut*O^bP?4t`UQn literal 0 HcmV?d00001 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 { -- 2.30.9