From f12fbca3c559ad9f441c497f9659fdfcce6208db Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Timo=20K=C3=B6sters?= <timo@koesters.xyz>
Date: Tue, 22 Dec 2020 20:08:20 +0100
Subject: [PATCH] fix: send state in /sync, element displays wrong membership
 changes

---
 src/client_server/membership.rs |  2 +-
 src/client_server/sync.rs       | 34 +++++++++++++++++++++-----------
 src/database/rooms.rs           | 35 +++++++++++++++++++++++++++++----
 src/server_server.rs            |  2 +-
 4 files changed, 55 insertions(+), 18 deletions(-)

diff --git a/src/client_server/membership.rs b/src/client_server/membership.rs
index e8d57bc..b459d37 100644
--- a/src/client_server/membership.rs
+++ b/src/client_server/membership.rs
@@ -686,7 +686,7 @@ async fn join_room_by_id_helper(
             pdu_id.extend_from_slice(&count.to_be_bytes());
             db.rooms.append_pdu(
                 &PduEvent::from(&**pdu),
-                &utils::to_canonical_object(&**pdu).expect("Pdu is valid canonical object"),
+                utils::to_canonical_object(&**pdu).expect("Pdu is valid canonical object"),
                 count,
                 pdu_id.clone().into(),
                 &db.globals,
diff --git a/src/client_server/sync.rs b/src/client_server/sync.rs
index 8213651..3136116 100644
--- a/src/client_server/sync.rs
+++ b/src/client_server/sync.rs
@@ -91,15 +91,7 @@ pub async fn sync_events_route(
 
         // They /sync response doesn't always return all messages, so we say the output is
         // limited unless there are events in non_timeline_pdus
-        let mut limited = false;
-
-        let mut state_pdus = Vec::new();
-        for (_, pdu) in non_timeline_pdus {
-            if pdu.state_key.is_some() {
-                state_pdus.push(pdu);
-            }
-            limited = true;
-        }
+        let limited = non_timeline_pdus.next().is_some();
 
         // Database queries:
 
@@ -342,7 +334,7 @@ pub async fn sync_events_route(
             })?;
 
         let room_events = timeline_pdus
-            .into_iter()
+            .iter()
             .map(|(_, pdu)| pdu.to_sync_room_event())
             .collect::<Vec<_>>();
 
@@ -392,7 +384,6 @@ pub async fn sync_events_route(
                 prev_batch,
                 events: room_events,
             },
-            // TODO: state before timeline
             state: sync_events::State {
                 events: if joined_since_last_sync {
                     db.rooms
@@ -401,7 +392,26 @@ pub async fn sync_events_route(
                         .map(|(_, pdu)| pdu.to_sync_state_event())
                         .collect()
                 } else {
-                    Vec::new()
+                    match since_state {
+                        None => Vec::new(),
+                        Some(Some(since_state)) => current_state
+                            .iter()
+                            .filter(|(key, value)| {
+                                since_state.get(key).map(|e| &e.event_id) != Some(&value.event_id)
+                            })
+                            .filter(|(_, value)| {
+                                !timeline_pdus.iter().any(|(_, timeline_pdu)| {
+                                    timeline_pdu.kind == value.kind
+                                        && timeline_pdu.state_key == value.state_key
+                                })
+                            })
+                            .map(|(_, pdu)| pdu.to_sync_state_event())
+                            .collect(),
+                        Some(None) => current_state
+                            .iter()
+                            .map(|(_, pdu)| pdu.to_sync_state_event())
+                            .collect(),
+                    }
                 },
             },
             ephemeral: sync_events::Ephemeral { events: edus },
diff --git a/src/database/rooms.rs b/src/database/rooms.rs
index 3f096a9..e59c77f 100644
--- a/src/database/rooms.rs
+++ b/src/database/rooms.rs
@@ -15,7 +15,7 @@ use ruma::{
         },
         EventType,
     },
-    serde::{to_canonical_value, CanonicalJsonObject, Raw},
+    serde::{to_canonical_value, CanonicalJsonObject, CanonicalJsonValue, Raw},
     EventId, RoomAliasId, RoomId, RoomVersionId, ServerName, UserId,
 };
 use sled::IVec;
@@ -444,13 +444,40 @@ impl Rooms {
     pub fn append_pdu(
         &self,
         pdu: &PduEvent,
-        pdu_json: &CanonicalJsonObject,
+        mut pdu_json: CanonicalJsonObject,
         count: u64,
         pdu_id: IVec,
         globals: &super::globals::Globals,
         account_data: &super::account_data::AccountData,
         admin: &super::admin::Admin,
     ) -> Result<()> {
+        // Make unsigned fields correct. This is not properly documented in the spec, but state
+        // events need to have previous content in the unsigned field, so clients can easily
+        // interpret things like membership changes
+        if let Some(state_key) = &pdu.state_key {
+            if let CanonicalJsonValue::Object(unsigned) = pdu_json
+                .entry("unsigned".to_owned())
+                .or_insert_with(|| CanonicalJsonValue::Object(Default::default()))
+            {
+                if let Some(prev_state_hash) = self.pdu_state_hash(&pdu_id).unwrap() {
+                    if let Some(prev_state) = self
+                        .state_get(&pdu.room_id, &prev_state_hash, &pdu.kind, &state_key)
+                        .unwrap()
+                    {
+                        unsigned.insert(
+                            "prev_content".to_owned(),
+                            CanonicalJsonValue::Object(
+                                utils::to_canonical_object(prev_state.1.content)
+                                    .expect("event is valid, we just created it"),
+                            ),
+                        );
+                    }
+                }
+            } else {
+                error!("Invalid unsigned type in pdu.");
+            }
+        }
+
         self.replace_pdu_leaves(&pdu.room_id, &pdu.event_id)?;
 
         // Mark as read first so the sending client doesn't get a notification even if appending
@@ -460,7 +487,7 @@ impl Rooms {
 
         self.pduid_pdu.insert(
             &pdu_id,
-            &*serde_json::to_string(pdu_json)
+            &*serde_json::to_string(&pdu_json)
                 .expect("CanonicalJsonObject is always a valid String"),
         )?;
 
@@ -905,7 +932,7 @@ impl Rooms {
 
         self.append_pdu(
             &pdu,
-            &pdu_json,
+            pdu_json,
             count,
             pdu_id.clone().into(),
             globals,
diff --git a/src/server_server.rs b/src/server_server.rs
index 0653959..7abce5a 100644
--- a/src/server_server.rs
+++ b/src/server_server.rs
@@ -494,7 +494,7 @@ pub async fn send_transaction_message_route<'a>(
 
         db.rooms.append_pdu(
             &pdu,
-            &value,
+            value,
             count,
             pdu_id.clone().into(),
             &db.globals,