From: Joe Tsai Date: Tue, 18 Oct 2016 23:38:54 +0000 (-0700) Subject: archive/tar: fix parsePAXTime X-Git-Tag: go1.8beta1~747 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=ef8e85e8b8176fb096683dac00cf536fd7d228bc;p=gostls13.git archive/tar: fix parsePAXTime Issues fixed: * Could not handle quantity of seconds greater than 1<<31 on 32bit machines since strconv.ParseInt did not treat integers as 64b. * Did not handle negative timestamps properly if nanoseconds were used. Note that "-123.456" should result in a call to time.Unix(-123, -456000000). * Incorrectly allowed a '-' right after the '.' (e.g., -123.-456) * Did not detect invalid input after the truncation point (e.g., 123.123456789badbadbad). Note that negative timestamps are allowed by PAX, but are not guaranteed to be portable. See the relevant specification: <<< If pax encounters a file with a negative timestamp in copy or write mode, it can reject the file, substitute a non-negative timestamp, or generate a non-portable timestamp with a leading '-'. >>> Since the previous behavior already partially supported negative timestamps, we are bound by Go's compatibility rules to keep support for them. However, we should at least make sure we handle them properly. Change-Id: I5686997708bfb59110ea7981175427290be737d1 Reviewed-on: https://go-review.googlesource.com/31441 Run-TryBot: Joe Tsai TryBot-Result: Gobot Gobot Reviewed-by: Brad Fitzpatrick --- diff --git a/src/archive/tar/strconv.go b/src/archive/tar/strconv.go index 45e46b13b8..bb5b51c02d 100644 --- a/src/archive/tar/strconv.go +++ b/src/archive/tar/strconv.go @@ -12,8 +12,6 @@ import ( "time" ) -const maxNanoSecondIntSize = 9 - func isASCII(s string) bool { for _, c := range s { if c >= 0x80 { @@ -165,40 +163,41 @@ func (f *formatter) formatOctal(b []byte, x int64) { f.formatString(b, s) } -// parsePAXTime takes a string of the form %d.%d as described in -// the PAX specification. -func parsePAXTime(t string) (time.Time, error) { - buf := []byte(t) - pos := bytes.IndexByte(buf, '.') - var seconds, nanoseconds int64 - var err error - if pos == -1 { - seconds, err = strconv.ParseInt(t, 10, 0) - if err != nil { - return time.Time{}, err - } +// parsePAXTime takes a string of the form %d.%d as described in the PAX +// specification. Note that this implementation allows for negative timestamps, +// which is allowed for by the PAX specification, but not always portable. +func parsePAXTime(s string) (time.Time, error) { + const maxNanoSecondDigits = 9 + + // Split string into seconds and sub-seconds parts. + ss, sn := s, "" + if pos := strings.IndexByte(s, '.'); pos >= 0 { + ss, sn = s[:pos], s[pos+1:] + } + + // Parse the seconds. + secs, err := strconv.ParseInt(ss, 10, 64) + if err != nil { + return time.Time{}, ErrHeader + } + if len(sn) == 0 { + return time.Unix(secs, 0), nil // No sub-second values + } + + // Parse the nanoseconds. + if strings.Trim(sn, "0123456789") != "" { + return time.Time{}, ErrHeader + } + if len(sn) < maxNanoSecondDigits { + sn += strings.Repeat("0", maxNanoSecondDigits-len(sn)) // Right pad } else { - seconds, err = strconv.ParseInt(string(buf[:pos]), 10, 0) - if err != nil { - return time.Time{}, err - } - nanoBuf := string(buf[pos+1:]) - // Pad as needed before converting to a decimal. - // For example .030 -> .030000000 -> 30000000 nanoseconds - if len(nanoBuf) < maxNanoSecondIntSize { - // Right pad - nanoBuf += strings.Repeat("0", maxNanoSecondIntSize-len(nanoBuf)) - } else if len(nanoBuf) > maxNanoSecondIntSize { - // Right truncate - nanoBuf = nanoBuf[:maxNanoSecondIntSize] - } - nanoseconds, err = strconv.ParseInt(nanoBuf, 10, 0) - if err != nil { - return time.Time{}, err - } + sn = sn[:maxNanoSecondDigits] // Right truncate + } + nsecs, _ := strconv.ParseInt(sn, 10, 64) // Must succeed + if len(ss) > 0 && ss[0] == '-' { + return time.Unix(secs, -1*int64(nsecs)), nil // Negative correction } - ts := time.Unix(seconds, nanoseconds) - return ts, nil + return time.Unix(secs, int64(nsecs)), nil } // TODO(dsnet): Implement formatPAXTime. diff --git a/src/archive/tar/strconv_test.go b/src/archive/tar/strconv_test.go index 7caecb5196..beb70938bf 100644 --- a/src/archive/tar/strconv_test.go +++ b/src/archive/tar/strconv_test.go @@ -156,20 +156,82 @@ func TestFormatNumeric(t *testing.T) { } func TestParsePAXTime(t *testing.T) { - timestamps := map[string]time.Time{ - "1350244992.023960108": time.Unix(1350244992, 23960108), // The common case - "1350244992.02396010": time.Unix(1350244992, 23960100), // Lower precision value - "1350244992.0239601089": time.Unix(1350244992, 23960108), // Higher precision value - "1350244992": time.Unix(1350244992, 0), // Low precision value + vectors := []struct { + in string + want time.Time + ok bool + }{ + {"1350244992.023960108", time.Unix(1350244992, 23960108), true}, + {"1350244992.02396010", time.Unix(1350244992, 23960100), true}, + {"1350244992.0239601089", time.Unix(1350244992, 23960108), true}, + {"1350244992.3", time.Unix(1350244992, 300000000), true}, + {"1350244992", time.Unix(1350244992, 0), true}, + {"-1.000000001", time.Unix(-1, -1e0+0e0), true}, + {"-1.000001", time.Unix(-1, -1e3+0e0), true}, + {"-1.001000", time.Unix(-1, -1e6+0e0), true}, + {"-1", time.Unix(-1, -0e0+0e0), true}, + {"-1.999000", time.Unix(-1, -1e9+1e6), true}, + {"-1.999999", time.Unix(-1, -1e9+1e3), true}, + {"-1.999999999", time.Unix(-1, -1e9+1e0), true}, + {"0.000000001", time.Unix(0, 1e0+0e0), true}, + {"0.000001", time.Unix(0, 1e3+0e0), true}, + {"0.001000", time.Unix(0, 1e6+0e0), true}, + {"0", time.Unix(0, 0e0), true}, + {"0.999000", time.Unix(0, 1e9-1e6), true}, + {"0.999999", time.Unix(0, 1e9-1e3), true}, + {"0.999999999", time.Unix(0, 1e9-1e0), true}, + {"1.000000001", time.Unix(+1, +1e0-0e0), true}, + {"1.000001", time.Unix(+1, +1e3-0e0), true}, + {"1.001000", time.Unix(+1, +1e6-0e0), true}, + {"1", time.Unix(+1, +0e0-0e0), true}, + {"1.999000", time.Unix(+1, +1e9-1e6), true}, + {"1.999999", time.Unix(+1, +1e9-1e3), true}, + {"1.999999999", time.Unix(+1, +1e9-1e0), true}, + {"-1350244992.023960108", time.Unix(-1350244992, -23960108), true}, + {"-1350244992.02396010", time.Unix(-1350244992, -23960100), true}, + {"-1350244992.0239601089", time.Unix(-1350244992, -23960108), true}, + {"-1350244992.3", time.Unix(-1350244992, -300000000), true}, + {"-1350244992", time.Unix(-1350244992, 0), true}, + {"", time.Time{}, false}, + {"0", time.Unix(0, 0), true}, + {"1.", time.Unix(1, 0), true}, + {"0.0", time.Unix(0, 0), true}, + {".5", time.Time{}, false}, + {"-1.3", time.Unix(-1, -3e8), true}, + {"-1.0", time.Unix(-1, -0e0), true}, + {"-0.0", time.Unix(-0, -0e0), true}, + {"-0.1", time.Unix(-0, -1e8), true}, + {"-0.01", time.Unix(-0, -1e7), true}, + {"-0.99", time.Unix(-0, -99e7), true}, + {"-0.98", time.Unix(-0, -98e7), true}, + {"-1.1", time.Unix(-1, -1e8), true}, + {"-1.01", time.Unix(-1, -1e7), true}, + {"-2.99", time.Unix(-2, -99e7), true}, + {"-5.98", time.Unix(-5, -98e7), true}, + {"-", time.Time{}, false}, + {"+", time.Time{}, false}, + {"-1.-1", time.Time{}, false}, + {"99999999999999999999999999999999999999999999999", time.Time{}, false}, + {"0.123456789abcdef", time.Time{}, false}, + {"foo", time.Time{}, false}, + {"\x00", time.Time{}, false}, + {"𝟵𝟴𝟳𝟲𝟱.𝟰𝟯𝟮𝟭𝟬", time.Time{}, false}, // Unicode numbers (U+1D7EC to U+1D7F5) + {"98765﹒43210", time.Time{}, false}, // Unicode period (U+FE52) } - for input, expected := range timestamps { - ts, err := parsePAXTime(input) - if err != nil { - t.Fatal(err) + for _, v := range vectors { + ts, err := parsePAXTime(v.in) + ok := (err == nil) + if v.ok != ok { + if v.ok { + t.Errorf("parsePAXTime(%q): got parsing failure, want success", v.in) + } else { + t.Errorf("parsePAXTime(%q): got parsing success, want failure", v.in) + } } - if !ts.Equal(expected) { - t.Fatalf("Time parsing failure %s %s", ts, expected) + if ok && !ts.Equal(v.want) { + t.Errorf("parsePAXTime(%q): got (%ds %dns), want (%ds %dns)", + v.in, ts.Unix(), ts.Nanosecond(), v.want.Unix(), v.want.Nanosecond()) } } }