Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added more strict validation on from/until parameters in api #200

Closed
wants to merge 6 commits into from
Closed

Added more strict validation on from/until parameters in api #200

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e1a6eb4
specify empty time range more precisely
KozzyKoder Sep 10, 2019
6dc02fb
added more tests for date converter
KozzyKoder Sep 10, 2019
df125d6
initial implementation of time restriction
KozzyKoder Sep 10, 2019
d94ad39
added detailed error messages
KozzyKoder Sep 11, 2019
fc734e0
codestyle violation fix
KozzyKoder Sep 12, 2019
e44d55a
Merge branch 'master' into vpestov/from-until-times-verification
grzkv Jan 22, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 20 additions & 4 deletions app/carbonapi/http_handlers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,8 +173,15 @@ func (app *App) renderHandler(w http.ResponseWriter, r *http.Request) {
logAsError = true
return
}
if form.from32 == form.until32 {
writeError(ctx, r, w, http.StatusBadRequest, "Invalid empty time range", form)
if form.from32 >= form.until32 {
KozzyKoder marked this conversation as resolved.
Show resolved Hide resolved
var clientErrMsgFmt string
if form.from32 == form.until32 {
clientErrMsgFmt = "Parameter from=%s has the same value as parameter until=%s. Result time range is empty"
} else {
clientErrMsgFmt = "Parameter from=%s greater than parameter until=%s. Result time range is empty"
}
clientErrMsg := fmt.Sprintf(clientErrMsgFmt, form.from, form.until)
writeError(ctx, r, w, http.StatusBadRequest, clientErrMsg, form)
toLog.HttpCode = http.StatusBadRequest
toLog.Reason = "invalid empty time range"
logAsError = true
Expand Down Expand Up @@ -555,8 +562,9 @@ func (app *App) renderHandlerProcessForm(r *http.Request, accessLogDetails *carb

// normalize from and until values
res.qtz = r.FormValue("tz")
res.from32 = date.DateParamToEpoch(res.from, res.qtz, timeNow().Add(-24*time.Hour).Unix(), app.defaultTimeZone)
res.until32 = date.DateParamToEpoch(res.until, res.qtz, timeNow().Unix(), app.defaultTimeZone)
var errFrom, errUntil error
res.from32, errFrom = date.DateParamToEpoch(res.from, res.qtz, timeNow().Add(-24*time.Hour).Unix(), app.defaultTimeZone)
res.until32, errUntil = date.DateParamToEpoch(res.until, res.qtz, timeNow().Unix(), app.defaultTimeZone)

accessLogDetails.UseCache = res.useCache
accessLogDetails.FromRaw = res.from
Expand All @@ -568,6 +576,14 @@ func (app *App) renderHandlerProcessForm(r *http.Request, accessLogDetails *carb
accessLogDetails.Format = res.format
accessLogDetails.Targets = res.targets

if errFrom != nil || errUntil != nil {
errFmt := "%s, invalid parameter %s=%s"
if errFrom != nil {
return res, errors.New(fmt.Sprintf(errFmt, errFrom.Error(), "from", res.from))
}
return res, errors.New(fmt.Sprintf(errFmt, errUntil.Error(), "until", res.until))
}

return res, nil
}

Expand Down
32 changes: 19 additions & 13 deletions date/date.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@ import (
"github.com/bookingcom/carbonapi/pkg/parser"
)

var errBadTime = errors.New("bad time")
var errBadTime = errors.New("Time has incorrect format")
KozzyKoder marked this conversation as resolved.
Show resolved Hide resolved
var errBadRelativeTime = errors.New("Invalid relative timestamp")
var errTsPartsCount = errors.New("Timestamp has too many parts")
var errDateFormat = errors.New("Invalid date format")
var timeNow = time.Now

// parseTime parses a time and returns hours and minutes
Expand Down Expand Up @@ -46,37 +49,37 @@ func parseTime(s string) (hour, minute int, err error) {
var TimeFormats = []string{"20060102", "01/02/06"}

// DateParamToEpoch turns a passed string parameter into a unix epoch
func DateParamToEpoch(s string, qtz string, d int64, defaultTimeZone *time.Location) int32 {
func DateParamToEpoch(s string, qtz string, d int64, defaultTimeZone *time.Location) (int32, error) {

if s == "" {
// return the default if nothing was passed
return int32(d)
return int32(d), nil
}

// relative timestamp
if s[0] == '-' {
offset, err := parser.IntervalString(s, -1)
if err != nil {
return int32(d)
return 0, errBadRelativeTime
}

return int32(timeNow().Add(time.Duration(offset) * time.Second).Unix())
return int32(timeNow().Add(time.Duration(offset) * time.Second).Unix()), nil
}

switch s {
case "now":
return int32(timeNow().Unix())
return int32(timeNow().Unix()), nil
case "midnight", "noon", "teatime":
yy, mm, dd := timeNow().Date()
hh, min, _ := parseTime(s) // error ignored, we know it's valid
dt := time.Date(yy, mm, dd, hh, min, 0, 0, defaultTimeZone)
return int32(dt.Unix())
return int32(dt.Unix()), nil
}

sint, err := strconv.Atoi(s)
// need to check that len(s) > 8 to avoid turning 20060102 into seconds
if err == nil && len(s) > 8 {
return int32(sint) // We got a timestamp so returning it
return int32(sint), nil // We got a timestamp so returning it
}

s = strings.Replace(s, "_", " ", 1) // Go can't parse _ in date strings
Expand All @@ -90,7 +93,7 @@ func DateParamToEpoch(s string, qtz string, d int64, defaultTimeZone *time.Locat
case len(split) == 2:
ts, ds = split[0], split[1]
case len(split) > 2:
return int32(d)
return 0, errTsPartsCount
}

var tz = defaultTimeZone
Expand Down Expand Up @@ -118,17 +121,20 @@ dateStringSwitch:
}
}

return int32(d)
return 0, errDateFormat
}

var hour, minute int
var parseErr error
if ts != "" {
hour, minute, _ = parseTime(ts)
// defaults to hour=0, minute=0 on error, which is midnight, which is fine for now
hour, minute, parseErr = parseTime(ts)
if parseErr != nil {
return 0, parseErr
}
}

yy, mm, dd := t.Date()
t = time.Date(yy, mm, dd, hour, minute, 0, 0, defaultTimeZone)

return int32(t.Unix())
return int32(t.Unix()), nil
}
57 changes: 41 additions & 16 deletions date/date_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,35 +15,60 @@ func TestDateParamToEpoch(t *testing.T) {
}

const shortForm = "15:04 2006-Jan-02"
const defaultTsStr = "17:30 2019-Apr-25"

var tests = []struct {
input string
output string
error bool
}{
{"midnight", "00:00 1994-Aug-16"},
{"noon", "12:00 1994-Aug-16"},
{"teatime", "16:00 1994-Aug-16"},
{"tomorrow", "00:00 1994-Aug-17"},

{"noon 08/12/94", "12:00 1994-Aug-12"},
{"midnight 20060812", "00:00 2006-Aug-12"},
{"noon tomorrow", "12:00 1994-Aug-17"},

{"17:04 19940812", "17:04 1994-Aug-12"},
{"-1day", "15:30 1994-Aug-15"},
{"19940812", "00:00 1994-Aug-12"},
{"midnight", "00:00 1994-Aug-16", false},
{"noon", "12:00 1994-Aug-16", false},
{"teatime", "16:00 1994-Aug-16", false},
{"tomorrow", "00:00 1994-Aug-17", false},

{"noon 08/12/94", "12:00 1994-Aug-12", false},
{"midnight 20060812", "00:00 2006-Aug-12", false},
{"noon tomorrow", "12:00 1994-Aug-17", false},

{"17:04 19940812", "17:04 1994-Aug-12", false},
{"-1day", "15:30 1994-Aug-15", false},
{"19940812", "00:00 1994-Aug-12", false},
{"now", "15:30 1994-Aug-16", false},
{"hh:mm 19940812", "00:00 1994-Aug-12", true},
{"12:30:00 19940812", "00:00 1994-Aug-12", true},
{"12:mm 19940812", "00:00 1994-Aug-12", true},
{"today", "00:00 1994-Aug-16", false},
{"yesterday", "00:00 1994-Aug-15", false},
{"1556201160", "16:06 2019-Apr-25", false},
{"", defaultTsStr, false},
{"-something", defaultTsStr, true},
{"17:04 19940812 1001", defaultTsStr, true},
{"12:30 08/15/06", "12:30 2006-Aug-15", false},
{"12:30 08-15-06", defaultTsStr, true},
{"08/15/06 12:30", defaultTsStr, true},
{"+5m", defaultTsStr, true},
}

defaultTime, _ := time.ParseInLocation(shortForm, defaultTsStr, defaultTimeZone)
defaultTs := defaultTime.Unix()

for _, tt := range tests {
got := DateParamToEpoch(tt.input, "Local", 0, defaultTimeZone)
got, parseErr := DateParamToEpoch(tt.input, "Local", defaultTs, defaultTimeZone)
ts, err := time.ParseInLocation(shortForm, tt.output, defaultTimeZone)
if err != nil {
panic(fmt.Sprintf("error parsing time: %q: %v", tt.output, err))
}
if (tt.error && parseErr == nil) || (!tt.error && parseErr != nil) {
t.Errorf("dateParamToEpoch(%q, 0)=%v, want error: %v", tt.input, got, tt.error)
continue
}

want := int32(ts.Unix())
if got != want {
t.Errorf("dateParamToEpoch(%q, 0)=%v, want %v", tt.input, got, want)
if !tt.error {
want := int32(ts.Unix())
if got != want {
t.Errorf("dateParamToEpoch(%q, 0)=%v, want %v", tt.input, got, want)
}
}
}
}