From 55cfe391f726a06c6303a7bf71ffee28d00f9f33 Mon Sep 17 00:00:00 2001
From: Neil Alexander <neilalexander@users.noreply.github.com>
Date: Wed, 13 Jan 2021 12:59:29 +0000
Subject: [PATCH] Update /messages pagination token behaviour (#1708)

* Tweak pagination tokens

* start should be the specified from

* Don't reverse start and end

* Tweak getStartEnd again

* Update sytest-whitelist

* NOTSPEC: Re-add iOS end of topology
---
 syncapi/routing/messages.go | 57 ++++++++++++++++++-------------------
 sytest-whitelist            |  2 ++
 2 files changed, 30 insertions(+), 29 deletions(-)

diff --git a/syncapi/routing/messages.go b/syncapi/routing/messages.go
index 14389ebbf..e294c8803 100644
--- a/syncapi/routing/messages.go
+++ b/syncapi/routing/messages.go
@@ -273,6 +273,14 @@ func (r *messagesReq) retrieveEvents() (
 		return []gomatrixserverlib.ClientEvent{}, *r.from, *r.to, nil
 	}
 
+	// Get the position of the first and the last event in the room's topology.
+	// This position is currently determined by the event's depth, so we could
+	// also use it instead of retrieving from the database. However, if we ever
+	// change the way topological positions are defined (as depth isn't the most
+	// reliable way to define it), it would be easier and less troublesome to
+	// only have to change it in one place, i.e. the database.
+	start, end, err = r.getStartEnd(events)
+
 	// Sort the events to ensure we send them in the right order.
 	if r.backwardOrdering {
 		// This reverses the array from old->new to new->old
@@ -292,14 +300,6 @@ func (r *messagesReq) retrieveEvents() (
 
 	// Convert all of the events into client events.
 	clientEvents = gomatrixserverlib.HeaderedToClientEvents(events, gomatrixserverlib.FormatAll)
-	// Get the position of the first and the last event in the room's topology.
-	// This position is currently determined by the event's depth, so we could
-	// also use it instead of retrieving from the database. However, if we ever
-	// change the way topological positions are defined (as depth isn't the most
-	// reliable way to define it), it would be easier and less troublesome to
-	// only have to change it in one place, i.e. the database.
-	start, end, err = r.getStartEnd(events)
-
 	return clientEvents, start, end, err
 }
 
@@ -363,7 +363,7 @@ func (r *messagesReq) filterHistoryVisible(events []*gomatrixserverlib.HeaderedE
 			return events // apply no filtering as it defaults to Shared.
 		}
 		hisVis, _ := hisVisEvent.HistoryVisibility()
-		if hisVis == "shared" {
+		if hisVis == "shared" || hisVis == "world_readable" {
 			return events // apply no filtering
 		}
 		if membershipEvent == nil {
@@ -388,26 +388,16 @@ func (r *messagesReq) filterHistoryVisible(events []*gomatrixserverlib.HeaderedE
 }
 
 func (r *messagesReq) getStartEnd(events []*gomatrixserverlib.HeaderedEvent) (start, end types.TopologyToken, err error) {
-	start, err = r.db.EventPositionInTopology(
-		r.ctx, events[0].EventID(),
-	)
-	if err != nil {
-		err = fmt.Errorf("EventPositionInTopology: for start event %s: %w", events[0].EventID(), err)
-		return
-	}
-	if r.backwardOrdering && events[len(events)-1].Type() == gomatrixserverlib.MRoomCreate {
-		// We've hit the beginning of the room so there's really nowhere else
-		// to go. This seems to fix Riot iOS from looping on /messages endlessly.
-		end = types.TopologyToken{}
-	} else {
-		end, err = r.db.EventPositionInTopology(
-			r.ctx, events[len(events)-1].EventID(),
-		)
-		if err != nil {
-			err = fmt.Errorf("EventPositionInTopology: for end event %s: %w", events[len(events)-1].EventID(), err)
-			return
-		}
-		if r.backwardOrdering {
+	if r.backwardOrdering {
+		start = *r.from
+		if events[len(events)-1].Type() == gomatrixserverlib.MRoomCreate {
+			// NOTSPEC: We've hit the beginning of the room so there's really nowhere
+			// else to go. This seems to fix Riot iOS from looping on /messages endlessly.
+			end = types.TopologyToken{}
+		} else {
+			end, err = r.db.EventPositionInTopology(
+				r.ctx, events[0].EventID(),
+			)
 			// A stream/topological position is a cursor located between two events.
 			// While they are identified in the code by the event on their right (if
 			// we consider a left to right chronological order), tokens need to refer
@@ -415,6 +405,15 @@ func (r *messagesReq) getStartEnd(events []*gomatrixserverlib.HeaderedEvent) (st
 			// end position we send in the response if we're going backward.
 			end.Decrement()
 		}
+	} else {
+		start = *r.from
+		end, err = r.db.EventPositionInTopology(
+			r.ctx, events[len(events)-1].EventID(),
+		)
+	}
+	if err != nil {
+		err = fmt.Errorf("EventPositionInTopology: for end event %s: %w", events[len(events)-1].EventID(), err)
+		return
 	}
 	return
 }
diff --git a/sytest-whitelist b/sytest-whitelist
index cfbe54434..cb84913b8 100644
--- a/sytest-whitelist
+++ b/sytest-whitelist
@@ -501,3 +501,5 @@ Can forget room you've been kicked from
 /joined_members return joined members
 A next_batch token can be used in the v1 messages API
 Users receive device_list updates for their own devices
+m.room.history_visibility == "world_readable" allows/forbids appropriately for Guest users
+m.room.history_visibility == "world_readable" allows/forbids appropriately for Real users