From 51b30d9ba3426ef1e4070406fa9153d99bbea5d0 Mon Sep 17 00:00:00 2001 From: Charles Hall Date: Tue, 5 Nov 2024 11:34:24 -0800 Subject: [PATCH] largely stop using `RoomCreateEventContent` This became a problem because #foundation-office:matrix.org has a malformed create event with its `predecessor` set to a string instead of a map. The solution to this is, unfortunately, to do more shotgun parsing to extract only the desired fields rather than trying to parse the entire content every time. To prevent this kind of problem from happening again, `RoomCreateEventContent` must only be used for creating new PDUs, existing PDUs must be shotgun-parsed. --- src/api/client_server/directory.rs | 20 +++++------------- src/service/rooms/spaces.rs | 24 ++++------------------ src/service/rooms/state.rs | 33 ++++++++++++++++++++++++++++-- src/service/rooms/state_cache.rs | 24 +++++++++------------- 4 files changed, 50 insertions(+), 51 deletions(-) diff --git a/src/api/client_server/directory.rs b/src/api/client_server/directory.rs index 3d4e1e06..ed95e034 100644 --- a/src/api/client_server/directory.rs +++ b/src/api/client_server/directory.rs @@ -15,7 +15,6 @@ use ruma::{ room::{ avatar::RoomAvatarEventContent, canonical_alias::RoomCanonicalAliasEventContent, - create::RoomCreateEventContent, guest_access::{GuestAccess, RoomGuestAccessEventContent}, history_visibility::{ HistoryVisibility, RoomHistoryVisibilityEventContent, @@ -29,7 +28,9 @@ use ruma::{ }; use tracing::{error, info, warn}; -use crate::{services, Ar, Error, Ra, Result}; +use crate::{ + service::rooms::state::ExtractType, services, Ar, Error, Ra, Result, +}; /// # `POST /_matrix/client/r0/publicRooms` /// @@ -382,19 +383,8 @@ fn room_id_to_chunk(room_id: ruma::OwnedRoomId) -> Result { Error::bad_database("Missing room join rule event for room.") })?; - let room_type = services() - .rooms - .state_accessor - .room_state_get(&room_id, &StateEventType::RoomCreate, "")? - .map(|s| { - serde_json::from_str::(s.content.get()) - .map_err(|error| { - error!(%error, "Invalid room create event in database"); - Error::BadDatabase("Invalid room create event in database.") - }) - }) - .transpose()? - .and_then(|e| e.room_type); + let room_type = + services().rooms.state.get_create_content::(&room_id)?; Ok(PublicRoomsChunk { canonical_alias, diff --git a/src/service/rooms/spaces.rs b/src/service/rooms/spaces.rs index 2011d5b5..88b0c4a9 100644 --- a/src/service/rooms/spaces.rs +++ b/src/service/rooms/spaces.rs @@ -13,7 +13,6 @@ use ruma::{ room::{ avatar::RoomAvatarEventContent, canonical_alias::RoomCanonicalAliasEventContent, - create::RoomCreateEventContent, guest_access::{GuestAccess, RoomGuestAccessEventContent}, history_visibility::{ HistoryVisibility, RoomHistoryVisibilityEventContent, @@ -32,6 +31,7 @@ use ruma::{ use tokio::sync::Mutex; use tracing::{debug, error, warn}; +use super::state::ExtractType; use crate::{services, Error, PduEvent, Result}; pub(crate) enum CachedJoinRule { @@ -496,27 +496,11 @@ impl Service { Self::translate_joinrule(&join_rule)? }, + room_type: services() .rooms - .state_accessor - .room_state_get(room_id, &StateEventType::RoomCreate, "")? - .map(|s| { - serde_json::from_str::( - s.content.get(), - ) - .map_err(|error| { - error!( - %error, - event_id = %s.event_id, - "Invalid room create event", - ); - Error::BadDatabase( - "Invalid room create event in database.", - ) - }) - }) - .transpose()? - .and_then(|e| e.room_type), + .state + .get_create_content::(room_id)?, children_state: children .into_iter() .map(|pdu| pdu.to_stripped_spacechild_state_event()) diff --git a/src/service/rooms/state.rs b/src/service/rooms/state.rs index 72741417..5db98287 100644 --- a/src/service/rooms/state.rs +++ b/src/service/rooms/state.rs @@ -7,9 +7,10 @@ use std::{ use ruma::{ api::client::error::ErrorKind, events::{ - room::member::MembershipState, AnyStrippedStateEvent, StateEventType, - TimelineEventType, + room::{create::PreviousRoom, member::MembershipState}, + AnyStrippedStateEvent, StateEventType, TimelineEventType, }, + room::RoomType, serde::Raw, state_res::{self, StateMap}, EventId, OwnedEventId, OwnedRoomId, RoomId, RoomVersionId, UserId, @@ -51,6 +52,34 @@ impl ExtractCreateContent for ExtractVersion { } } +/// Extract the `type` from an `m.room.create` event +#[derive(Deserialize)] +pub(crate) struct ExtractType { + #[serde(rename = "type")] + kind: Option, +} + +impl ExtractCreateContent for ExtractType { + type Extract = Option; + + fn extract(self) -> Self::Extract { + self.kind + } +} + +#[derive(Deserialize)] +pub(crate) struct ExtractPredecessor { + predecessor: Option, +} + +impl ExtractCreateContent for ExtractPredecessor { + type Extract = Option; + + fn extract(self) -> Self::Extract { + self.predecessor + } +} + pub(crate) struct Service { pub(crate) db: &'static dyn Data, } diff --git a/src/service/rooms/state_cache.rs b/src/service/rooms/state_cache.rs index 9cf264a9..e3783d74 100644 --- a/src/service/rooms/state_cache.rs +++ b/src/service/rooms/state_cache.rs @@ -5,10 +5,9 @@ use std::{ use ruma::{ events::{ - ignored_user_list::IgnoredUserListEvent, - room::{create::RoomCreateEventContent, member::MembershipState}, + ignored_user_list::IgnoredUserListEvent, room::member::MembershipState, AnyStrippedStateEvent, AnySyncStateEvent, GlobalAccountDataEventType, - RoomAccountDataEventType, StateEventType, + RoomAccountDataEventType, }, serde::Raw, OwnedRoomId, OwnedServerName, OwnedUserId, RoomId, ServerName, UserId, @@ -25,6 +24,8 @@ mod data; pub(crate) use data::Data; +use super::state::ExtractPredecessor; + pub(crate) struct Service { db: &'static dyn Data, appservice_in_room_cache: @@ -72,18 +73,13 @@ impl Service { // Check if the room has a predecessor if let Some(predecessor) = services() .rooms - .state_accessor - .room_state_get( - room_id, - &StateEventType::RoomCreate, - "", - )? - .and_then(|create| { - serde_json::from_str(create.content.get()).ok() - }) - .and_then(|content: RoomCreateEventContent| { - content.predecessor + .state + .get_create_content::(room_id) + .inspect_err(|error| { + warn!(%error, "Failed to get room predecessor"); }) + .ok() + .flatten() { self.copy_upgraded_account_data( user_id,