From c34d1fe76e65900da31ae22417bd646c0cb64652 Mon Sep 17 00:00:00 2001 From: timedout Date: Sat, 11 Jul 2026 05:00:39 +0100 Subject: [PATCH] feat: Move PDU validation out of parsing and into check 1 --- .../fetch_and_handle_outliers.rs | 2 +- src/service/rooms/event_handler/fetch_auth.rs | 6 ++- .../rooms/event_handler/handle_outlier_pdu.rs | 8 +++- .../rooms/event_handler/parse_incoming_pdu.rs | 48 ++++++------------- src/service/rooms/event_handler/pdu_checks.rs | 39 +++++++++++++-- 5 files changed, 62 insertions(+), 41 deletions(-) diff --git a/src/service/rooms/event_handler/fetch_and_handle_outliers.rs b/src/service/rooms/event_handler/fetch_and_handle_outliers.rs index 527cdb61c..67c5a4a16 100644 --- a/src/service/rooms/event_handler/fetch_and_handle_outliers.rs +++ b/src/service/rooms/event_handler/fetch_and_handle_outliers.rs @@ -385,7 +385,7 @@ async fn fetch_event_via( .await?; let (calculated_event_id, value) = - self.parse_incoming_pdu_with_known_room(&res.pdu, room_version_rules)?; + Self::parse_incoming_pdu_with_known_room(&res.pdu, room_version_rules)?; if calculated_event_id != event_id { Err!(Request(BadJson(warn!( diff --git a/src/service/rooms/event_handler/fetch_auth.rs b/src/service/rooms/event_handler/fetch_auth.rs index 48ff38a9c..742fc958a 100644 --- a/src/service/rooms/event_handler/fetch_auth.rs +++ b/src/service/rooms/event_handler/fetch_auth.rs @@ -56,7 +56,11 @@ pub(super) async fn fetch_and_persist_event_auth( continue; }, }; - if let Err(e) = Self::pdu_format_check_1(&auth_pdu_json) { + if let Err(e) = Self::pdu_format_check_1( + &auth_pdu_json, + room_version_rules, + &create_event.event_id().to_owned(), + ) { // drop this PDU warn!(%auth_event_id, error=?e, "Dropping auth chain event as it violates the room event format"); continue; diff --git a/src/service/rooms/event_handler/handle_outlier_pdu.rs b/src/service/rooms/event_handler/handle_outlier_pdu.rs index 665c1ab7d..b778c7bbf 100644 --- a/src/service/rooms/event_handler/handle_outlier_pdu.rs +++ b/src/service/rooms/event_handler/handle_outlier_pdu.rs @@ -44,7 +44,13 @@ pub(super) async fn handle_outlier_pdu<'a, Pdu>( // 1. Check that the PDU follows the format for the room version // (in this case, just size check) - Self::pdu_format_check_1(&value).inspect_err(|e| { + let room_version_rules = get_room_version_rules(create_event)?; + Self::pdu_format_check_1( + &value, + &room_version_rules, + &create_event.event_id().to_owned(), + ) + .inspect_err(|e| { info!( err=?e, "Dropping incoming PDU from {origin} because it violates the room event format" diff --git a/src/service/rooms/event_handler/parse_incoming_pdu.rs b/src/service/rooms/event_handler/parse_incoming_pdu.rs index e30fdebb0..66122bf96 100644 --- a/src/service/rooms/event_handler/parse_incoming_pdu.rs +++ b/src/service/rooms/event_handler/parse_incoming_pdu.rs @@ -23,7 +23,12 @@ fn extract_room_id(event_type: &str, pdu: &CanonicalJsonObject) -> Result Result Result { - // Since v3: - // `event_id` should not be present on the PDU. - // NOTE: The above is ignored since technically it's still allowed to be - // included, but should be ignored instead. - // `auth_events` and `prev_events` must be an array of event IDs - let auth_events = expect_event_id_array(pdu, "auth_events")?; - if auth_events.len() > 10 { - return Err!(Request(BadJson("PDU has too many auth events"))); - } - let prev_events = expect_event_id_array(pdu, "prev_events")?; - if prev_events.len() > 20 { - return Err!(Request(BadJson("PDU has too many prev events"))); - } - Ok(()) - } - + /// Parses an incoming PDU JSON object, generating an event ID for it. Does + /// not insert the event ID into the returned object. Does not discover the + /// room ID. pub(super) fn parse_incoming_pdu_with_known_room( - &self, pdu: &RawJsonValue, room_version_rules: &RoomVersionRules, ) -> Result<(OwnedEventId, CanonicalJsonObject)> { @@ -114,10 +93,13 @@ pub(super) fn parse_incoming_pdu_with_known_room( gen_event_id_canonical_json(pdu, room_version_rules).map_err(|e| { err!(Request(InvalidParam("Could not convert event to canonical json: {e}"))) })?; - self.validate_pdu(&value)?; + // NOTE: validation checks are now performed by `pdu_format_check_1`. Ok((event_id, value)) } + /// Parses an incoming PDU JSON object, generating an event ID for it and + /// attempts to discover the associated room ID. Does not insert the event + /// ID into the returned object. pub async fn parse_incoming_pdu(&self, pdu: &RawJsonValue) -> Result { let value = serde_json::from_str::(pdu.get()).map_err(|e| { err!(BadServerResponse(debug_warn!("Error parsing incoming event {e:?}"))) @@ -142,7 +124,7 @@ pub async fn parse_incoming_pdu(&self, pdu: &RawJsonValue) -> Result { gen_event_id_canonical_json(pdu, &room_version_rules).map_err(|e| { err!(Request(InvalidParam("Could not convert event to canonical json: {e}"))) })?; - self.validate_pdu(&value)?; + // NOTE: validation checks are now performed by `pdu_format_check_1`. Ok((room_id, event_id, value)) } } diff --git a/src/service/rooms/event_handler/pdu_checks.rs b/src/service/rooms/event_handler/pdu_checks.rs index 26005c17e..3862103a4 100644 --- a/src/service/rooms/event_handler/pdu_checks.rs +++ b/src/service/rooms/event_handler/pdu_checks.rs @@ -10,20 +10,49 @@ events::StateEventType, room_version_rules::RoomVersionRules, }; -use crate::rooms::timeline::pdu_fits; +use crate::rooms::{ + event_handler::parse_incoming_pdu::expect_event_id_array, timeline::pdu_fits, +}; impl super::Service { /// Checks that the PDU conforms to the PDU format (check 1). This is /// already mostly done during deserialisation, so this function just checks /// that the PDU isn't a too large. - pub(super) fn pdu_format_check_1(pdu_json: &CanonicalJsonObject) -> Result<()> { + pub(super) fn pdu_format_check_1( + pdu_json: &CanonicalJsonObject, + room_version_rules: &RoomVersionRules, + create_event_id: &OwnedEventId, + ) -> Result<()> { + let event_format = &room_version_rules.event_format; // NOTE: if we do any more validation outside of deserialisation, it has to be // done here. + if !pdu_fits(pdu_json) { - Err!(Request(TooLarge("PDU is too large"))) - } else { - Ok(()) + return Err!(Request(TooLarge("PDU is too large"))); } + + if event_format.require_room_create_room_id { + if pdu_json.get("room_id").is_none() { + return Err!(Request(BadJson("Missing required PDU field: `room_id`"))); + } + } + + let auth_events = expect_event_id_array(pdu_json, "auth_events")?; + if auth_events.len() > 10 { + return Err!(Request(BadJson("PDU has too many auth events"))); + } + + let create_event_in_auth_events = auth_events.contains(create_event_id); + if !event_format.allow_room_create_in_auth_events && create_event_in_auth_events { + return Err!(Request(BadJson("PDU references a create event"))); + } + + let prev_events = expect_event_id_array(pdu_json, "prev_events")?; + if prev_events.len() > 20 { + return Err!(Request(BadJson("PDU has too many prev events"))); + } + + Ok(()) } /// Checks that the PDU has a valid signature (check 2), and redacts it if