archive/tar: ignore ChangeTime and AccessTime unless Format is specified

CL 59230 changed Writer.WriteHeader to ignore the ChangeTime and AccessTime
fields when considering using the USTAR format when the format is unspecified.
This policy is confusing and leads to unexpected behavior where some files
have ModTime only, while others have ModTime+AccessTime+ChangeTime if the
format became PAX for some unrelated reason (e.g., long pathname).

Change the policy to simply always ignore ChangeTime, AccessTime, and
sub-second time resolutions unless the user explicitly specifies a format.
This is a safe policy change since WriteHeader had no support for the
above features in any Go release.

Support for ChangeTime and AccessTime was added in CL 55570.
Support for sub-second times was added in CL 55552.
Both CLs landed after the latest Go release (i.e., Go1.9), which was
cut from the master branch around August 6th, 2017.

Change-Id: Ib82baa1bf9dd4573ed4f674b7d55d15f733a4843
Reviewed-on: https://go-review.googlesource.com/69296
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 7f8abdf989e108dc5c59f9894c20655af258991a..e3609536c01503f4ffba821fbf38045b8c104a7d 100644 (file)
--- a/src/archive/tar/common.go
+++ b/src/archive/tar/common.go
@@ -152,10 +152,11 @@ type Header struct {
        Uname string // User name of owner
        Gname string // Group name of owner
 
-       // The PAX format encodes the timestamps with sub-second resolution,
-       // while the other formats (USTAR and GNU) truncate to the nearest second.
-       // If the Format is unspecified, then Writer.WriteHeader ignores
-       // AccessTime and ChangeTime when using the USTAR format.
+       // If the Format is unspecified, then Writer.WriteHeader rounds ModTime
+       // to the nearest second and ignores the AccessTime and ChangeTime fields.
+       //
+       // To use AccessTime or ChangeTime, specify the Format as PAX or GNU.
+       // To use sub-second resolution, specify the Format as PAX.
        ModTime    time.Time // Modification time
        AccessTime time.Time // Access time (requires either PAX or GNU support)
        ChangeTime time.Time // Change time (requires either PAX or GNU support)
@@ -340,7 +341,8 @@ type fileState interface {
 //
 // As a by-product of checking the fields, this function returns paxHdrs, which
 // contain all fields that could not be directly encoded.
-func (h *Header) allowedFormats() (format Format, paxHdrs map[string]string, err error) {
+// A value receiver ensures that this method does not mutate the source Header.
+func (h Header) allowedFormats() (format Format, paxHdrs map[string]string, err error) {
        format = FormatUSTAR | FormatPAX | FormatGNU
        paxHdrs = make(map[string]string)
 
@@ -402,8 +404,7 @@ func (h *Header) allowedFormats() (format Format, paxHdrs map[string]string, err
                }
                isMtime := paxKey == paxMtime
                fitsOctal := fitsInOctal(size, ts.Unix())
-               noACTime := !isMtime && h.Format != FormatUnknown
-               if (isMtime && !fitsOctal) || noACTime {
+               if (isMtime && !fitsOctal) || !isMtime {
                        whyNoUSTAR = fmt.Sprintf("USTAR cannot encode %s=%v", name, ts)
                        format.mustNotBe(FormatUSTAR)
                }
@@ -452,7 +453,7 @@ func (h *Header) allowedFormats() (format Format, paxHdrs map[string]string, err
        case TypeXHeader, TypeGNULongName, TypeGNULongLink:
                return FormatUnknown, nil, headerError{"cannot manually encode TypeXHeader, TypeGNULongName, or TypeGNULongLink headers"}
        case TypeXGlobalHeader:
-               if !reflect.DeepEqual(h, &Header{Typeflag: h.Typeflag, Xattrs: h.Xattrs, PAXRecords: h.PAXRecords, Format: h.Format}) {
+               if !reflect.DeepEqual(h, Header{Typeflag: h.Typeflag, Xattrs: h.Xattrs, PAXRecords: h.PAXRecords, Format: h.Format}) {
                        return FormatUnknown, nil, headerError{"only PAXRecords may be set for TypeXGlobalHeader"}
                }
                whyOnlyPAX = "only PAX supports TypeXGlobalHeader"

diff --git a/src/archive/tar/tar_test.go b/src/archive/tar/tar_test.go
index 61f52be31d38f0a5fe672deb0db5402950527081..9b0cba4e12fa94916370e01b67bc70cb9c43f3bf 100644 (file)
--- a/src/archive/tar/tar_test.go
+++ b/src/archive/tar/tar_test.go
@@ -696,7 +696,7 @@ func TestHeaderAllowedFormats(t *testing.T) {
        }, {
                header:  &Header{AccessTime: time.Unix(0, 0)},
                paxHdrs: map[string]string{paxAtime: "0"},
-               formats: FormatUSTAR | FormatPAX | FormatGNU,
+               formats: FormatPAX | FormatGNU,
        }, {
                header:  &Header{AccessTime: time.Unix(0, 0), Format: FormatUSTAR},
                paxHdrs: map[string]string{paxAtime: "0"},
@@ -712,7 +712,7 @@ func TestHeaderAllowedFormats(t *testing.T) {
        }, {
                header:  &Header{AccessTime: time.Unix(-123, 0)},
                paxHdrs: map[string]string{paxAtime: "-123"},
-               formats: FormatUSTAR | FormatPAX | FormatGNU,
+               formats: FormatPAX | FormatGNU,
        }, {
                header:  &Header{AccessTime: time.Unix(-123, 0), Format: FormatPAX},
                paxHdrs: map[string]string{paxAtime: "-123"},
@@ -720,7 +720,7 @@ func TestHeaderAllowedFormats(t *testing.T) {
        }, {
                header:  &Header{ChangeTime: time.Unix(123, 456)},
                paxHdrs: map[string]string{paxCtime: "123.000000456"},
-               formats: FormatUSTAR | FormatPAX | FormatGNU,
+               formats: FormatPAX | FormatGNU,
        }, {
                header:  &Header{ChangeTime: time.Unix(123, 456), Format: FormatUSTAR},
                paxHdrs: map[string]string{paxCtime: "123.000000456"},

diff --git a/src/archive/tar/writer.go b/src/archive/tar/writer.go
index f938dfbfde4955d0613560f4ec95eddabf014893..2eed619348653d16d5e0041034ceda26221fcb95 100644 (file)
--- a/src/archive/tar/writer.go
+++ b/src/archive/tar/writer.go
@@ -70,8 +70,19 @@ func (tw *Writer) WriteHeader(hdr *Header) error {
        if err := tw.Flush(); err != nil {
                return err
        }
-
        tw.hdr = *hdr // Shallow copy of Header
+
+       // Round ModTime and ignore AccessTime and ChangeTime unless
+       // the format is explicitly chosen.
+       // This ensures nominal usage of WriteHeader (without specifying the format)
+       // does not always result in the PAX format being chosen, which
+       // causes a 1KiB increase to every header.
+       if tw.hdr.Format == FormatUnknown {
+               tw.hdr.ModTime = tw.hdr.ModTime.Round(time.Second)
+               tw.hdr.AccessTime = time.Time{}
+               tw.hdr.ChangeTime = time.Time{}
+       }
+
        allowedFormats, paxHdrs, err := tw.hdr.allowedFormats()
        switch {
        case allowedFormats.has(FormatUSTAR):