From 577aab0c595825ba113b63f4ef1460e8471c803e Mon Sep 17 00:00:00 2001 From: Joe Tsai Date: Mon, 9 Oct 2017 11:45:43 -0700 Subject: [PATCH] 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 Run-TryBot: Ian Lance Taylor TryBot-Result: Gobot Gobot --- src/archive/tar/common.go | 17 +++++++++-------- src/archive/tar/tar_test.go | 6 +++--- src/archive/tar/writer.go | 13 ++++++++++++- 3 files changed, 24 insertions(+), 12 deletions(-) diff --git a/src/archive/tar/common.go b/src/archive/tar/common.go index 7f8abdf989..e3609536c0 100644 --- 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 61f52be31d..9b0cba4e12 100644 --- 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 f938dfbfde..2eed619348 100644 --- 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): -- 2.48.1