compress/gzip: only encode MTIME if it is valid

The GZIP format records the ModTime as an uint32 counting seconds since
the Unix epoch. The zero value is explicitly defined in section 2.3.1
as meaning no timestamp is available.

Currently, the Writer always encodes the ModTime even if it is the zero
time.Time value, which causes the Writer to try and encode the value
-62135596800 into the uint32 MTIME field. This causes an overflow and
results in our GZIP files having MTIME fields indicating a date in 2042-07-13.

We alter the Writer to only encode ModTime if the value does not underflow
the MTIME field (i.e., it is newer than the Unix epoch). We do not attempt
to fix what happens when the timestamp overflows in the year 2106.

We alter the Reader to only decode ModTime if the value is non-zero.
There is no risk of overflowing time.Time when decoding.

Fixes #17663

Change-Id: Ie1b65770c6342cd7b14aeebe10e5a49e6c9eb730
Reviewed-on: https://go-review.googlesource.com/32325
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

diff --git a/src/compress/gzip/gunzip.go b/src/compress/gzip/gunzip.go
index bc303898b3304aeeb3593c368e652cede0cdecb6..8bd750bd8b3a006c8fa4b114c3d842962c3562a6 100644 (file)
--- a/src/compress/gzip/gunzip.go
+++ b/src/compress/gzip/gunzip.go
@@ -186,7 +186,11 @@ func (z *Reader) readHeader() (hdr Header, err error) {
                return hdr, ErrHeader
        }
        flg := z.buf[3]
-       hdr.ModTime = time.Unix(int64(le.Uint32(z.buf[4:8])), 0)
+       if t := int64(le.Uint32(z.buf[4:8])); t > 0 {
+               // Section 2.3.1, the zero value for MTIME means that the
+               // modified time is not set.
+               hdr.ModTime = time.Unix(t, 0)
+       }
        // z.buf[8] is XFL and is currently ignored.
        hdr.OS = z.buf[9]
        z.digest = crc32.ChecksumIEEE(z.buf[:10])

diff --git a/src/compress/gzip/gzip.go b/src/compress/gzip/gzip.go
index c70321970b94838cbcfa8b8824104d753ba169c8..aafb442a66747512ae1358c0ec47ac8a959de6b4 100644 (file)
--- a/src/compress/gzip/gzip.go
+++ b/src/compress/gzip/gzip.go
@@ -10,6 +10,7 @@ import (
        "fmt"
        "hash/crc32"
        "io"
+       "time"
 )
 
 // These constants are copied from the flate package, so that code that imports
@@ -143,10 +144,7 @@ func (z *Writer) Write(p []byte) (int, error) {
        // Write the GZIP header lazily.
        if !z.wroteHeader {
                z.wroteHeader = true
-               z.buf[0] = gzipID1
-               z.buf[1] = gzipID2
-               z.buf[2] = gzipDeflate
-               z.buf[3] = 0
+               z.buf = [10]byte{0: gzipID1, 1: gzipID2, 2: gzipDeflate}
                if z.Extra != nil {
                        z.buf[3] |= 0x04
                }
@@ -156,13 +154,15 @@ func (z *Writer) Write(p []byte) (int, error) {
                if z.Comment != "" {
                        z.buf[3] |= 0x10
                }
-               le.PutUint32(z.buf[4:8], uint32(z.ModTime.Unix()))
+               if z.ModTime.After(time.Unix(0, 0)) {
+                       // Section 2.3.1, the zero value for MTIME means that the
+                       // modified time is not set.
+                       le.PutUint32(z.buf[4:8], uint32(z.ModTime.Unix()))
+               }
                if z.level == BestCompression {
                        z.buf[8] = 2
                } else if z.level == BestSpeed {
                        z.buf[8] = 4
-               } else {
-                       z.buf[8] = 0
                }
                z.buf[9] = z.OS
                n, z.err = z.w.Write(z.buf[:10])

diff --git a/src/compress/gzip/gzip_test.go b/src/compress/gzip/gzip_test.go
index 09271b24e92037492a7e9e753ec58f96a88d8ace..865c529f55c240bf46238fa03690c38bb8033b93 100644 (file)
--- a/src/compress/gzip/gzip_test.go
+++ b/src/compress/gzip/gzip_test.go
@@ -8,6 +8,7 @@ import (
        "bufio"
        "bytes"
        "io/ioutil"
+       "reflect"
        "testing"
        "time"
 )
@@ -24,6 +25,9 @@ func TestEmpty(t *testing.T) {
        if err != nil {
                t.Fatalf("NewReader: %v", err)
        }
+       if want := (Header{OS: 255}); !reflect.DeepEqual(r.Header, want) {
+               t.Errorf("Header mismatch:\ngot  %#v\nwant %#v", r.Header, want)
+       }
        b, err := ioutil.ReadAll(r)
        if err != nil {
                t.Fatalf("ReadAll: %v", err)

diff --git a/src/compress/gzip/issue14937_test.go b/src/compress/gzip/issue14937_test.go
index 432ad16b1c206606f6e25c89d6215fc0f3a4b315..e76d47cc4ee5700d9faedfeb8b94a3c7370ac3ae 100644 (file)
--- a/src/compress/gzip/issue14937_test.go
+++ b/src/compress/gzip/issue14937_test.go
@@ -7,7 +7,6 @@ import (
        "runtime"
        "strings"
        "testing"
-       "time"
 )
 
 // Per golang.org/issue/14937, check that every .gz file
@@ -16,8 +15,12 @@ func TestGZIPFilesHaveZeroMTimes(t *testing.T) {
        if testing.Short() && testenv.Builder() == "" {
                t.Skip("skipping in short mode")
        }
+       goroot, err := filepath.EvalSymlinks(runtime.GOROOT())
+       if err != nil {
+               t.Fatal("error evaluating GOROOT: ", err)
+       }
        var files []string
-       err := filepath.Walk(runtime.GOROOT(), func(path string, info os.FileInfo, err error) error {
+       err = filepath.Walk(goroot, func(path string, info os.FileInfo, err error) error {
                if err != nil {
                        return err
                }
@@ -53,7 +56,7 @@ func checkZeroMTime(t *testing.T, path string) {
                return
        }
        defer gz.Close()
-       if !gz.ModTime.Equal(time.Unix(0, 0)) {
+       if !gz.ModTime.IsZero() {
                t.Errorf("gzip file %s has non-zero mtime (%s)", path, gz.ModTime)
        }
 }