diff --git a/src/api/client/context.rs b/src/api/client/context.rs index cb1ebf92e..cc7f3fbe8 100644 --- a/src/api/client/context.rs +++ b/src/api/client/context.rs @@ -104,10 +104,7 @@ pub(crate) async fn get_context_route( .timeline .get_unsigned_context(&pdu.1, Some(sender_user)) .await; - pdu.1 - .set_unsigned(ctx.user_id, ctx.membership, ctx.prev_content, ctx.redacted_because) - .log_err() - .ok(); + pdu.1.set_unsigned(Some(ctx)).log_err().ok(); if let Err(e) = services .rooms .pdu_metadata @@ -135,10 +132,7 @@ pub(crate) async fn get_context_route( .timeline .get_unsigned_context(&pdu.1, Some(sender_user)) .await; - pdu.1 - .set_unsigned(ctx.user_id, ctx.membership, ctx.prev_content, ctx.redacted_because) - .log_err() - .ok(); + pdu.1.set_unsigned(Some(ctx)).log_err().ok(); if let Err(e) = services .rooms .pdu_metadata diff --git a/src/api/client/message.rs b/src/api/client/message.rs index 1a064f438..72b1b29e5 100644 --- a/src/api/client/message.rs +++ b/src/api/client/message.rs @@ -145,10 +145,7 @@ pub(crate) async fn get_message_events_route( .timeline .get_unsigned_context(&pdu.1, Some(sender_user)) .await; - pdu.1 - .set_unsigned(ctx.user_id, ctx.membership, ctx.prev_content, ctx.redacted_because) - .log_err() - .ok(); + pdu.1.set_unsigned(Some(ctx)).log_err().ok(); if let Err(e) = services .rooms .pdu_metadata diff --git a/src/api/client/room/event.rs b/src/api/client/room/event.rs index a54d6d597..268ca92c1 100644 --- a/src/api/client/room/event.rs +++ b/src/api/client/room/event.rs @@ -63,7 +63,7 @@ pub(crate) async fn get_room_event_route( .timeline .get_unsigned_context(&event, Some(sender_user)) .await; - event.set_unsigned(ctx.user_id, ctx.membership, ctx.prev_content, ctx.redacted_because)?; + event.set_unsigned(Some(ctx))?; Ok(get_room_event::v3::Response::new(event.into_format())) } diff --git a/src/api/client/room/initial_sync.rs b/src/api/client/room/initial_sync.rs index 73a76a7be..807753389 100644 --- a/src/api/client/room/initial_sync.rs +++ b/src/api/client/room/initial_sync.rs @@ -59,10 +59,7 @@ pub(crate) async fn room_initial_sync_route( .timeline .get_unsigned_context(&pdu.1, Some(sender_user)) .await; - pdu.1 - .set_unsigned(ctx.user_id, ctx.membership, ctx.prev_content, ctx.redacted_because) - .log_err() - .ok(); + pdu.1.set_unsigned(Some(ctx)).log_err().ok(); if let Err(e) = services .rooms .pdu_metadata diff --git a/src/api/client/sync/mod.rs b/src/api/client/sync/mod.rs index 1ef3bd4b0..e3ad680ca 100644 --- a/src/api/client/sync/mod.rs +++ b/src/api/client/sync/mod.rs @@ -91,15 +91,7 @@ async fn load_timeline( .timeline .get_unsigned_context(&pdu.1, Some(sender_user)) .await; - pdu.1 - .set_unsigned( - ctx.user_id, - ctx.membership, - ctx.prev_content, - ctx.redacted_because, - ) - .log_err() - .ok(); + pdu.1.set_unsigned(Some(ctx)).log_err().ok(); Some(pdu) }) .then(async move |mut pdu| { @@ -129,15 +121,7 @@ async fn load_timeline( .timeline .get_unsigned_context(&pdu.1, Some(sender_user)) .await; - pdu.1 - .set_unsigned( - ctx.user_id, - ctx.membership, - ctx.prev_content, - ctx.redacted_because, - ) - .log_err() - .ok(); + pdu.1.set_unsigned(Some(ctx)).log_err().ok(); Some(pdu) }) .then(async move |mut pdu| { diff --git a/src/api/client/sync/v3/joined.rs b/src/api/client/sync/v3/joined.rs index f24499d29..d2e8a8909 100644 --- a/src/api/client/sync/v3/joined.rs +++ b/src/api/client/sync/v3/joined.rs @@ -328,9 +328,7 @@ async fn build_sticky_events( .timeline .get_unsigned_context(&pdu, Some(sync_context.syncing_user)) .await; - pdu.set_unsigned(ctx.user_id, ctx.membership, ctx.prev_content, ctx.redacted_because) - .log_err() - .ok(); + pdu.set_unsigned(Some(ctx)).log_err().ok(); events.push(Event::into_format(pdu)); } diff --git a/src/api/client/sync/v5.rs b/src/api/client/sync/v5.rs index 94627bd29..f75785c86 100644 --- a/src/api/client/sync/v5.rs +++ b/src/api/client/sync/v5.rs @@ -1218,9 +1218,7 @@ async fn push_sticky_event( .timeline .get_unsigned_context(&pdu, Some(sender_user)) .await; - pdu.set_unsigned(ctx.user_id, ctx.membership, ctx.prev_content, ctx.redacted_because) - .log_err() - .ok(); + pdu.set_unsigned(Some(ctx)).log_err().ok(); response .rooms .entry(room_id) diff --git a/src/core/matrix/pdu.rs b/src/core/matrix/pdu.rs index 8d8a855d7..f7c0576d2 100644 --- a/src/core/matrix/pdu.rs +++ b/src/core/matrix/pdu.rs @@ -23,6 +23,7 @@ pub use self::{ id::{ShortId, *}, partial::PartialPdu, raw_id::*, + unsigned::UnsignedContext, }; use super::{Event, StateKey}; use crate::Result; diff --git a/src/core/matrix/pdu/tests.rs b/src/core/matrix/pdu/tests.rs index 48417d8ed..91de0652c 100644 --- a/src/core/matrix/pdu/tests.rs +++ b/src/core/matrix/pdu/tests.rs @@ -74,7 +74,7 @@ fn redaction_removes_stickiness() { fn sticky_ttl_is_added_to_unsigned() { let mut pdu = sticky_pdu(&serde_json::json!({ "duration_ms": 300_000 })); pdu.origin_server_ts = ruma::MilliSecondsSinceUnixEpoch::now().get(); - pdu.set_unsigned(None, None, None, None).unwrap(); + pdu.set_unsigned(None).unwrap(); let unsigned: serde_json::Value = serde_json::from_str(pdu.unsigned.as_deref().expect("unsigned is set").get()) @@ -99,7 +99,7 @@ fn set_unsigned_adds_the_sticky_ttl() { // section let mut pdu = sticky_pdu(&serde_json::json!({ "duration_ms": 300_000 })); pdu.origin_server_ts = ruma::MilliSecondsSinceUnixEpoch::now().get(); - pdu.set_unsigned(None, None, None, None).unwrap(); + pdu.set_unsigned(None).unwrap(); let unsigned: serde_json::Value = serde_json::from_str(pdu.unsigned.as_deref().expect("unsigned is set").get()) diff --git a/src/core/matrix/pdu/unsigned.rs b/src/core/matrix/pdu/unsigned.rs index 0177b0fae..83326df49 100644 --- a/src/core/matrix/pdu/unsigned.rs +++ b/src/core/matrix/pdu/unsigned.rs @@ -1,7 +1,7 @@ use std::collections::BTreeMap; use ruma::{ - MilliSecondsSinceUnixEpoch, + MilliSecondsSinceUnixEpoch, UserId, events::{AnyTimelineEvent, room::member::MembershipState}, serde::Raw, }; @@ -12,6 +12,14 @@ use crate::Event; type Unsigned = BTreeMap>; +#[derive(Default, Clone)] +pub struct UnsignedContext<'a> { + pub user_id: Option<&'a UserId>, + pub membership: Option, + pub prev_content: Option>, + pub redacted_because: Option>, +} + impl Pdu { /// Sets the `unsigned` field of the PDU using a mix of the provided /// optional data, and information available in the PDU itself. The @@ -30,10 +38,7 @@ impl Pdu { /// well-formed. pub fn set_unsigned( &mut self, - user_id: Option<&ruma::UserId>, - membership: Option, - prev_content: Option>, - redacted_because: Option>, + context: Option>, ) -> crate::Result { // See: https://spec.matrix.org/v1.19/client-server-api/#definition-clientevent_unsigneddata let mut unsigned = self @@ -47,25 +52,25 @@ impl Pdu { to_raw_value(&now_i.saturating_sub(self.origin_server_ts.into()))?, ); - if let Some(membership) = membership { - unsigned.insert("membership".to_owned(), to_raw_value(&membership)?); - } + if let Some(context) = context { + if let Some(membership) = context.membership { + unsigned.insert("membership".to_owned(), to_raw_value(&membership)?); + } - if let Some(prev_content) = prev_content { - unsigned.insert("prev_content".to_owned(), to_raw_value(&prev_content)?); - // TODO(nex): prev_sender is still inserted in append.rs because - // only the prev *content* is passed to this function. - // I think ideally passing the actual direct `UnsignedContext` to - // this function instead of unpacking its parameters would be the - // proper solution, especially since every callsite has one - } - if let Some(redacted_because) = redacted_because { - unsigned.remove("org.continuwuity.redacted_by"); - unsigned.insert("redacted_because".to_owned(), to_raw_value(&redacted_because)?); - } + if let Some(prev_content) = context.prev_content { + unsigned.insert("prev_content".to_owned(), to_raw_value(&prev_content)?); + } + if let Some(redacted_because) = context.redacted_because { + unsigned.remove("org.continuwuity.redacted_by"); + unsigned.insert("redacted_because".to_owned(), to_raw_value(&redacted_because)?); + } - // Remove transaction_id unless the user is the sender - if user_id.is_none_or(|u| u != self.sender()) { + // replaces_state is embedded and always included. + + if context.user_id.is_none_or(|u| u != self.sender()) { + unsigned.remove("transaction_id"); + } + } else { unsigned.remove("transaction_id"); } diff --git a/src/service/rooms/pdu_metadata/data.rs b/src/service/rooms/pdu_metadata/data.rs index be529d20f..db3973f06 100644 --- a/src/service/rooms/pdu_metadata/data.rs +++ b/src/service/rooms/pdu_metadata/data.rs @@ -100,9 +100,7 @@ impl Data { .timeline .get_unsigned_context(&pdu, Some(user_id)) .await; - pdu.set_unsigned(ctx.user_id, ctx.membership, ctx.prev_content, ctx.redacted_because) - .log_err() - .ok(); + pdu.set_unsigned(Some(ctx)).log_err().ok(); Some((shorteventid, pdu)) }) diff --git a/src/service/rooms/search/mod.rs b/src/service/rooms/search/mod.rs index 317cf2648..b84d47c88 100644 --- a/src/service/rooms/search/mod.rs +++ b/src/service/rooms/search/mod.rs @@ -143,14 +143,7 @@ impl Service { .timeline .get_unsigned_context(&pdu, Some(sender_user)) .await; - pdu.set_unsigned( - ctx.user_id, - ctx.membership, - ctx.prev_content, - ctx.redacted_because, - ) - .log_err() - .ok(); + pdu.set_unsigned(Some(ctx)).log_err().ok(); Some(pdu) }) diff --git a/src/service/rooms/threads/mod.rs b/src/service/rooms/threads/mod.rs index 48622586c..a29ca5511 100644 --- a/src/service/rooms/threads/mod.rs +++ b/src/service/rooms/threads/mod.rs @@ -170,14 +170,7 @@ impl Service { .timeline .get_unsigned_context(&pdu, Some(user_id)) .await; - pdu.set_unsigned( - ctx.user_id, - ctx.membership, - ctx.prev_content, - ctx.redacted_because, - ) - .log_err() - .ok(); + pdu.set_unsigned(Some(ctx)).log_err().ok(); Some((pdu_id.shorteventid, pdu)) }); diff --git a/src/service/rooms/timeline/helpers.rs b/src/service/rooms/timeline/helpers.rs index f2926cab1..170b35889 100644 --- a/src/service/rooms/timeline/helpers.rs +++ b/src/service/rooms/timeline/helpers.rs @@ -1,8 +1,8 @@ //! Helpers for submitting events with the right checks performed -use conduwuit::{ +pub(crate) use conduwuit::{ Err, Event, Result, err, - matrix::pdu::{PartialPdu, sticky}, + matrix::pdu::{PartialPdu, UnsignedContext, sticky}, trace, }; use ruma::{ @@ -23,14 +23,6 @@ use ruma::{ use crate::rooms::state::RoomMutexGuard; -#[derive(Default, Clone)] -pub struct UnsignedContext<'a> { - pub user_id: Option<&'a UserId>, - pub membership: Option, - pub prev_content: Option>, - pub redacted_because: Option>, -} - impl super::Service { #[allow(clippy::too_many_arguments)] pub async fn send_state_event_for_key( @@ -369,11 +361,7 @@ impl super::Service { let can_see = self .services .state_accessor - .user_can_see_event( - sender_user, - &event.room_id_or_hash(), - event.event_id(), - ) + .user_can_see_event(sender_user, &event.room_id_or_hash(), &event_id) .await; if can_see { trace!(prev_event_id=%event_id, "Fetching prev content"); diff --git a/src/service/rooms/timeline/mod.rs b/src/service/rooms/timeline/mod.rs index 4279db1b2..715d52855 100644 --- a/src/service/rooms/timeline/mod.rs +++ b/src/service/rooms/timeline/mod.rs @@ -27,7 +27,7 @@ use ruma::{ use serde::Deserialize; use self::data::Data; -pub use self::{create::pdu_fits, data::PdusIterItem, helpers::UnsignedContext}; +pub use self::{create::pdu_fits, data::PdusIterItem}; use crate::{ Dep, account_data, admin, appservice, config, globals, pusher, rooms, sending, server_keys, sync, users,