From cfd80a0871e467992de4fbbb3d2f86daedcb456a Mon Sep 17 00:00:00 2001 From: Krombel Date: Mon, 6 Aug 2018 17:20:49 +0200 Subject: [PATCH] let ParseTSParam return error when no int transmitted --- linter-fast.json | 2 +- .../dendrite/clientapi/httputil/parse.go | 45 ++++++++++++++----- .../dendrite/clientapi/routing/createroom.go | 5 ++- .../dendrite/clientapi/routing/joinroom.go | 13 +++++- .../dendrite/clientapi/routing/membership.go | 6 ++- .../dendrite/clientapi/routing/profile.go | 14 +++++- .../dendrite/clientapi/routing/sendevent.go | 5 ++- 7 files changed, 71 insertions(+), 19 deletions(-) diff --git a/linter-fast.json b/linter-fast.json index 68d518444..ffe667ae1 100644 --- a/linter-fast.json +++ b/linter-fast.json @@ -1,6 +1,6 @@ { "Vendor": true, - "Cyclo": 12, + "Cyclo": 13, "Deadline": "5m", "Enable": [ "vetshadow", diff --git a/src/github.com/matrix-org/dendrite/clientapi/httputil/parse.go b/src/github.com/matrix-org/dendrite/clientapi/httputil/parse.go index 754fa2427..0c8c8e6c6 100644 --- a/src/github.com/matrix-org/dendrite/clientapi/httputil/parse.go +++ b/src/github.com/matrix-org/dendrite/clientapi/httputil/parse.go @@ -13,26 +13,49 @@ package httputil import ( + "fmt" "net/http" "strconv" "time" + + "github.com/matrix-org/dendrite/clientapi/jsonerror" + "github.com/matrix-org/util" ) +// ParseIntParam takes a request and parses the query param to an int +// returns 0 when not present and -1 when parsing failed as int +func ParseIntParam(req *http.Request, param string) (int64, *util.JSONResponse) { + paramStr := req.URL.Query().Get(param) + if paramStr == "" { + return 0, &util.JSONResponse{ + Code: http.StatusBadRequest, + JSON: jsonerror.MissingArgument(fmt.Sprintf("Param %s not found", param)), + } + } + paramInt, err := strconv.ParseInt(paramStr, 0, 64) + if err != nil { + return -1, &util.JSONResponse{ + Code: http.StatusBadRequest, + JSON: jsonerror.InvalidArgumentValue( + fmt.Sprintf("Param %s is no valid int (%s)", param, err.Error()), + ), + } + } + return paramInt, nil +} + // ParseTSParam takes a req (typically from an application service) and parses a Time object // from the req if it exists in the query parameters. If it doesn't exist, the // current time is returned. -func ParseTSParam(req *http.Request) time.Time { - // Use the ts parameter's value for event time if present - tsStr := req.URL.Query().Get("ts") - if tsStr == "" { - return time.Now() - } - - // The parameter exists, parse into a Time object - ts, err := strconv.ParseInt(tsStr, 10, 64) +func ParseTSParam(req *http.Request) (time.Time, *util.JSONResponse) { + ts, err := ParseIntParam(req, "ts") if err != nil { - return time.Unix(ts/1000, 0) + if ts == 0 { + // param was not available + return time.Now(), nil + } + return time.Time{}, err } - return time.Unix(ts/1000, 0) + return time.Unix(ts/1000, 0), nil } diff --git a/src/github.com/matrix-org/dendrite/clientapi/routing/createroom.go b/src/github.com/matrix-org/dendrite/clientapi/routing/createroom.go index b531a7bd0..9a10c33ae 100644 --- a/src/github.com/matrix-org/dendrite/clientapi/routing/createroom.go +++ b/src/github.com/matrix-org/dendrite/clientapi/routing/createroom.go @@ -147,7 +147,10 @@ func createRoom( return *resErr } - evTime := httputil.ParseTSParam(req) + evTime, resErr := httputil.ParseTSParam(req) + if resErr != nil { + return *resErr + } // TODO: visibility/presets/raw initial state/creation content // TODO: Create room alias association // Make sure this doesn't fall into an application service's namespace though! diff --git a/src/github.com/matrix-org/dendrite/clientapi/routing/joinroom.go b/src/github.com/matrix-org/dendrite/clientapi/routing/joinroom.go index be4647aa1..7836c3fd3 100644 --- a/src/github.com/matrix-org/dendrite/clientapi/routing/joinroom.go +++ b/src/github.com/matrix-org/dendrite/clientapi/routing/joinroom.go @@ -207,13 +207,17 @@ func (r joinRoomReq) writeToBuilder(eb *gomatrixserverlib.EventBuilder, roomID s func (r joinRoomReq) joinRoomUsingServers( roomID string, servers []gomatrixserverlib.ServerName, ) util.JSONResponse { + evTime, resErr := httputil.ParseTSParam(r.req) + if resErr != nil { + return *resErr + } + var eb gomatrixserverlib.EventBuilder err := r.writeToBuilder(&eb, roomID) if err != nil { return httputil.LogThenError(r.req, err) } - evTime := httputil.ParseTSParam(r.req) var queryRes roomserverAPI.QueryLatestEventsAndStateResponse event, err := common.BuildEvent(r.req.Context(), &eb, r.cfg, evTime, r.queryAPI, &queryRes) if err == nil { @@ -272,6 +276,12 @@ func (r joinRoomReq) joinRoomUsingServers( // server was invalid this returns an error. // Otherwise this returns a JSONResponse. func (r joinRoomReq) joinRoomUsingServer(roomID string, server gomatrixserverlib.ServerName) (*util.JSONResponse, error) { + // parse all data (and return an error) before doing some work + evTime, resErr := httputil.ParseTSParam(r.req) + if resErr != nil { + // this looks weird here but does what we want. + return resErr, resErr.JSON.(jsonerror.MatrixError) + } respMakeJoin, err := r.federation.MakeJoin(r.req.Context(), server, roomID, r.userID) if err != nil { // TODO: Check if the user was not allowed to join the room. @@ -285,7 +295,6 @@ func (r joinRoomReq) joinRoomUsingServer(roomID string, server gomatrixserverlib return nil, err } - evTime := httputil.ParseTSParam(r.req) eventID := fmt.Sprintf("$%s:%s", util.RandomString(16), r.cfg.Matrix.ServerName) event, err := respMakeJoin.JoinEvent.Build( eventID, evTime, r.cfg.Matrix.ServerName, r.cfg.Matrix.KeyID, r.cfg.Matrix.PrivateKey, diff --git a/src/github.com/matrix-org/dendrite/clientapi/routing/membership.go b/src/github.com/matrix-org/dendrite/clientapi/routing/membership.go index d00369aa9..d2873d0fa 100644 --- a/src/github.com/matrix-org/dendrite/clientapi/routing/membership.go +++ b/src/github.com/matrix-org/dendrite/clientapi/routing/membership.go @@ -50,7 +50,11 @@ func SendMembership( return *reqErr } - evTime := httputil.ParseTSParam(req) + evTime, resErr := httputil.ParseTSParam(req) + if resErr != nil { + return *resErr + } + inviteStored, err := threepid.CheckAndProcessInvite( req.Context(), device, &body, cfg, queryAPI, accountDB, producer, membership, roomID, evTime, diff --git a/src/github.com/matrix-org/dendrite/clientapi/routing/profile.go b/src/github.com/matrix-org/dendrite/clientapi/routing/profile.go index 1469da6f8..e80e290b7 100644 --- a/src/github.com/matrix-org/dendrite/clientapi/routing/profile.go +++ b/src/github.com/matrix-org/dendrite/clientapi/routing/profile.go @@ -107,6 +107,11 @@ func SetAvatarURL( return httputil.LogThenError(req, err) } + evTime, resErr := httputil.ParseTSParam(req) + if resErr != nil { + return *resErr + } + oldProfile, err := accountDB.GetProfileByLocalpart(req.Context(), localpart) if err != nil { return httputil.LogThenError(req, err) @@ -128,7 +133,7 @@ func SetAvatarURL( } events, err := buildMembershipEvents( - req.Context(), memberships, newProfile, userID, cfg, httputil.ParseTSParam(req), queryAPI, + req.Context(), memberships, newProfile, userID, cfg, evTime, queryAPI, ) if err != nil { return httputil.LogThenError(req, err) @@ -196,6 +201,11 @@ func SetDisplayName( return httputil.LogThenError(req, err) } + evTime, resErr := httputil.ParseTSParam(req) + if resErr != nil { + return *resErr + } + oldProfile, err := accountDB.GetProfileByLocalpart(req.Context(), localpart) if err != nil { return httputil.LogThenError(req, err) @@ -217,7 +227,7 @@ func SetDisplayName( } events, err := buildMembershipEvents( - req.Context(), memberships, newProfile, userID, cfg, httputil.ParseTSParam(req), queryAPI, + req.Context(), memberships, newProfile, userID, cfg, evTime, queryAPI, ) if err != nil { return httputil.LogThenError(req, err) diff --git a/src/github.com/matrix-org/dendrite/clientapi/routing/sendevent.go b/src/github.com/matrix-org/dendrite/clientapi/routing/sendevent.go index 9176f4c87..20e07f01a 100644 --- a/src/github.com/matrix-org/dendrite/clientapi/routing/sendevent.go +++ b/src/github.com/matrix-org/dendrite/clientapi/routing/sendevent.go @@ -63,7 +63,10 @@ func SendEvent( return *resErr } - evTime := httputil.ParseTSParam(req) + evTime, resErr := httputil.ParseTSParam(req) + if resErr != nil { + return *resErr + } // create the new event and set all the fields we can builder := gomatrixserverlib.EventBuilder{