From 95473f4c242dc409d8d4da9be0c01171d42250ac Mon Sep 17 00:00:00 2001 From: timedout Date: Sat, 30 May 2026 16:55:32 +0100 Subject: [PATCH] fix: Make PDU handle errors noisier & correct error types --- .../rooms/event_handler/fetch_state.rs | 9 +++-- .../event_handler/handle_incoming_pdu.rs | 13 +++---- .../rooms/event_handler/handle_outlier_pdu.rs | 35 +++++++++++-------- .../event_handler/upgrade_outlier_pdu.rs | 2 +- 4 files changed, 36 insertions(+), 23 deletions(-) diff --git a/src/service/rooms/event_handler/fetch_state.rs b/src/service/rooms/event_handler/fetch_state.rs index c23a562a0..a4feaaa66 100644 --- a/src/service/rooms/event_handler/fetch_state.rs +++ b/src/service/rooms/event_handler/fetch_state.rs @@ -165,8 +165,13 @@ pub(super) async fn fetch_state( .get_shortstatekey(&StateEventType::RoomCreate, "") .await?; - if state.get(&create_shortstatekey).map(AsRef::as_ref) != Some(create_event.event_id()) { - return Err!(Request(Forbidden("Incoming event refers to wrong create event."))); + let create_event_id = state.get(&create_shortstatekey).map(AsRef::as_ref); + if create_event_id != Some(create_event.event_id()) { + return Err!(Request(Forbidden( + "Incoming event refers to wrong create event: expected {}, got: \ + {create_event_id:?}", + create_event.event_id() + ))); } Ok(Some(state)) diff --git a/src/service/rooms/event_handler/handle_incoming_pdu.rs b/src/service/rooms/event_handler/handle_incoming_pdu.rs index 736288e15..faf7708a9 100644 --- a/src/service/rooms/event_handler/handle_incoming_pdu.rs +++ b/src/service/rooms/event_handler/handle_incoming_pdu.rs @@ -1,7 +1,7 @@ use std::{collections::BTreeMap, time::Instant}; use conduwuit::{ - Err, Event, PduEvent, Result, debug_error, debug_info, defer, err, info, trace, warn, + Err, Event, PduEvent, Result, debug_error, debug_info, defer, err, error, info, trace, warn, }; use futures::{ FutureExt, @@ -147,9 +147,7 @@ pub async fn handle_incoming_pdu<'a>( .and_then(|v| v.as_str()) .ok_or_else(|| err!("No sender in object")) .and_then(|v| Ok(UserId::parse(v)?)) - .map_err(|e| { - err!(Request(InvalidParam("PDU does not have a valid sender key: {e}"))) - })?; + .map_err(|e| err!(Request(BadJson("PDU does not have a valid sender key: {e}"))))?; let sender_acl_check: OptionFuture<_> = sender .server_name() @@ -230,7 +228,8 @@ pub async fn handle_incoming_pdu<'a>( let (incoming_pdu, val) = self .handle_outlier_pdu(origin, create_event, event_id, room_id, value, false) - .await?; + .await + .inspect(|e| error!("[TODO] Failed to handle outlier PDU: {e:?}"))?; // 8. if not timeline event: stop if !is_timeline_event { @@ -249,10 +248,12 @@ pub async fn handle_incoming_pdu<'a>( // These are timeline events self.fetch_prevs(room_id, create_event, &incoming_pdu, origin) - .await?; + .await + .inspect_err(|e| error!("[TODO] Failed to fetch_prevs: {e:?}"))?; // Done with prev events, now handling the incoming event self.upgrade_outlier_to_timeline_pdu(incoming_pdu, val, create_event, origin, room_id) .await + .inspect_err(|e| error!("[TODO] Failed to upgrade outlier to timeline pdu: {e:?}")) } } diff --git a/src/service/rooms/event_handler/handle_outlier_pdu.rs b/src/service/rooms/event_handler/handle_outlier_pdu.rs index 332de86c8..4e37755de 100644 --- a/src/service/rooms/event_handler/handle_outlier_pdu.rs +++ b/src/service/rooms/event_handler/handle_outlier_pdu.rs @@ -1,8 +1,8 @@ use std::collections::{BTreeMap, HashMap, hash_map}; use conduwuit::{ - Err, Event, PduEvent, Result, debug, debug_info, debug_warn, err, info, state_res, trace, - warn, + Err, Event, PduEvent, Result, debug, debug_info, debug_warn, err, info, state_res, + state_res::EventTypeExt, trace, warn, }; use futures::future::ready; use ruma::{ @@ -110,7 +110,7 @@ pub(super) async fn handle_outlier_pdu<'a, Pdu>( event_id, ); self.services.pdu_metadata.mark_event_rejected(event_id); - return Err!(Request(InvalidParam("Event has rejected auth event: {aid}"))); + return Err!(Request(Forbidden("Event has rejected auth event: {aid}"))); } if let Ok(auth_event) = self.services.timeline.get_pdu(aid).await { @@ -148,7 +148,7 @@ pub(super) async fn handle_outlier_pdu<'a, Pdu>( let (auth_event_room_id, auth_event_id, auth_pdu_json) = self.parse_incoming_pdu(&auth_pdu_json).await?; if auth_event_room_id != room_id { - return Err!(Request(BadJson( + return Err!(Request(Forbidden( "Auth event {auth_event_id} is in {auth_event_room_id}, not {room_id}." ))); } @@ -201,7 +201,7 @@ pub(super) async fn handle_outlier_pdu<'a, Pdu>( .outlier .add_pdu_outlier(pdu_event.event_id(), &incoming_pdu); self.services.pdu_metadata.mark_event_rejected(event_id); - return Err!(Request(InvalidParam( + return Err!(Request(Forbidden( "Auth event's type and state_key combination exists multiple times: {}, \ {}", auth_event.kind, @@ -212,15 +212,22 @@ pub(super) async fn handle_outlier_pdu<'a, Pdu>( } // The original create event must be in the auth events - if !matches!( - auth_events_by_key.get(&(StateEventType::RoomCreate, String::new().into())), - Some(_) | None - ) { - self.services.pdu_metadata.mark_event_rejected(event_id); - self.services - .outlier - .add_pdu_outlier(pdu_event.event_id(), &incoming_pdu); - return Err!(Request(InvalidParam("Incoming event refers to wrong create event."))); + let claimed_create_event = + auth_events_by_key.get(&StateEventType::RoomCreate.with_state_key("")); + if let Some(claimed_create_event) = claimed_create_event { + if claimed_create_event.event_id() != create_event.event_id() { + self.services.pdu_metadata.mark_event_rejected(event_id); + self.services + .outlier + .add_pdu_outlier(pdu_event.event_id(), &incoming_pdu); + return Err!(Request(Forbidden( + "Incoming event refers to wrong create event (expected {}, got {})", + create_event.event_id(), + claimed_create_event.event_id(), + ))); + } + } else if !pdu_event.auth_events.is_empty() { + return Err!(Request(Forbidden("Incoming event does not refer to any create event"))); } let state_fetch = |ty: &StateEventType, sk: &str| { diff --git a/src/service/rooms/event_handler/upgrade_outlier_pdu.rs b/src/service/rooms/event_handler/upgrade_outlier_pdu.rs index 1f5d8f17e..7d8236382 100644 --- a/src/service/rooms/event_handler/upgrade_outlier_pdu.rs +++ b/src/service/rooms/event_handler/upgrade_outlier_pdu.rs @@ -92,7 +92,7 @@ pub(super) async fn upgrade_outlier_to_timeline_pdu( .fetch_state(origin, create_event, room_id, incoming_pdu.event_id()) .await .debug_inspect_err(|e| { - debug_error!("Could not fetch state from {origin}: {e}") + debug_error!("Could not fetch state from {origin}: {e}"); })?; }