archive/tar: make Reader handle GNU format properly

The GNU format does not have a prefix field, so we should make
no attempt to read it. It does however have atime and ctime fields.
Since Go previously placed incorrect values here, we liberally
read the atime and ctime fields and ignore errors so that old tar
files written by Go can at least be partially read.

This fixes half of #12594. The Writer is much harder to fix.

Updates #12594

Change-Id: Ia32845e2f262ee53366cf41dfa935f4d770c7a30
Reviewed-on: https://go-review.googlesource.com/31444
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

src/archive/tar/reader.go

@@ -457,6 +457,26 @@ func (tr *Reader) readHeader() (*Header, *block, error) {
 	hdr.Typeflag = v7.TypeFlag()[0]
 	hdr.Linkname = p.parseString(v7.LinkName())
 
+	// The atime and ctime fields are often left unused. Some versions of Go
+	// had a bug in the tar.Writer where it would output an invalid tar file
+	// in certain rare situations because the logic incorrectly believed that
+	// the old GNU format had a prefix field. This is wrong and leads to
+	// an outputted file that actually mangles the atime and ctime fields.
+	//
+	// In order to continue reading tar files created by a buggy writer, we
+	// try to parse the atime and ctime fields, but just return the zero value
+	// of time.Time when we cannot parse them.
+	//
+	// See https://golang.org/issues/12594
+	tryParseTime := func(b []byte) time.Time {
+		var p parser
+		n := p.parseNumeric(b)
+		if b[0] != 0x00 && p.err == nil {
+			return time.Unix(n, 0)
+		}
+		return time.Time{}
+	}
+
 	// Unpack format specific fields.
 	if format > formatV7 {
 		ustar := tr.blk.USTAR()
@@ -469,9 +489,7 @@ func (tr *Reader) readHeader() (*Header, *block, error) {
 
 		var prefix string
 		switch format {
-		case formatUSTAR, formatGNU:
+		case formatUSTAR:
-			// TODO(dsnet): Do not use the prefix field for the GNU format!
-			// See golang.org/issues/12594
 			ustar := tr.blk.USTAR()
 			prefix = p.parseString(ustar.Prefix())
 		case formatSTAR:
@@ -479,6 +497,10 @@ func (tr *Reader) readHeader() (*Header, *block, error) {
 			prefix = p.parseString(star.Prefix())
 			hdr.AccessTime = time.Unix(p.parseNumeric(star.AccessTime()), 0)
 			hdr.ChangeTime = time.Unix(p.parseNumeric(star.ChangeTime()), 0)
+		case formatGNU:
+			gnu := tr.blk.GNU()
+			hdr.AccessTime = tryParseTime(gnu.AccessTime())
+			hdr.ChangeTime = tryParseTime(gnu.ChangeTime())
 		}
 		if len(prefix) > 0 {
 			hdr.Name = prefix + "/" + hdr.Name

src/archive/tar/reader_test.go

@@ -325,6 +325,53 @@ var untarTests = []*untarTest{
 			},
 		},
 	},
+	{
+		// GNU tar file with atime and ctime fields set.
+		// Created with the GNU tar v1.27.1.
+		//	tar --incremental -S -cvf gnu-incremental.tar test2
+		file: "testdata/gnu-incremental.tar",
+		headers: []*Header{
+			{
+				Name:       "test2/",
+				Mode:       16877,
+				Uid:        1000,
+				Gid:        1000,
+				Size:       14,
+				ModTime:    time.Unix(1441973427, 0),
+				Typeflag:   'D',
+				Uname:      "rawr",
+				Gname:      "dsnet",
+				AccessTime: time.Unix(1441974501, 0),
+				ChangeTime: time.Unix(1441973436, 0),
+			},
+			{
+				Name:       "test2/foo",
+				Mode:       33188,
+				Uid:        1000,
+				Gid:        1000,
+				Size:       64,
+				ModTime:    time.Unix(1441973363, 0),
+				Typeflag:   '0',
+				Uname:      "rawr",
+				Gname:      "dsnet",
+				AccessTime: time.Unix(1441974501, 0),
+				ChangeTime: time.Unix(1441973436, 0),
+			},
+			{
+				Name:       "test2/sparse",
+				Mode:       33188,
+				Uid:        1000,
+				Gid:        1000,
+				Size:       536870912,
+				ModTime:    time.Unix(1441973427, 0),
+				Typeflag:   'S',
+				Uname:      "rawr",
+				Gname:      "dsnet",
+				AccessTime: time.Unix(1441991948, 0),
+				ChangeTime: time.Unix(1441973436, 0),
+			},
+		},
+	},
 	{
 		// Matches the behavior of GNU and BSD tar utilities.
 		file: "testdata/pax-multi-hdrs.tar",