From d1210acffd19482a0471f68d62baf10695fee8b9 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Martin=20Mo=CC=88hrmann?= Date: Wed, 7 Jan 2015 19:56:06 +0100 Subject: [PATCH] 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 Reviewed-by: Robert Griesemer --- src/time/format.go | 71 +++++++++++++++++++++++++++---------------- src/time/time_test.go | 23 +++++++++++++- 2 files changed, 66 insertions(+), 28 deletions(-) diff --git a/src/time/format.go b/src/time/format.go index 04e79f32dc..0325399132 100644 --- 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 7e31dd78a9..757474a30b 100644 --- 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++ { -- 2.50.0