time: correctly parse large input durations and avoid precision loss

Do not lose precision for durations specified without fractions
that can be represented by an int64 such as 1<<53+1 nanoseconds.
Previously there was some precision lost in floating point conversion.

Handle overflow for durations above 1<<63-1 nanoseconds but not earlier.

Add tests to cover the above cases.

Change-Id: I4bcda93cee1673e501ecb6a9eef3914ee29aecd2
Reviewed-on: https://go-review.googlesource.com/2461
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>

diff --git a/src/time/format.go b/src/time/format.go
index 04e79f32dcd4d2a080a5c857a7711ca2598abd58..0325399132c5549a59037b3f0ba3a43856cb2c55 100644 (file)
--- a/src/time/format.go
+++ b/src/time/format.go
@@ -1131,24 +1131,28 @@ func leadingInt(s string) (x int64, rem string, err error) {
                if c < '0' || c > '9' {
                        break
                }
-               if x >= (1<<63-10)/10 {
+               if x > (1<<63-1)/10 {
                        // overflow
                        return 0, "", errLeadingInt
                }
                x = x*10 + int64(c) - '0'
+               if x < 0 {
+                       // overflow
+                       return 0, "", errLeadingInt
+               }
        }
        return x, s[i:], nil
 }
 
-var unitMap = map[string]float64{
-       "ns": float64(Nanosecond),
-       "us": float64(Microsecond),
-       "µs": float64(Microsecond), // U+00B5 = micro symbol
-       "μs": float64(Microsecond), // U+03BC = Greek letter mu
-       "ms": float64(Millisecond),
-       "s":  float64(Second),
-       "m":  float64(Minute),
-       "h":  float64(Hour),
+var unitMap = map[string]int64{
+       "ns": int64(Nanosecond),
+       "us": int64(Microsecond),
+       "µs": int64(Microsecond), // U+00B5 = micro symbol
+       "μs": int64(Microsecond), // U+03BC = Greek letter mu
+       "ms": int64(Millisecond),
+       "s":  int64(Second),
+       "m":  int64(Minute),
+       "h":  int64(Hour),
 }
 
 // ParseDuration parses a duration string.
@@ -1159,7 +1163,7 @@ var unitMap = map[string]float64{
 func ParseDuration(s string) (Duration, error) {
        // [-+]?([0-9]*(\.[0-9]*)?[a-z]+)+
        orig := s
-       f := float64(0)
+       var d int64
        neg := false
 
        // Consume [-+]?
@@ -1178,22 +1182,23 @@ func ParseDuration(s string) (Duration, error) {
                return 0, errors.New("time: invalid duration " + orig)
        }
        for s != "" {
-               g := float64(0) // this element of the sequence
+               var (
+                       v, f  int64       // integers before, after decimal point
+                       scale float64 = 1 // value = v + f/scale
+               )
 
-               var x int64
                var err error
 
                // The next character must be [0-9.]
-               if !(s[0] == '.' || ('0' <= s[0] && s[0] <= '9')) {
+               if !(s[0] == '.' || '0' <= s[0] && s[0] <= '9') {
                        return 0, errors.New("time: invalid duration " + orig)
                }
                // Consume [0-9]*
                pl := len(s)
-               x, s, err = leadingInt(s)
+               v, s, err = leadingInt(s)
                if err != nil {
                        return 0, errors.New("time: invalid duration " + orig)
                }
-               g = float64(x)
                pre := pl != len(s) // whether we consumed anything before a period
 
                // Consume (\.[0-9]*)?
@@ -1201,15 +1206,13 @@ func ParseDuration(s string) (Duration, error) {
                if s != "" && s[0] == '.' {
                        s = s[1:]
                        pl := len(s)
-                       x, s, err = leadingInt(s)
+                       f, s, err = leadingInt(s)
                        if err != nil {
                                return 0, errors.New("time: invalid duration " + orig)
                        }
-                       scale := 1.0
                        for n := pl - len(s); n > 0; n-- {
                                scale *= 10
                        }
-                       g += float64(x) / scale
                        post = pl != len(s)
                }
                if !pre && !post {
@@ -1221,7 +1224,7 @@ func ParseDuration(s string) (Duration, error) {
                i := 0
                for ; i < len(s); i++ {
                        c := s[i]
-                       if c == '.' || ('0' <= c && c <= '9') {
+                       if c == '.' || '0' <= c && c <= '9' {
                                break
                        }
                }
@@ -1234,15 +1237,29 @@ func ParseDuration(s string) (Duration, error) {
                if !ok {
                        return 0, errors.New("time: unknown unit " + u + " in duration " + orig)
                }
-
-               f += g * unit
+               if v > (1<<63-1)/unit {
+                       // overflow
+                       return 0, errors.New("time: invalid duration " + orig)
+               }
+               v *= unit
+               if f > 0 {
+                       // float64 is needed to be nanosecond accurate for fractions of hours.
+                       // v >= 0 && (f*unit/scale) <= 3.6e+12 (ns/h, h is the largest unit)
+                       v += int64(float64(f) * (float64(unit) / scale))
+                       if v < 0 {
+                               // overflow
+                               return 0, errors.New("time: invalid duration " + orig)
+                       }
+               }
+               d += v
+               if d < 0 {
+                       // overflow
+                       return 0, errors.New("time: invalid duration " + orig)
+               }
        }
 
        if neg {
-               f = -f
-       }
-       if f < float64(-1<<63) || f > float64(1<<63-1) {
-               return 0, errors.New("time: overflow parsing duration")
+               d = -d
        }
-       return Duration(f), nil
+       return Duration(d), nil
 }

diff --git a/src/time/time_test.go b/src/time/time_test.go
index 7e31dd78a926276bc69148fc868a3fc1b955da3b..757474a30b93f39484fa5babc2d94067613b8d33 100644 (file)
--- a/src/time/time_test.go
+++ b/src/time/time_test.go
@@ -832,6 +832,14 @@ var parseDurationTests = []struct {
        {"52763797000ns", true, 52763797000 * Nanosecond},
        // more than 9 digits after decimal point, see http://golang.org/issue/6617
        {"0.3333333333333333333h", true, 20 * Minute},
+       // 9007199254740993 = 1<<53+1 cannot be stored precisely in a float64
+       {"9007199254740993ns", true, (1<<53 + 1) * Nanosecond},
+       // largest duration that can be represented by int64 in nanoseconds
+       {"9223372036854775807ns", true, (1<<63 - 1) * Nanosecond},
+       {"9223372036854775.807us", true, (1<<63 - 1) * Nanosecond},
+       {"9223372036s854ms775us807ns", true, (1<<63 - 1) * Nanosecond},
+       // large negative value
+       {"-9223372036854775807ns", true, -1<<63 + 1*Nanosecond},
 
        // errors
        {"", false, 0},
@@ -842,7 +850,13 @@ var parseDurationTests = []struct {
        {"-.", false, 0},
        {".s", false, 0},
        {"+.s", false, 0},
-       {"3000000h", false, 0}, // overflow
+       {"3000000h", false, 0},                  // overflow
+       {"9223372036854775808ns", false, 0},     // overflow
+       {"9223372036854775.808us", false, 0},    // overflow
+       {"9223372036854ms775us808ns", false, 0}, // overflow
+       // largest negative value of type int64 in nanoseconds should fail
+       // see https://go-review.googlesource.com/#/c/2461/
+       {"-9223372036854775808ns", false, 0},
 }
 
 func TestParseDuration(t *testing.T) {
@@ -1052,6 +1066,13 @@ func BenchmarkParse(b *testing.B) {
        }
 }
 
+func BenchmarkParseDuration(b *testing.B) {
+       for i := 0; i < b.N; i++ {
+               ParseDuration("9007199254.740993ms")
+               ParseDuration("9007199254740993ns")
+       }
+}
+
 func BenchmarkHour(b *testing.B) {
        t := Now()
        for i := 0; i < b.N; i++ {