From 8a6152ca70aa86df651db54efb3b1ec68ec9ec78 Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Thu, 4 Jun 2020 10:53:39 +0100 Subject: [PATCH 1/2] Enable room version 6 (#1087) * Return bad request on CS API /send if bad JSON * Return some more M_BAD_JSON in the right places * nolint because damnit gocyclo all I added was a type check for an error * Update gomatrixserverlib * Update gomatrixserverlib * Update sytest-whitelist * Update gomatrixserverlib * Update sytest-whitelist * NotJSON -> BadJSON --- clientapi/routing/membership.go | 8 +++++++- clientapi/routing/profile.go | 20 ++++++++++++++++++-- clientapi/routing/sendevent.go | 5 +++++ federationapi/routing/join.go | 7 ++++++- federationapi/routing/leave.go | 5 +++++ go.mod | 2 +- go.sum | 4 ++-- sytest-whitelist | 12 ++++++++++++ 8 files changed, 56 insertions(+), 7 deletions(-) diff --git a/clientapi/routing/membership.go b/clientapi/routing/membership.go index cd0ce7328..74d92a059 100644 --- a/clientapi/routing/membership.go +++ b/clientapi/routing/membership.go @@ -276,7 +276,13 @@ func checkAndProcessThreepid( Code: http.StatusNotFound, JSON: jsonerror.NotFound(err.Error()), } - } else if err != nil { + } else if e, ok := err.(gomatrixserverlib.BadJSONError); ok { + return inviteStored, &util.JSONResponse{ + Code: http.StatusBadRequest, + JSON: jsonerror.BadJSON(e.Error()), + } + } + if err != nil { util.GetLogger(req.Context()).WithError(err).Error("threepid.CheckAndProcessInvite failed") er := jsonerror.InternalServerError() return inviteStored, &er diff --git a/clientapi/routing/profile.go b/clientapi/routing/profile.go index 1c1ee8036..dbf6ef1d9 100644 --- a/clientapi/routing/profile.go +++ b/clientapi/routing/profile.go @@ -91,6 +91,7 @@ func GetAvatarURL( } // SetAvatarURL implements PUT /profile/{userID}/avatar_url +// nolint:gocyclo func SetAvatarURL( req *http.Request, accountDB accounts.Database, device *authtypes.Device, userID string, producer *producers.UserUpdateProducer, cfg *config.Dendrite, @@ -156,7 +157,14 @@ func SetAvatarURL( events, err := buildMembershipEvents( req.Context(), memberships, newProfile, userID, cfg, evTime, rsAPI, ) - if err != nil { + switch e := err.(type) { + case nil: + case gomatrixserverlib.BadJSONError: + return util.JSONResponse{ + Code: http.StatusBadRequest, + JSON: jsonerror.BadJSON(e.Error()), + } + default: util.GetLogger(req.Context()).WithError(err).Error("buildMembershipEvents failed") return jsonerror.InternalServerError() } @@ -205,6 +213,7 @@ func GetDisplayName( } // SetDisplayName implements PUT /profile/{userID}/displayname +// nolint:gocyclo func SetDisplayName( req *http.Request, accountDB accounts.Database, device *authtypes.Device, userID string, producer *producers.UserUpdateProducer, cfg *config.Dendrite, @@ -270,7 +279,14 @@ func SetDisplayName( events, err := buildMembershipEvents( req.Context(), memberships, newProfile, userID, cfg, evTime, rsAPI, ) - if err != nil { + switch e := err.(type) { + case nil: + case gomatrixserverlib.BadJSONError: + return util.JSONResponse{ + Code: http.StatusBadRequest, + JSON: jsonerror.BadJSON(e.Error()), + } + default: util.GetLogger(req.Context()).WithError(err).Error("buildMembershipEvents failed") return jsonerror.InternalServerError() } diff --git a/clientapi/routing/sendevent.go b/clientapi/routing/sendevent.go index aa7317094..a8f8893ba 100644 --- a/clientapi/routing/sendevent.go +++ b/clientapi/routing/sendevent.go @@ -154,6 +154,11 @@ func generateSendEvent( Code: http.StatusNotFound, JSON: jsonerror.NotFound("Room does not exist"), } + } else if e, ok := err.(gomatrixserverlib.BadJSONError); ok { + return nil, &util.JSONResponse{ + Code: http.StatusBadRequest, + JSON: jsonerror.BadJSON(e.Error()), + } } else if err != nil { util.GetLogger(req.Context()).WithError(err).Error("internal.BuildEvent failed") resErr := jsonerror.InternalServerError() diff --git a/federationapi/routing/join.go b/federationapi/routing/join.go index 617a6cab0..5b4e8db3e 100644 --- a/federationapi/routing/join.go +++ b/federationapi/routing/join.go @@ -102,6 +102,11 @@ func MakeJoin( Code: http.StatusNotFound, JSON: jsonerror.NotFound("Room does not exist"), } + } else if e, ok := err.(gomatrixserverlib.BadJSONError); ok { + return util.JSONResponse{ + Code: http.StatusBadRequest, + JSON: jsonerror.BadJSON(e.Error()), + } } else if err != nil { util.GetLogger(httpReq.Context()).WithError(err).Error("internal.BuildEvent failed") return jsonerror.InternalServerError() @@ -157,7 +162,7 @@ func SendJoin( if err != nil { return util.JSONResponse{ Code: http.StatusBadRequest, - JSON: jsonerror.NotJSON("The request body could not be decoded into valid JSON. " + err.Error()), + JSON: jsonerror.BadJSON("The request body could not be decoded into valid JSON: " + err.Error()), } } diff --git a/federationapi/routing/leave.go b/federationapi/routing/leave.go index bd226d7ee..62ca11457 100644 --- a/federationapi/routing/leave.go +++ b/federationapi/routing/leave.go @@ -76,6 +76,11 @@ func MakeLeave( Code: http.StatusNotFound, JSON: jsonerror.NotFound("Room does not exist"), } + } else if e, ok := err.(gomatrixserverlib.BadJSONError); ok { + return util.JSONResponse{ + Code: http.StatusBadRequest, + JSON: jsonerror.BadJSON(e.Error()), + } } else if err != nil { util.GetLogger(httpReq.Context()).WithError(err).Error("internal.BuildEvent failed") return jsonerror.InternalServerError() diff --git a/go.mod b/go.mod index 021cc02a4..5510e8e0e 100644 --- a/go.mod +++ b/go.mod @@ -18,7 +18,7 @@ require ( github.com/matrix-org/go-http-js-libp2p v0.0.0-20200518170932-783164aeeda4 github.com/matrix-org/go-sqlite3-js v0.0.0-20200522092705-bc8506ccbcf3 github.com/matrix-org/gomatrix v0.0.0-20190528120928-7df988a63f26 - github.com/matrix-org/gomatrixserverlib v0.0.0-20200602125825-24ff01093eca + github.com/matrix-org/gomatrixserverlib v0.0.0-20200604085359-baf0c20ac96f github.com/matrix-org/naffka v0.0.0-20200422140631-181f1ee7401f github.com/matrix-org/util v0.0.0-20190711121626-527ce5ddefc7 github.com/mattn/go-sqlite3 v2.0.2+incompatible diff --git a/go.sum b/go.sum index 3f235d51e..5df03cbaa 100644 --- a/go.sum +++ b/go.sum @@ -356,8 +356,8 @@ github.com/matrix-org/go-sqlite3-js v0.0.0-20200522092705-bc8506ccbcf3 h1:Yb+Wlf github.com/matrix-org/go-sqlite3-js v0.0.0-20200522092705-bc8506ccbcf3/go.mod h1:e+cg2q7C7yE5QnAXgzo512tgFh1RbQLC0+jozuegKgo= github.com/matrix-org/gomatrix v0.0.0-20190528120928-7df988a63f26 h1:Hr3zjRsq2bhrnp3Ky1qgx/fzCtCALOoGYylh2tpS9K4= github.com/matrix-org/gomatrix v0.0.0-20190528120928-7df988a63f26/go.mod h1:3fxX6gUjWyI/2Bt7J1OLhpCzOfO/bB3AiX0cJtEKud0= -github.com/matrix-org/gomatrixserverlib v0.0.0-20200602125825-24ff01093eca h1:s/dJePRDKjD1fGeoTnEYFqPmp1v7fC6GTd6iFwCKxw8= -github.com/matrix-org/gomatrixserverlib v0.0.0-20200602125825-24ff01093eca/go.mod h1:JsAzE1Ll3+gDWS9JSUHPJiiyAksvOOnGWF2nXdg4ZzU= +github.com/matrix-org/gomatrixserverlib v0.0.0-20200604085359-baf0c20ac96f h1:G0B8Yu/5RVg4PiNhr9Adw3g7RLmQ95wnOL7bz6tUKrU= +github.com/matrix-org/gomatrixserverlib v0.0.0-20200604085359-baf0c20ac96f/go.mod h1:JsAzE1Ll3+gDWS9JSUHPJiiyAksvOOnGWF2nXdg4ZzU= github.com/matrix-org/naffka v0.0.0-20200422140631-181f1ee7401f h1:pRz4VTiRCO4zPlEMc3ESdUOcW4PXHH4Kj+YDz1XyE+Y= github.com/matrix-org/naffka v0.0.0-20200422140631-181f1ee7401f/go.mod h1:y0oDTjZDv5SM9a2rp3bl+CU+bvTRINQsdb7YlDql5Go= github.com/matrix-org/util v0.0.0-20190711121626-527ce5ddefc7 h1:ntrLa/8xVzeSs8vHFHK25k0C+NV74sYMJnNSg5NoSRo= diff --git a/sytest-whitelist b/sytest-whitelist index 6236b28ed..c1c0c13ac 100644 --- a/sytest-whitelist +++ b/sytest-whitelist @@ -301,3 +301,15 @@ Can send messages with a wildcard device id to two devices Wildcard device messages wake up /sync # TODO: separate PR for: Wildcard device messages over federation wake up /sync Can send a to-device message to two users which both receive it using /sync +User can create and send/receive messages in a room with version 6 +local user can join room with version 6 +User can invite local user to room with version 6 +remote user can join room with version 6 +User can invite remote user to room with version 6 +Remote user can backfill in a room with version 6 +Inbound: send_join rejects invalid JSON for room version 6 +Outbound federation rejects backfill containing invalid JSON for events in room version 6 +Invalid JSON integers +Invalid JSON special values +Invalid JSON floats +Outbound federation will ignore a missing event with bad JSON for room version 6 From 225b72bd42fb104bccd4a6b98e770b7473eb00f7 Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Thu, 4 Jun 2020 10:54:10 +0100 Subject: [PATCH 2/2] Don't reset counters before successful outgoing federation request (#1089) * Don't reset counters before successful outgoing federation request on incoming federation request * Comments --- federationsender/queue/destinationqueue.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/federationsender/queue/destinationqueue.go b/federationsender/queue/destinationqueue.go index 4ab610de7..bf9042f45 100644 --- a/federationsender/queue/destinationqueue.go +++ b/federationsender/queue/destinationqueue.go @@ -59,8 +59,11 @@ func (oq *destinationQueue) retry() { // and then skip ahead a lot which feels non-ideal but equally we can't persist thousands of events // in-memory to maybe-send it one day. Ideally we would just shove these pending events in a database // so we can send a lot of events. - oq.statistics.Success() - // if we were backing off, swap to not backing off and interrupt the select. + // + // Interrupt the backoff. If the federation request that happens as a result of this is successful + // then the counters will be reset there and the backoff will cancel. If the federation request + // fails then we will retry at the current backoff interval, so as to prevent us from spamming + // homeservers which are behaving badly. // We need to use an atomic bool here to prevent multiple calls to retry() blocking on the channel // as it is unbuffered. if oq.backingOff.CAS(true, false) {