Default to the default timeline limit if it's unset, also strip the extra event correctly

syncapi/routing/filter.go

@@ -15,14 +15,17 @@
 package routing
 
 import (
+	"encoding/json"
+	"io/ioutil"
 	"net/http"
 
-	"github.com/matrix-org/dendrite/clientapi/httputil"
 	"github.com/matrix-org/dendrite/clientapi/jsonerror"
 	"github.com/matrix-org/dendrite/syncapi/storage"
+	"github.com/matrix-org/dendrite/syncapi/sync"
 	"github.com/matrix-org/dendrite/userapi/api"
 	"github.com/matrix-org/gomatrixserverlib"
 	"github.com/matrix-org/util"
+	"github.com/tidwall/gjson"
 )
 
 // GetFilter implements GET /_matrix/client/r0/user/{userId}/filter/{filterId}
@@ -81,8 +84,27 @@ func PutFilter(
 
 	var filter gomatrixserverlib.Filter
 
-	if reqErr := httputil.UnmarshalJSONRequest(req, &filter); reqErr != nil {
+	defer req.Body.Close() // nolint:errcheck
-		return *reqErr
+	body, err := ioutil.ReadAll(req.Body)
+	if err != nil {
+		return util.JSONResponse{
+			Code: http.StatusBadRequest,
+			JSON: jsonerror.BadJSON("The request body could not be read. " + err.Error()),
+		}
+	}
+
+	if err = json.Unmarshal(body, &filter); err != nil {
+		return util.JSONResponse{
+			Code: http.StatusBadRequest,
+			JSON: jsonerror.BadJSON("The request body could not be decoded into valid JSON. " + err.Error()),
+		}
+	}
+	// the filter `limit` is `int` which defaults to 0 if not set which is not what we want. We want to use the default
+	// limit if it is unset, which is what this does.
+	limitRes := gjson.GetBytes(body, "room.timeline.limit")
+	if !limitRes.Exists() {
+		util.GetLogger(req.Context()).Infof("missing timeline limit, using default")
+		filter.Room.Timeline.Limit = sync.DefaultTimelineLimit
 	}
 
 	// Validate generates a user-friendly error

syncapi/storage/postgres/output_room_events_table.go

@@ -317,13 +317,6 @@ func (s *outputRoomEventsStatements) SelectRecentEvents(
 	if err != nil {
 		return nil, false, err
 	}
-	// we queried for 1 more than the limit, so if we returned one more mark limited=true
-	limited := false
-	if len(events) > limit {
-		limited = true
-		// re-slice the extra event out
-		events = events[:len(events)-1]
-	}
 	if chronologicalOrder {
 		// The events need to be returned from oldest to latest, which isn't
 		// necessary the way the SQL query returns them, so a sort is necessary to
@@ -332,6 +325,17 @@ func (s *outputRoomEventsStatements) SelectRecentEvents(
 			return events[i].StreamPosition < events[j].StreamPosition
 		})
 	}
+	// we queried for 1 more than the limit, so if we returned one more mark limited=true
+	limited := false
+	if len(events) > limit {
+		limited = true
+		// re-slice the extra (oldest) event out: in chronological order this is the first entry, else the last.
+		if chronologicalOrder {
+			events = events[1:]
+		} else {
+			events = events[:len(events)-1]
+		}
+	}
 
 	return events, limited, nil
 }

syncapi/storage/sqlite3/output_room_events_table.go

@@ -328,13 +328,6 @@ func (s *outputRoomEventsStatements) SelectRecentEvents(
 	if err != nil {
 		return nil, false, err
 	}
-	// we queried for 1 more than the limit, so if we returned one more mark limited=true
-	limited := false
-	if len(events) > limit {
-		limited = true
-		// re-slice the extra event out
-		events = events[:len(events)-1]
-	}
 	if chronologicalOrder {
 		// The events need to be returned from oldest to latest, which isn't
 		// necessary the way the SQL query returns them, so a sort is necessary to
@@ -343,6 +336,17 @@ func (s *outputRoomEventsStatements) SelectRecentEvents(
 			return events[i].StreamPosition < events[j].StreamPosition
 		})
 	}
+	// we queried for 1 more than the limit, so if we returned one more mark limited=true
+	limited := false
+	if len(events) > limit {
+		limited = true
+		// re-slice the extra (oldest) event out: in chronological order this is the first entry, else the last.
+		if chronologicalOrder {
+			events = events[1:]
+		} else {
+			events = events[:len(events)-1]
+		}
+	}
 	return events, limited, nil
 }
 

syncapi/sync/notifier_test.go

@@ -363,7 +363,7 @@ func newTestSyncRequest(userID, deviceID string, since types.StreamingToken) syn
 		timeout:       1 * time.Minute,
 		since:         &since,
 		wantFullState: false,
-		limit:         defaultTimelineLimit,
+		limit:         DefaultTimelineLimit,
 		log:           util.GetLogger(context.TODO()),
 		ctx:           context.TODO(),
 	}

syncapi/sync/request.go

@@ -30,7 +30,7 @@ import (
 )
 
 const defaultSyncTimeout = time.Duration(0)
-const defaultTimelineLimit = 20
+const DefaultTimelineLimit = 20
 
 type filter struct {
 	Room struct {
@@ -68,7 +68,7 @@ func newSyncRequest(req *http.Request, device userapi.Device, syncDB storage.Dat
 		tok := types.NewStreamToken(0, 0)
 		since = &tok
 	}
-	timelineLimit := defaultTimelineLimit
+	timelineLimit := DefaultTimelineLimit
 	// TODO: read from stored filters too
 	filterQuery := req.URL.Query().Get("filter")
 	if filterQuery != "" {