use event content in account_data service api instead of full events

This eliminates the possibility of passing an event that has a
mismatching type, reducing the space of possible invalid states.
This commit is contained in:
Olivia Lee 2025-03-23 14:45:33 -07:00
parent 66210bc32d
commit b82458a460
No known key found for this signature in database
GPG key ID: 54D568A15B9CD1F9
15 changed files with 277 additions and 240 deletions

View file

@ -2,14 +2,16 @@ use std::collections::HashMap;
use ruma::{
events::{
AnyGlobalAccountDataEvent, AnyRoomAccountDataEvent,
AnyGlobalAccountDataEvent, AnyGlobalAccountDataEventContent,
AnyRoomAccountDataEvent, AnyRoomAccountDataEventContent,
GlobalAccountDataEventType, RoomAccountDataEventType,
},
serde::Raw,
RoomId, UserId,
};
use serde::{Deserialize, Serialize};
use crate::Result;
use crate::{Error, Result};
mod data;
@ -19,6 +21,78 @@ pub(crate) struct Service {
pub(crate) db: &'static dyn Data,
}
pub(crate) fn raw_global_event_to_parts(
event: &Raw<AnyGlobalAccountDataEvent>,
) -> serde_json::Result<(
GlobalAccountDataEventType,
Raw<AnyGlobalAccountDataEventContent>,
)> {
#[derive(Deserialize)]
struct Parts {
#[serde(rename = "type")]
event_type: GlobalAccountDataEventType,
content: Raw<AnyGlobalAccountDataEventContent>,
}
let parts = event.deserialize_as::<Parts>()?;
Ok((parts.event_type, parts.content))
}
pub(crate) fn raw_global_event_from_parts(
event_type: &GlobalAccountDataEventType,
content: &Raw<AnyGlobalAccountDataEventContent>,
) -> Raw<AnyGlobalAccountDataEvent> {
#[derive(Serialize)]
struct Parts<'a> {
#[serde(rename = "type")]
event_type: &'a GlobalAccountDataEventType,
content: &'a Raw<AnyGlobalAccountDataEventContent>,
}
Raw::new(&Parts {
event_type,
content,
})
.expect("json serialization should always succeed")
.cast::<AnyGlobalAccountDataEvent>()
}
pub(crate) fn raw_room_event_to_parts(
event: &Raw<AnyRoomAccountDataEvent>,
) -> serde_json::Result<(
RoomAccountDataEventType,
Raw<AnyRoomAccountDataEventContent>,
)> {
#[derive(Deserialize)]
struct Parts {
#[serde(rename = "type")]
event_type: RoomAccountDataEventType,
content: Raw<AnyRoomAccountDataEventContent>,
}
let parts = event.deserialize_as::<Parts>()?;
Ok((parts.event_type, parts.content))
}
pub(crate) fn raw_room_event_from_parts(
event_type: &RoomAccountDataEventType,
content: &Raw<AnyRoomAccountDataEventContent>,
) -> Raw<AnyRoomAccountDataEvent> {
#[derive(Serialize)]
struct Parts<'a> {
#[serde(rename = "type")]
event_type: &'a RoomAccountDataEventType,
content: &'a Raw<AnyRoomAccountDataEventContent>,
}
Raw::new(&Parts {
event_type,
content,
})
.expect("json serialization should always succeed")
.cast::<AnyRoomAccountDataEvent>()
}
impl Service {
pub(crate) fn new(db: &'static dyn Data) -> Self {
Self {
@ -28,26 +102,28 @@ impl Service {
/// Places one event in the global account data of the user and removes the
/// previous entry.
#[tracing::instrument(skip(self, user_id, event))]
#[tracing::instrument(skip(self, user_id, content))]
pub(crate) fn update_global(
&self,
user_id: &UserId,
event_type: &GlobalAccountDataEventType,
event: &Raw<AnyGlobalAccountDataEvent>,
content: &Raw<AnyGlobalAccountDataEventContent>,
) -> Result<()> {
let event = raw_global_event_from_parts(event_type, content);
self.db.update(None, user_id, &event_type.to_string(), event.json())
}
/// Places one event in the room account data of the user and removes the
/// previous entry for that room.
#[tracing::instrument(skip(self, room_id, user_id, event))]
#[tracing::instrument(skip(self, room_id, user_id, content))]
pub(crate) fn update_room(
&self,
room_id: &RoomId,
user_id: &UserId,
event_type: &RoomAccountDataEventType,
event: &Raw<AnyRoomAccountDataEvent>,
content: &Raw<AnyRoomAccountDataEventContent>,
) -> Result<()> {
let event = raw_room_event_from_parts(event_type, content);
self.db.update(
Some(room_id),
user_id,
@ -62,11 +138,17 @@ impl Service {
&self,
user_id: &UserId,
event_type: &GlobalAccountDataEventType,
) -> Result<Option<Raw<AnyGlobalAccountDataEvent>>> {
Ok(self
.db
.get(None, user_id, &event_type.to_string())?
.map(Raw::from_json))
) -> Result<Option<Raw<AnyGlobalAccountDataEventContent>>> {
let Some(event) =
self.db.get(None, user_id, &event_type.to_string())?
else {
return Ok(None);
};
let event = Raw::<AnyGlobalAccountDataEvent>::from_json(event);
let (_, content) = raw_global_event_to_parts(&event).map_err(|_| {
Error::bad_database("Invalid account data event in db.")
})?;
Ok(Some(content))
}
/// Searches the room account data for a specific kind.
@ -76,11 +158,17 @@ impl Service {
room_id: &RoomId,
user_id: &UserId,
event_type: &RoomAccountDataEventType,
) -> Result<Option<Raw<AnyRoomAccountDataEvent>>> {
Ok(self
.db
.get(Some(room_id), user_id, &event_type.to_string())?
.map(Raw::from_json))
) -> Result<Option<Raw<AnyRoomAccountDataEventContent>>> {
let Some(event) =
self.db.get(Some(room_id), user_id, &event_type.to_string())?
else {
return Ok(None);
};
let event = Raw::<AnyRoomAccountDataEvent>::from_json(event);
let (_, content) = raw_room_event_to_parts(&event).map_err(|_| {
Error::bad_database("Invalid account data event in db.")
})?;
Ok(Some(content))
}
/// Returns all changes to global account data that happened after `since`.
@ -93,16 +181,21 @@ impl Service {
user_id: &UserId,
since: u64,
) -> Result<
HashMap<GlobalAccountDataEventType, Raw<AnyGlobalAccountDataEvent>>,
HashMap<
GlobalAccountDataEventType,
Raw<AnyGlobalAccountDataEventContent>,
>,
> {
Ok(self
.db
self.db
.changes_since(None, user_id, since)?
.into_iter()
.map(|(event_type, event)| {
(event_type.into(), Raw::from_json(event))
.into_values()
.map(|event| {
let event = Raw::<AnyGlobalAccountDataEvent>::from_json(event);
raw_global_event_to_parts(&event).map_err(|_| {
Error::bad_database("Invalid account data event in db")
})
})
.collect())
.collect()
}
/// Returns all changes to room account data that happened after `since`.
@ -115,15 +208,18 @@ impl Service {
user_id: &UserId,
room_id: &RoomId,
since: u64,
) -> Result<HashMap<RoomAccountDataEventType, Raw<AnyRoomAccountDataEvent>>>
{
Ok(self
.db
) -> Result<
HashMap<RoomAccountDataEventType, Raw<AnyRoomAccountDataEventContent>>,
> {
self.db
.changes_since(Some(room_id), user_id, since)?
.into_iter()
.map(|(event_type, event)| {
(event_type.into(), Raw::from_json(event))
.into_values()
.map(|event| {
let event = Raw::<AnyRoomAccountDataEvent>::from_json(event);
raw_room_event_to_parts(&event).map_err(|_| {
Error::bad_database("Invalid account data event in db")
})
})
.collect())
.collect()
}
}

View file

@ -9,6 +9,9 @@ use crate::Result;
/// distinguish between global and room events. Because there are no ruma types
/// that cover both, we use strings for the event types and raw json values for
/// the contents.
//
// TODO: once we have the ability to make db schema changes, we should consider
// storing only the content in the db, rather than the whole event object.
pub(crate) trait Data: Send + Sync {
/// Places one event in the account data of the user and removes the
/// previous entry.

View file

@ -5,7 +5,7 @@ use regex::Regex;
use ruma::{
api::appservice::Registration,
events::{
push_rules::{PushRulesEvent, PushRulesEventContent},
push_rules::PushRulesEventContent,
room::{
canonical_alias::RoomCanonicalAliasEventContent,
create::RoomCreateEventContent,
@ -20,7 +20,7 @@ use ruma::{
power_levels::RoomPowerLevelsEventContent,
topic::RoomTopicEventContent,
},
AnyGlobalAccountDataEvent, TimelineEventType,
TimelineEventType,
},
serde::Raw,
signatures::verify_json,
@ -774,15 +774,15 @@ impl Service {
services().account_data.update_global(
&user_id,
&ruma::events::GlobalAccountDataEventType::PushRules,
&Raw::new(&PushRulesEvent {
content: PushRulesEventContent {
&Raw::new(
&PushRulesEventContent {
global: ruma::push::Ruleset::server_default(
&user_id,
),
},
})
.expect("json serialization should always succeed")
.cast::<AnyGlobalAccountDataEvent>(),
}
.into(),
)
.expect("json serialization should always succeed"),
)?;
// we dont add a device since we're not the user, just the

View file

@ -18,8 +18,7 @@ use ruma::{
api::federation::discovery::ServerSigningKeys,
events::{
push_rules::PushRulesEventContent,
room::message::RoomMessageEventContent, AnyGlobalAccountDataEvent,
GlobalAccountDataEvent, GlobalAccountDataEventType,
room::message::RoomMessageEventContent, GlobalAccountDataEventType,
},
push::Ruleset,
serde::{Base64, Raw},
@ -495,13 +494,13 @@ impl Service {
services().account_data.update_global(
admin_bot,
&GlobalAccountDataEventType::PushRules,
&Raw::new(&GlobalAccountDataEvent {
content: PushRulesEventContent {
&Raw::new(
&PushRulesEventContent {
global: ruleset,
},
})
.expect("json serialization should always succeed")
.cast::<AnyGlobalAccountDataEvent>(),
}
.into(),
)
.expect("json serialization should always succeed"),
)?;
res

View file

@ -5,9 +5,10 @@ use std::{
use ruma::{
events::{
ignored_user_list::IgnoredUserListEvent, room::member::MembershipState,
AnyGlobalAccountDataEvent, AnyStrippedStateEvent, AnySyncStateEvent,
GlobalAccountDataEventType, RoomAccountDataEventType,
ignored_user_list::IgnoredUserListEventContent,
room::member::MembershipState, AnyGlobalAccountDataEventContent,
AnyStrippedStateEvent, AnySyncStateEvent, GlobalAccountDataEventType,
RoomAccountDataEventType,
},
serde::Raw,
OwnedRoomId, OwnedServerName, OwnedUserId, RoomId, ServerName, UserId,
@ -102,7 +103,7 @@ impl Service {
&GlobalAccountDataEventType::IgnoredUserList,
)?
.map(|event| {
event.deserialize_as::<IgnoredUserListEvent>()
event.deserialize_as::<IgnoredUserListEventContent>()
.map_err(|error| {
warn!(
%error,
@ -114,7 +115,6 @@ impl Service {
.transpose()?
.is_some_and(|ignored| {
ignored
.content
.ignored_users
.iter()
.any(|(user, _details)| user == sender)
@ -221,14 +221,14 @@ impl Service {
from_room_id: &RoomId,
to_room_id: &RoomId,
) -> Result<()> {
let Some(event) = services()
let Some(event_content) = services()
.account_data
.get_global(user_id, &GlobalAccountDataEventType::Direct)?
else {
return Ok(());
};
let mut event = event
let mut event_content = event_content
.deserialize_as::<serde_json::Value>()
.expect("RawValue -> Value should always succeed");
@ -240,14 +240,11 @@ impl Service {
//
// [1]: https://github.com/element-hq/element-web/issues/27630
//
// A valid m.direct event looks like this:
// Valid m.direct event content looks like this:
//
// {
// "type": "m.account_data",
// "content": {
// "@userid1": [ "!roomid1", "!roomid2" ],
// "@userid2": [ "!roomid3" ],
// }
// "@userid1": [ "!roomid1", "!roomid2" ],
// "@userid2": [ "!roomid3" ],
// }
//
// We want to find userid keys where the value contains from_room_id,
@ -257,10 +254,7 @@ impl Service {
// parts.
let mut event_updated = false;
let Some(direct_user_ids) = event.get_mut("content") else {
return Ok(());
};
let Some(direct_user_ids) = direct_user_ids.as_object_mut() else {
let Some(direct_user_ids) = event_content.as_object_mut() else {
return Ok(());
};
for room_ids in direct_user_ids.values_mut() {
@ -277,9 +271,9 @@ impl Service {
if let Err(error) = services().account_data.update_global(
user_id,
&GlobalAccountDataEventType::Direct,
&Raw::new(&event)
&Raw::new(&event_content)
.expect("json serialization should always succeed")
.cast::<AnyGlobalAccountDataEvent>(),
.cast::<AnyGlobalAccountDataEventContent>(),
) {
warn!(%error, "error writing m.direct account data event after upgrading room");
}

View file

@ -9,7 +9,7 @@ use ruma::{
api::{client::error::ErrorKind, federation},
canonical_json::to_canonical_value,
events::{
push_rules::PushRulesEvent,
push_rules::PushRulesEventContent,
room::{
create::RoomCreateEventContent, encrypted::Relation,
member::MembershipState, message::RoomMessageEventContent,
@ -419,15 +419,16 @@ impl Service {
.account_data
.get_global(user, &GlobalAccountDataEventType::PushRules)?
.map(|event| {
event.deserialize_as::<PushRulesEvent>().map_err(|_| {
Error::bad_database("Invalid push rules event in db.")
})
event.deserialize_as::<PushRulesEventContent>().map_err(
|_| {
Error::bad_database(
"Invalid push rules event in db.",
)
},
)
})
.transpose()?
.map_or_else(
|| Ruleset::server_default(user),
|ev: PushRulesEvent| ev.content.global,
);
.map_or_else(|| Ruleset::server_default(user), |ev| ev.global);
let mut highlight = false;
let mut notify = false;

View file

@ -22,7 +22,7 @@ use ruma::{
},
device_id,
events::{
push_rules::PushRulesEvent, receipt::ReceiptType,
push_rules::PushRulesEventContent, receipt::ReceiptType,
AnySyncEphemeralRoomEvent, GlobalAccountDataEventType,
},
push,
@ -859,10 +859,12 @@ async fn handle_push_event(
.account_data
.get_global(userid, &GlobalAccountDataEventType::PushRules)
.unwrap_or_default()
.and_then(|event| event.deserialize_as::<PushRulesEvent>().ok())
.and_then(|event| {
event.deserialize_as::<PushRulesEventContent>().ok()
})
.map_or_else(
|| push::Ruleset::server_default(userid),
|ev: PushRulesEvent| ev.content.global,
|ev| ev.global,
);
let unread: UInt = services()