From 9af5e73fdffcae776ad328e39e140e1b649566b3 Mon Sep 17 00:00:00 2001 From: "Crom (Thibaut CHARLES)" Date: Sat, 23 Dec 2017 15:09:29 +0100 Subject: [PATCH] Filter unmarshal & validation before storing Signed-off-by: Thibaut CHARLES cromfr@gmail.com --- .../auth/storage/accounts/filter_table.go | 22 ++++++++++++++----- .../auth/storage/accounts/storage.go | 5 +++-- .../dendrite/clientapi/routing/filter.go | 17 +++++++------- .../github.com/matrix-org/gomatrix/filter.go | 9 ++++++++ 4 files changed, 36 insertions(+), 17 deletions(-) diff --git a/src/github.com/matrix-org/dendrite/clientapi/auth/storage/accounts/filter_table.go b/src/github.com/matrix-org/dendrite/clientapi/auth/storage/accounts/filter_table.go index 81bae4545..8b7c6af22 100644 --- a/src/github.com/matrix-org/dendrite/clientapi/auth/storage/accounts/filter_table.go +++ b/src/github.com/matrix-org/dendrite/clientapi/auth/storage/accounts/filter_table.go @@ -17,8 +17,9 @@ package accounts import ( "context" "database/sql" + "encoding/json" - "github.com/matrix-org/gomatrixserverlib" + "github.com/matrix-org/gomatrix" ) const filterSchema = ` @@ -71,20 +72,29 @@ func (s *filterStatements) prepare(db *sql.DB) (err error) { func (s *filterStatements) selectFilter( ctx context.Context, localpart string, filterID string, -) (filter []byte, err error) { - err = s.selectFilterStmt.QueryRowContext(ctx, localpart, filterID).Scan(&filter) - return +) (*gomatrix.Filter, error) { + var filterData []byte + err := s.selectFilterStmt.QueryRowContext(ctx, localpart, filterID).Scan(&filterData) + if err != nil { + return nil, err + } + + var filter gomatrix.Filter + if err = json.Unmarshal(filterData, &filter); err != nil { + return nil, err + } + return &filter, err } func (s *filterStatements) insertFilter( - ctx context.Context, filter []byte, localpart string, + ctx context.Context, filter *gomatrix.Filter, localpart string, ) (filterID string, err error) { var existingFilterID string // This can result in a race condition when two clients try to insert the // same filter and localpart at the same time, however this is not a // problem as both calls will result in the same filterID - filterJSON, err := gomatrixserverlib.CanonicalJSON(filter) + filterJSON, err := json.Marshal(filter) if err != nil { return "", err } diff --git a/src/github.com/matrix-org/dendrite/clientapi/auth/storage/accounts/storage.go b/src/github.com/matrix-org/dendrite/clientapi/auth/storage/accounts/storage.go index e88942e34..652983529 100644 --- a/src/github.com/matrix-org/dendrite/clientapi/auth/storage/accounts/storage.go +++ b/src/github.com/matrix-org/dendrite/clientapi/auth/storage/accounts/storage.go @@ -21,6 +21,7 @@ import ( "github.com/matrix-org/dendrite/clientapi/auth/authtypes" "github.com/matrix-org/dendrite/common" + "github.com/matrix-org/gomatrix" "github.com/matrix-org/gomatrixserverlib" "golang.org/x/crypto/bcrypt" // Import the postgres database driver. @@ -329,7 +330,7 @@ func (d *Database) GetThreePIDsForLocalpart( // no such filter exists or if there was an error talking to the database. func (d *Database) GetFilter( ctx context.Context, localpart string, filterID string, -) ([]byte, error) { +) (*gomatrix.Filter, error) { return d.filter.selectFilter(ctx, localpart, filterID) } @@ -337,7 +338,7 @@ func (d *Database) GetFilter( // Returns the filterID as a string. Otherwise returns an error if something // goes wrong. func (d *Database) PutFilter( - ctx context.Context, localpart string, filter []byte, + ctx context.Context, localpart string, filter *gomatrix.Filter, ) (string, error) { return d.filter.insertFilter(ctx, filter, localpart) } diff --git a/src/github.com/matrix-org/dendrite/clientapi/routing/filter.go b/src/github.com/matrix-org/dendrite/clientapi/routing/filter.go index 4b84e293d..d14aaeb96 100644 --- a/src/github.com/matrix-org/dendrite/clientapi/routing/filter.go +++ b/src/github.com/matrix-org/dendrite/clientapi/routing/filter.go @@ -49,7 +49,7 @@ func GetFilter( return httputil.LogThenError(req, err) } - res, err := accountDB.GetFilter(req.Context(), localpart, filterID) + filter, err := accountDB.GetFilter(req.Context(), localpart, filterID) if err != nil { //TODO better error handling. This error message is *probably* right, // but if there are obscure db errors, this will also be returned, @@ -59,15 +59,15 @@ func GetFilter( JSON: jsonerror.NotFound("No such filter"), } } - filter := gomatrix.Filter{} - err = json.Unmarshal(res, &filter) + + filterJSON, err := json.Marshal(filter) if err != nil { - httputil.LogThenError(req, err) + return httputil.LogThenError(req, err) } return util.JSONResponse{ Code: 200, - JSON: filter, + JSON: filterJSON, } } @@ -103,15 +103,14 @@ func PutFilter( return *reqErr } - filterArray, err := json.Marshal(filter) - if err != nil { + if err = filter.Validate(); err != nil { return util.JSONResponse{ Code: 400, - JSON: jsonerror.BadJSON("Filter is malformed"), + JSON: jsonerror.BadJSON("Invalid filter: " + err.Error()), } } - filterID, err := accountDB.PutFilter(req.Context(), localpart, filterArray) + filterID, err := accountDB.PutFilter(req.Context(), localpart, &filter) if err != nil { return httputil.LogThenError(req, err) } diff --git a/vendor/src/github.com/matrix-org/gomatrix/filter.go b/vendor/src/github.com/matrix-org/gomatrix/filter.go index e4e762873..3aa65fa24 100644 --- a/vendor/src/github.com/matrix-org/gomatrix/filter.go +++ b/vendor/src/github.com/matrix-org/gomatrix/filter.go @@ -14,6 +14,8 @@ package gomatrix +import "errors" + //Filter is used by clients to specify how the server should filter responses to e.g. sync requests //Specified by: https://matrix.org/docs/spec/client_server/r0.2.0.html#filtering type Filter struct { @@ -41,3 +43,10 @@ type FilterPart struct { Senders []string `json:"senders,omitempty"` Types []string `json:"types,omitempty"` } + +func (filter *Filter) Validate() error { + if filter.EventFormat != "client" && filter.EventFormat != "federation" { + return errors.New("Bad event_format value. Must be any of [\"client\", \"federation\"]") + } + return nil +}