feat: Move PDU validation out of parsing and into check 1

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!(

src/service/rooms/event_handler/fetch_auth.rs

@@ -56,7 +56,11 @@ pub(super) async fn fetch_and_persist_event_auth<Pdu>(
 						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;

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"

src/service/rooms/event_handler/parse_incoming_pdu.rs

@@ -23,7 +23,12 @@ fn extract_room_id(event_type: &str, pdu: &CanonicalJsonObject) -> Result<OwnedR
 			.map_err(|e| err!(Request(BadJson("Invalid room_id {room_id:?} in pdu: {e}"))));
 	}
 	// If there's no room ID, and this is not a create event, it is illegal.
-	if event_type != "m.room.create" || pdu.get("state_key").is_none() {
+	let is_create = event_type == "m.room.create"
+		&& pdu
+			.get("state_key")
+			.and_then(|v| v.as_str())
+			.is_some_and(str::is_empty);
+	if !is_create {
 		return Err!(Request(BadJson("Missing room_id in pdu")));
 	}
 
@@ -42,10 +47,7 @@ fn extract_room_id(event_type: &str, pdu: &CanonicalJsonObject) -> Result<OwnedR
 		return Err!(Request(BadJson("Unknown room version in pdu")));
 	};
 
-	if !room_version_rules
-		.authorization
-		.room_create_event_id_as_room_id
-	{
+	if room_version_rules.event_format.require_room_create_room_id {
 		return Err!(Request(BadJson("Missing room_id in pdu")));
 	}
 
@@ -80,33 +82,10 @@ pub(super) fn expect_event_id_array(
 }
 
 impl super::Service {
-	/// Performs some basic validation on the PDU to make sure it's not
-	/// obviously malformed. This is not a full validation, but guards against
-	/// extreme errors.
-	///
-	/// Currently, this just validates that prev/auth events are within
-	/// acceptable ranges. Other servers do some additional things like
-	/// checking depth range, but serde will do that later when converting the
-	/// object to a PduEvent.
-	pub fn validate_pdu(&self, pdu: &CanonicalJsonObject) -> 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<Parsed> {
 		let value = serde_json::from_str::<CanonicalJsonObject>(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<Parsed> {
 			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))
 	}
 }

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