sync: Avoid crash on invalid state events

The problem was in matrix_sdk_sqlite::state_store::SqliteStateStore save_changes, which saved the RoomInfo to the disk by serializing it.
Now the events are serialized as Raws using a separate struct.

crates/matrix-sdk-base/src/lib.rs

@@ -50,8 +50,8 @@ pub use http;
 pub use matrix_sdk_crypto as crypto;
 pub use once_cell;
 pub use rooms::{
-    DisplayName, Room, RoomCreateWithCreatorEventContent, RoomInfo, RoomInfoUpdate, RoomMember,
-    RoomMemberships, RoomState, RoomStateFilter,
+    DisplayName, RawRoomInfo, Room, RoomCreateWithCreatorEventContent, RoomInfo, RoomInfoUpdate,
+    RoomMember, RoomMemberships, RoomState, RoomStateFilter,
 };
 pub use store::{StateChanges, StateStore, StateStoreDataKey, StateStoreDataValue, StoreError};
 pub use utils::{

crates/matrix-sdk-base/src/rooms/mod.rs

@@ -22,9 +22,9 @@ use ruma::{
             canonical_alias::RoomCanonicalAliasEventContent,
             create::{PreviousRoom, RoomCreateEventContent},
             encryption::RoomEncryptionEventContent,
-            guest_access::RoomGuestAccessEventContent,
-            history_visibility::RoomHistoryVisibilityEventContent,
-            join_rules::RoomJoinRulesEventContent,
+            guest_access::{GuestAccess, RoomGuestAccessEventContent},
+            history_visibility::{HistoryVisibility, RoomHistoryVisibilityEventContent},
+            join_rules::{JoinRule, RoomJoinRulesEventContent},
             member::MembershipState,
             name::RoomNameEventContent,
             tombstone::RoomTombstoneEventContent,
@@ -35,6 +35,7 @@ use ruma::{
         RedactedStateEventContent, StaticStateEventContent, SyncStateEvent,
     },
     room::RoomType,
+    serde::Raw,
     EventId, OwnedUserId, RoomVersionId,
 };
 use serde::{Deserialize, Serialize};
@@ -77,33 +78,44 @@ impl fmt::Display for DisplayName {
 #[derive(Clone, Debug, Serialize, Deserialize)]
 pub struct BaseRoomInfo {
     /// The avatar URL of this room.
+    #[serde(skip_serializing)]
     pub(crate) avatar: Option<MinimalStateEvent<RoomAvatarEventContent>>,
     /// The canonical alias of this room.
+    #[serde(skip_serializing)]
     pub(crate) canonical_alias: Option<MinimalStateEvent<RoomCanonicalAliasEventContent>>,
     /// The `m.room.create` event content of this room.
+    #[serde(skip_serializing)]
     pub(crate) create: Option<MinimalStateEvent<RoomCreateWithCreatorEventContent>>,
     /// A list of user ids this room is considered as direct message, if this
     /// room is a DM.
     pub(crate) dm_targets: HashSet<OwnedUserId>,
     /// The `m.room.encryption` event content that enabled E2EE in this room.
+    #[serde(skip_serializing)]
     pub(crate) encryption: Option<RoomEncryptionEventContent>,
     /// The guest access policy of this room.
+    #[serde(skip_serializing)]
     pub(crate) guest_access: Option<MinimalStateEvent<RoomGuestAccessEventContent>>,
     /// The history visibility policy of this room.
+    #[serde(skip_serializing)]
     pub(crate) history_visibility: Option<MinimalStateEvent<RoomHistoryVisibilityEventContent>>,
     /// The join rule policy of this room.
+    #[serde(skip_serializing)]
     pub(crate) join_rules: Option<MinimalStateEvent<RoomJoinRulesEventContent>>,
     /// The maximal power level that can be found in this room.
     pub(crate) max_power_level: i64,
     /// The `m.room.name` of this room.
+    #[serde(skip_serializing)]
     pub(crate) name: Option<MinimalStateEvent<RoomNameEventContent>>,
     /// The `m.room.tombstone` event content of this room.
+    #[serde(skip_serializing)]
     pub(crate) tombstone: Option<MinimalStateEvent<RoomTombstoneEventContent>>,
     /// The topic of this room.
+    #[serde(skip_serializing)]
     pub(crate) topic: Option<MinimalStateEvent<RoomTopicEventContent>>,
     /// All minimal state events that containing one or more running matrixRTC
     /// memberships.
-    #[serde(skip_serializing_if = "BTreeMap::is_empty", default)]
+    // #[serde(skip_serializing_if = "BTreeMap::is_empty", default)]
+    #[serde(skip_serializing, default)]
     pub(crate) rtc_member: BTreeMap<OwnedUserId, MinimalStateEvent<CallMemberEventContent>>,
     /// Whether this room has been manually marked as unread.
     #[serde(default)]
@@ -116,6 +128,34 @@ pub struct BaseRoomInfo {
     pub(crate) notable_tags: RoomNotableTags,
 }
 
+#[derive(Clone, Debug, Default, Serialize, Deserialize)]
+pub struct RawRoomInfo {
+    /// The avatar URL of this room.
+    pub avatar: Option<Raw<AnySyncStateEvent>>,
+    /// The canonical alias of this room.
+    pub canonical_alias: Option<Raw<AnySyncStateEvent>>,
+    /// The `m.room.create` event content of this room.
+    pub create: Option<Raw<AnySyncStateEvent>>,
+    /// The `m.room.encryption` event content that enabled E2EE in this room.
+    pub encryption: Option<Raw<AnySyncStateEvent>>,
+    /// The guest access policy of this room.
+    pub guest_access: Option<Raw<AnySyncStateEvent>>,
+    /// The history visibility policy of this room.
+    pub history_visibility: Option<Raw<AnySyncStateEvent>>,
+    /// The join rule policy of this room.
+    pub join_rules: Option<Raw<AnySyncStateEvent>>,
+    /// The `m.room.name` of this room.
+    pub name: Option<Raw<AnySyncStateEvent>>,
+    /// The `m.room.tombstone` event content of this room.
+    pub tombstone: Option<Raw<AnySyncStateEvent>>,
+    /// The topic of this room.
+    pub topic: Option<Raw<AnySyncStateEvent>>,
+    /// All minimal state events that containing one or more running matrixRTC
+    /// memberships.
+    #[serde(skip_serializing_if = "BTreeMap::is_empty", default)]
+    pub rtc_member: BTreeMap<OwnedUserId, Raw<AnySyncStateEvent>>,
+}
+
 impl BaseRoomInfo {
     /// Create a new, empty base room info.
     pub fn new() -> Self {
@@ -190,7 +230,7 @@ impl BaseRoomInfo {
                     return false;
                 };
 
-                // we modify the event so that `origin_sever_ts` gets copied into
+                // we modify the event so that `origin_server_ts` gets copied into
                 // `content.created_ts`
                 let mut o_ev = o_ev.clone();
                 o_ev.content.set_created_ts_if_none(o_ev.origin_server_ts);

crates/matrix-sdk-base/src/rooms/normal.rs

@@ -68,7 +68,7 @@ use crate::{
     read_receipts::RoomReadReceipts,
     store::{DynStateStore, Result as StoreResult, StateStoreExt},
     sync::UnreadNotificationsCount,
-    MinimalStateEvent, OriginalMinimalStateEvent, RoomMemberships,
+    MinimalStateEvent, OriginalMinimalStateEvent, RawRoomInfo, RoomMemberships,
 };
 
 /// A summary of changes to room information.
@@ -886,6 +886,44 @@ impl RoomInfo {
         }
     }
 
+    pub fn replace_by_raw(&mut self, raws: RawRoomInfo) {
+        if let Some(event) = raws.avatar.and_then(|e| e.deserialize_as().ok()) {
+            self.base_info.avatar = Some(event);
+        }
+        if let Some(event) = raws.canonical_alias.and_then(|e| e.deserialize_as().ok()) {
+            self.base_info.canonical_alias = Some(event);
+        }
+        if let Some(event) = raws.create.and_then(|e| e.deserialize_as().ok()) {
+            self.base_info.create = Some(event);
+        }
+        if let Some(event) = raws.encryption.and_then(|e| e.deserialize_as().ok()) {
+            self.base_info.encryption = Some(event);
+        }
+        if let Some(event) = raws.guest_access.and_then(|e| e.deserialize_as().ok()) {
+            self.base_info.guest_access = Some(event);
+        }
+        if let Some(event) = raws.history_visibility.and_then(|e| e.deserialize_as().ok()) {
+            self.base_info.history_visibility = Some(event);
+        }
+        if let Some(event) = raws.join_rules.and_then(|e| e.deserialize_as().ok()) {
+            self.base_info.join_rules = Some(event);
+        }
+        if let Some(event) = raws.name.and_then(|e| e.deserialize_as().ok()) {
+            self.base_info.name = Some(event);
+        }
+        if let Some(event) = raws.tombstone.and_then(|e| e.deserialize_as().ok()) {
+            self.base_info.tombstone = Some(event);
+        }
+        if let Some(event) = raws.topic.and_then(|e| e.deserialize_as().ok()) {
+            self.base_info.topic = Some(event);
+        }
+        for (user, event) in raws.rtc_member {
+            if let Ok(event) = event.deserialize_as() {
+                self.base_info.rtc_member.insert(user, event);
+            }
+        }
+    }
+
     /// Mark this Room as joined.
     pub fn mark_as_joined(&mut self) {
         self.room_state = RoomState::Joined;
@@ -1127,21 +1165,24 @@ impl RoomInfo {
     fn guest_access(&self) -> &GuestAccess {
         match &self.base_info.guest_access {
             Some(MinimalStateEvent::Original(ev)) => &ev.content.guest_access,
-            _ => &GuestAccess::Forbidden,
+            Some(MinimalStateEvent::Redacted(_)) => &GuestAccess::Forbidden, /* Redaction does not keep field */
+            None => &GuestAccess::Forbidden,
         }
     }
 
     fn history_visibility(&self) -> &HistoryVisibility {
         match &self.base_info.history_visibility {
             Some(MinimalStateEvent::Original(ev)) => &ev.content.history_visibility,
-            _ => &HistoryVisibility::WorldReadable,
+            Some(MinimalStateEvent::Redacted(ev)) => &ev.content.history_visibility,
+            None => &HistoryVisibility::WorldReadable,
         }
     }
 
     fn join_rule(&self) -> &JoinRule {
         match &self.base_info.join_rules {
             Some(MinimalStateEvent::Original(ev)) => &ev.content.join_rule,
-            _ => &JoinRule::Public,
+            Some(MinimalStateEvent::Redacted(ev)) => &ev.content.join_rule,
+            None => &JoinRule::Public,
         }
     }
 
@@ -1313,13 +1354,15 @@ mod tests {
     use matrix_sdk_test::{async_test, ALICE, BOB, CAROL};
     use ruma::{
         api::client::sync::sync_events::v3::RoomSummary as RumaSummary,
+        event_id,
         events::{
             call::member::{
                 Application, CallApplicationContent, CallMemberEventContent, Focus, LivekitFocus,
                 Membership, MembershipInit, OriginalSyncCallMemberEvent,
             },
             room::{
                 canonical_alias::RoomCanonicalAliasEventContent,
+                join_rules::{RoomJoinRulesEvent, RoomJoinRulesEventContent},
                 member::{
                     MembershipState, RoomMemberEventContent, StrippedRoomMemberEvent,
                     SyncRoomMemberEvent,
@@ -1332,7 +1375,7 @@ mod tests {
         serde::Raw,
         user_id, MilliSecondsSinceUnixEpoch, OwnedEventId, OwnedUserId, UserId,
     };
-    use serde_json::json;
+    use serde_json::{json, value::to_raw_value};
     use stream_assert::{assert_pending, assert_ready};
 
     #[cfg(feature = "experimental-sliding-sync")]
@@ -1342,6 +1385,7 @@ mod tests {
     use crate::latest_event::LatestEvent;
     use crate::{
         store::{MemoryStore, StateChanges, StateStore},
+        sync::UnreadNotificationsCount,
         BaseClient, DisplayName, MinimalStateEvent, OriginalMinimalStateEvent, SessionMeta,
     };
 
@@ -1778,6 +1822,65 @@ mod tests {
         );
     }
 
+    #[async_test]
+    async fn test_save_bad_joinrules() {
+        let (store, room) = make_room(RoomState::Invited);
+        let room_id = room_id!("!test:localhost");
+        let matthew = user_id!("@matthew:example.org");
+        let me = user_id!("@me:example.org");
+        let mut changes = StateChanges::new("".to_owned());
+        let summary = assign!(RumaSummary::new(), {
+            heroes: vec![me.to_string(), matthew.to_string()],
+        });
+
+        let raw_join_rules_content = Raw::<MinimalStateEvent<RoomJoinRulesEventContent>>::from_json(
+            to_raw_value(&json!({ "join_rule": "test!" })).unwrap(),
+        );
+        let raw_join_rules = Raw::from_json(
+            to_raw_value(&json!({
+                "event_id": "$test",
+                "type": "m.room.join_rules",
+                "content": raw_join_rules_content,
+                "origin_server_ts": 0,
+                "sender": matthew,
+                "state_key": "",
+            }))
+            .unwrap(),
+        );
+        let join_rules_content =
+            raw_join_rules_content.deserialize_as::<RoomJoinRulesEventContent>().unwrap();
+        assert_eq!(
+            to_raw_value(&join_rules_content).unwrap_err().to_string(),
+            "the enum variant JoinRule::_Custom cannot be serialized"
+        );
+
+        let mut room_info = RoomInfo::new(room_id, RoomState::Joined);
+        room_info.base_info.join_rules =
+            Some(MinimalStateEvent::Original(raw_join_rules.deserialize().unwrap()));
+        changes.add_room(room_info);
+
+        changes.add_state_event(
+            room_id,
+            raw_join_rules.deserialize_as().unwrap(),
+            raw_join_rules.clone().cast(),
+        );
+        store.save_changes(&changes).await.unwrap();
+
+        let read_room_info = &store.get_room_infos().await.unwrap()[0];
+        assert_eq!(
+            read_room_info
+                .base_info
+                .join_rules
+                .as_ref()
+                .unwrap()
+                .as_original()
+                .unwrap()
+                .content
+                .join_rule,
+            raw_join_rules.deserialize().unwrap().content.join_rule
+        );
+    }
+
     #[async_test]
     async fn test_display_name_dm_invited_no_heroes() {
         let (store, room) = make_room(RoomState::Invited);