Fixed int64 overflow for timestamp in v1/api parseDuration and parseTime (#2501)

* Fixed int64 overflow for timestamp in v1/api parseDuration and parseTime

This led to unexpected results on wrong query with "(...)&start=148966367200.372&end=1489667272.372"
That query is wrong because of `start > end` but actually internal int64 overflow caused start to be something around MinInt64 (huge negative value) and was passing validation.

BTW: Not sure if negative timestamp makes sense even.. But model.Earliest is actually MinInt64, can someone explain me why?

Signed-off-by: Bartek Plotka <bwplotka@gmail.com>

* Added missing trailing periods on comments.

Signed-off-by: Bartek Plotka <bwplotka@gmail.com>

* MOved to only `<` and `>`. Removed equal.

Signed-off-by: Bartek Plotka <bwplotka@gmail.com>
This commit is contained in:
Bplotka 2017-03-16 14:16:20 +00:00 committed by Julius Volz
parent 48d221c11e
commit 1823ae8bc4
2 changed files with 40 additions and 8 deletions

View file

@ -17,6 +17,7 @@ import (
"encoding/json"
"errors"
"fmt"
"math"
"net/http"
"sort"
"strconv"
@ -477,8 +478,11 @@ func respondError(w http.ResponseWriter, apiErr *apiError, data interface{}) {
func parseTime(s string) (model.Time, error) {
if t, err := strconv.ParseFloat(s, 64); err == nil {
ts := int64(t * float64(time.Second))
return model.TimeFromUnixNano(ts), nil
ts := t * float64(time.Second)
if ts > float64(math.MaxInt64) || ts < float64(math.MinInt64) {
return 0, fmt.Errorf("cannot parse %q to a valid timestamp. It overflows int64", s)
}
return model.TimeFromUnixNano(int64(ts)), nil
}
if t, err := time.Parse(time.RFC3339Nano, s); err == nil {
return model.TimeFromUnixNano(t.UnixNano()), nil
@ -488,7 +492,11 @@ func parseTime(s string) (model.Time, error) {
func parseDuration(s string) (time.Duration, error) {
if d, err := strconv.ParseFloat(s, 64); err == nil {
return time.Duration(d * float64(time.Second)), nil
ts := d * float64(time.Second)
if ts > float64(math.MaxInt64) || ts < float64(math.MinInt64) {
return 0, fmt.Errorf("cannot parse %q to a valid duration. It overflows int64", s)
}
return time.Duration(ts), nil
}
if d, err := model.ParseDuration(s); err == nil {
return time.Duration(d), nil

View file

@ -221,7 +221,7 @@ func TestEndpoints(t *testing.T) {
},
errType: errorBadData,
},
// Invalid step
// Invalid step.
{
endpoint: api.queryRange,
query: url.Values{
@ -232,7 +232,7 @@ func TestEndpoints(t *testing.T) {
},
errType: errorBadData,
},
// Start after end
// Start after end.
{
endpoint: api.queryRange,
query: url.Values{
@ -243,6 +243,17 @@ func TestEndpoints(t *testing.T) {
},
errType: errorBadData,
},
// Start overflows int64 internally.
{
endpoint: api.queryRange,
query: url.Values{
"query": []string{"time()"},
"start": []string{"148966367200.372"},
"end": []string{"1489667272.372"},
"step": []string{"1"},
},
errType: errorBadData,
},
{
endpoint: api.labelValues,
params: map[string]string{
@ -593,15 +604,20 @@ func TestParseTime(t *testing.T) {
}, {
input: "30s",
fail: true,
}, {
// Internal int64 overflow.
input: "-148966367200.372",
fail: true,
}, {
// Internal int64 overflow.
input: "148966367200.372",
fail: true,
}, {
input: "123",
result: time.Unix(123, 0),
}, {
input: "123.123",
result: time.Unix(123, 123000000),
}, {
input: "123.123",
result: time.Unix(123, 123000000),
}, {
input: "2015-06-03T13:21:58.555Z",
result: ts,
@ -643,6 +659,14 @@ func TestParseDuration(t *testing.T) {
}, {
input: "2015-06-03T13:21:58.555Z",
fail: true,
}, {
// Internal int64 overflow.
input: "-148966367200.372",
fail: true,
}, {
// Internal int64 overflow.
input: "148966367200.372",
fail: true,
}, {
input: "123",
result: 123 * time.Second,