From 34a10b87c523caf3046cd473d027d2c0780d7a98 Mon Sep 17 00:00:00 2001 From: Jonas Platte Date: Sun, 5 Sep 2021 18:10:13 +0200 Subject: [PATCH] state-res: Return borrows from Event::{event_type, state_key} --- .../ruma-state-res/benches/state_res_bench.rs | 51 ++++++++++++++----- crates/ruma-state-res/src/event_auth.rs | 38 +++++++------- crates/ruma-state-res/src/lib.rs | 35 +++++++------ crates/ruma-state-res/src/state_event.rs | 4 +- crates/ruma-state-res/src/test_utils.rs | 32 ++++++------ crates/ruma-state-res/tests/event_sorting.rs | 2 +- .../ruma-state-res/tests/res_with_auth_ids.rs | 4 +- 7 files changed, 97 insertions(+), 69 deletions(-) diff --git a/crates/ruma-state-res/benches/state_res_bench.rs b/crates/ruma-state-res/benches/state_res_bench.rs index f279a100..52a9154e 100644 --- a/crates/ruma-state-res/benches/state_res_bench.rs +++ b/crates/ruma-state-res/benches/state_res_bench.rs @@ -100,7 +100,12 @@ fn resolve_deeper_event_set(c: &mut Criterion) { inner.get(&event_id("PA")).unwrap(), ] .iter() - .map(|ev| ((ev.event_type(), ev.state_key().unwrap()), ev.event_id().clone())) + .map(|ev| { + ( + (ev.event_type().to_owned(), ev.state_key().unwrap().to_owned()), + ev.event_id().clone(), + ) + }) .collect::>(); let state_set_b = [ @@ -113,7 +118,12 @@ fn resolve_deeper_event_set(c: &mut Criterion) { inner.get(&event_id("PA")).unwrap(), ] .iter() - .map(|ev| ((ev.event_type(), ev.state_key().unwrap()), ev.event_id().clone())) + .map(|ev| { + ( + (ev.event_type().to_owned(), ev.state_key().unwrap().to_owned()), + ev.event_id().clone(), + ) + }) .collect::>(); b.iter(|| { @@ -287,17 +297,32 @@ impl TestStore { let state_at_bob = [&create_event, &alice_mem, &join_rules, &bob_mem] .iter() - .map(|e| ((e.event_type(), e.state_key().unwrap()), e.event_id().clone())) + .map(|e| { + ( + (e.event_type().to_owned(), e.state_key().unwrap().to_owned()), + e.event_id().clone(), + ) + }) .collect::>(); let state_at_charlie = [&create_event, &alice_mem, &join_rules, &charlie_mem] .iter() - .map(|e| ((e.event_type(), e.state_key().unwrap()), e.event_id().clone())) + .map(|e| { + ( + (e.event_type().to_owned(), e.state_key().unwrap().to_owned()), + e.event_id().clone(), + ) + }) .collect::>(); let expected = [&create_event, &alice_mem, &join_rules, &bob_mem, &charlie_mem] .iter() - .map(|e| ((e.event_type(), e.state_key().unwrap()), e.event_id().clone())) + .map(|e| { + ( + (e.event_type().to_owned(), e.state_key().unwrap().to_owned()), + e.event_id().clone(), + ) + }) .collect::>(); (state_at_bob, state_at_charlie, expected) @@ -537,7 +562,7 @@ pub mod event { self.sender() } - fn event_type(&self) -> EventType { + fn event_type(&self) -> &EventType { self.event_type() } @@ -549,7 +574,7 @@ pub mod event { *self.origin_server_ts() } - fn state_key(&self) -> Option { + fn state_key(&self) -> Option<&str> { self.state_key() } @@ -680,18 +705,18 @@ pub mod event { _ => unreachable!("new PDU version"), } } - pub fn event_type(&self) -> EventType { + pub fn event_type(&self) -> &EventType { match &self.rest { - Pdu::RoomV1Pdu(ev) => ev.kind.clone(), - Pdu::RoomV3Pdu(ev) => ev.kind.clone(), + Pdu::RoomV1Pdu(ev) => &ev.kind, + Pdu::RoomV3Pdu(ev) => &ev.kind, #[cfg(not(feature = "unstable-exhaustive-types"))] _ => unreachable!("new PDU version"), } } - pub fn state_key(&self) -> Option { + pub fn state_key(&self) -> Option<&str> { match &self.rest { - Pdu::RoomV1Pdu(ev) => ev.state_key.clone(), - Pdu::RoomV3Pdu(ev) => ev.state_key.clone(), + Pdu::RoomV1Pdu(ev) => ev.state_key.as_deref(), + Pdu::RoomV3Pdu(ev) => ev.state_key.as_deref(), #[cfg(not(feature = "unstable-exhaustive-types"))] _ => unreachable!("new PDU version"), } diff --git a/crates/ruma-state-res/src/event_auth.rs b/crates/ruma-state-res/src/event_auth.rs index eb56150c..e6382b8b 100644 --- a/crates/ruma-state-res/src/event_auth.rs +++ b/crates/ruma-state-res/src/event_auth.rs @@ -21,7 +21,7 @@ use crate::{room_version::RoomVersion, Error, Event, Result, StateMap}; pub fn auth_types_for_event( kind: &EventType, sender: &UserId, - state_key: Option, + state_key: Option<&str>, content: serde_json::Value, ) -> Vec<(EventType, String)> { if kind == &EventType::RoomCreate { @@ -47,9 +47,9 @@ pub fn auth_types_for_event( } } - let key = (EventType::RoomMember, state_key); + let key = (EventType::RoomMember, state_key.to_owned()); if !auth_types.contains(&key) { - auth_types.push(key) + auth_types.push(key); } if membership == MembershipState::Invite { @@ -114,7 +114,7 @@ where // Implementation of https://matrix.org/docs/spec/rooms/v1#authorization-rules // // 1. If type is m.room.create: - if incoming_event.event_type() == EventType::RoomCreate { + if *incoming_event.event_type() == EventType::RoomCreate { info!("start m.room.create check"); // If it has any previous events, reject @@ -188,13 +188,13 @@ where // [synapse] checks for federation here // 4. If type is m.room.aliases - if incoming_event.event_type() == EventType::RoomAliases + if *incoming_event.event_type() == EventType::RoomAliases && room_version.special_case_aliases_auth { info!("starting m.room.aliases check"); // If sender's domain doesn't matches state_key, reject - if incoming_event.state_key().as_deref() != Some(sender.server_name().as_str()) { + if incoming_event.state_key() != Some(sender.server_name().as_str()) { warn!("state_key does not match sender"); return Ok(false); } @@ -206,7 +206,7 @@ where let power_levels_event = fetch_state(&EventType::RoomPowerLevels, ""); let sender_member_event = fetch_state(&EventType::RoomMember, sender.as_str()); - if incoming_event.event_type() == EventType::RoomMember { + if *incoming_event.event_type() == EventType::RoomMember { info!("starting m.room.member check"); let state_key = match incoming_event.state_key() { None => { @@ -288,7 +288,7 @@ where // Allow if and only if sender's current power level is greater than // or equal to the invite level - if incoming_event.event_type() == EventType::RoomThirdPartyInvite { + if *incoming_event.event_type() == EventType::RoomThirdPartyInvite { let invite_level = match &power_levels_event { Some(power_levels) => { serde_json::from_value::(power_levels.content())?.invite @@ -309,7 +309,7 @@ where return Ok(false); } - if incoming_event.event_type() == EventType::RoomPowerLevels { + if *incoming_event.event_type() == EventType::RoomPowerLevels { info!("starting m.room.power_levels check"); if let Some(required_pwr_lvl) = check_power_levels( @@ -337,7 +337,7 @@ where // power levels. if room_version.extra_redaction_checks - && incoming_event.event_type() == EventType::RoomRedaction + && *incoming_event.event_type() == EventType::RoomRedaction { let default = int!(50); let redact_level = if let Some(pl) = power_levels_event { @@ -431,7 +431,7 @@ fn valid_membership_change( } if let Some(prev) = prev_event { - if prev.event_type() == EventType::RoomCreate && prev.prev_events().is_empty() { + if *prev.event_type() == EventType::RoomCreate && prev.prev_events().is_empty() { return Ok(true); } } @@ -565,7 +565,7 @@ fn valid_membership_change( /// /// Does the event have the correct userId as its state_key if it's not the "" state_key. fn can_send_event(event: &Arc, ple: Option<&Arc>, user_level: Int) -> bool { - let event_type_power_level = get_send_level(&event.event_type(), event.state_key(), ple); + let event_type_power_level = get_send_level(event.event_type(), event.state_key(), ple); debug!("{} ev_type {} usr {}", event.event_id(), event_type_power_level, user_level); @@ -573,8 +573,8 @@ fn can_send_event(event: &Arc, ple: Option<&Arc>, user_level: In return false; } - if event.state_key().as_ref().map_or(false, |k| k.starts_with('@')) - && event.state_key().as_deref() != Some(event.sender().as_str()) + if event.state_key().map_or(false, |k| k.starts_with('@')) + && event.state_key() != Some(event.sender().as_str()) { return false; // permission required to post in this room } @@ -592,7 +592,7 @@ fn check_power_levels( where E: Event, { - match power_event.state_key().as_deref() { + match power_event.state_key() { Some("") => {} Some(key) => { error!("m.room.power_levels event has non-empty state key: {}", key); @@ -796,7 +796,7 @@ pub fn can_federate(auth_events: &StateMap>) -> bool { /// `e_type` based on the rooms "m.room.power_level" event. pub fn get_send_level( e_type: &EventType, - state_key: Option, + state_key: Option<&str>, power_lvl: Option<&Arc>, ) -> Int { power_lvl @@ -833,7 +833,7 @@ pub fn verify_third_party_invite( // If there is no m.room.third_party_invite event in the current room state // with state_key matching token, reject if let Some(current_tpid) = current_third_party_invite { - if current_tpid.state_key().as_ref() != Some(&tp_id.signed.token) { + if current_tpid.state_key() != Some(&tp_id.signed.token) { return false; } @@ -884,7 +884,7 @@ mod tests { let auth_events = events .values() - .map(|ev| ((ev.event_type(), ev.state_key()), Arc::clone(ev))) + .map(|ev| ((ev.event_type().to_owned(), ev.state_key().to_owned()), Arc::clone(ev))) .collect::>(); let requester = to_pdu_event( @@ -926,7 +926,7 @@ mod tests { let auth_events = events .values() - .map(|ev| ((ev.event_type(), ev.state_key()), Arc::clone(ev))) + .map(|ev| ((ev.event_type().to_owned(), ev.state_key().to_owned()), Arc::clone(ev))) .collect::>(); let requester = to_pdu_event( diff --git a/crates/ruma-state-res/src/lib.rs b/crates/ruma-state-res/src/lib.rs index fc28084b..b4e28655 100644 --- a/crates/ruma-state-res/src/lib.rs +++ b/crates/ruma-state-res/src/lib.rs @@ -343,7 +343,7 @@ where for aid in event.as_ref().map(|pdu| pdu.auth_events()).unwrap_or_default() { if let Some(aev) = fetch_event(&aid) { - if is_type_and_key(&aev, EventType::RoomPowerLevels, "") { + if is_type_and_key(&aev, &EventType::RoomPowerLevels, "") { pl = Some(aev); break; } @@ -408,10 +408,12 @@ where // related to soft-failing auth_events.insert( ( - ev.event_type(), - ev.state_key().ok_or_else(|| { - Error::InvalidPdu("State event had no state key".to_owned()) - })?, + ev.event_type().to_owned(), + ev.state_key() + .ok_or_else(|| { + Error::InvalidPdu("State event had no state key".to_owned()) + })? + .to_owned(), ), ev, ); @@ -421,15 +423,15 @@ where } for key in auth_types_for_event( - &event.event_type(), + event.event_type(), event.sender(), - Some(state_key.clone()), + Some(state_key), event.content(), ) { if let Some(ev_id) = resolved_state.get(&key) { if let Some(event) = fetch_event(ev_id) { // TODO synapse checks `rejected_reason` is None here - auth_events.insert(key.clone(), event); + auth_events.insert(key.to_owned(), event); } } } @@ -442,7 +444,7 @@ where // The key for this is (eventType + a state_key of the signed token not sender) so // search for it let current_third_party = auth_events.iter().find_map(|(_, pdu)| { - (pdu.event_type() == EventType::RoomThirdPartyInvite).then(|| { + (*pdu.event_type() == EventType::RoomThirdPartyInvite).then(|| { // TODO no clone, auth_events is borrowed while moved pdu.clone() }) @@ -456,7 +458,8 @@ where |ty, key| auth_events.get(&(ty.clone(), key.to_owned())).cloned(), )? { // add event to resolved state map - resolved_state.insert((event.event_type(), state_key), event_id.clone()); + resolved_state + .insert((event.event_type().to_owned(), state_key.to_owned()), event_id.clone()); } else { // synapse passes here on AuthError. We do not add this event to resolved_state. warn!("event {} failed the authentication check", event_id); @@ -504,7 +507,7 @@ where for aid in auth_events { let ev = fetch_event(aid) .ok_or_else(|| Error::NotFound(format!("Failed to find {}", aid)))?; - if is_type_and_key(&ev, EventType::RoomPowerLevels, "") { + if is_type_and_key(&ev, &EventType::RoomPowerLevels, "") { pl = Some(aid.clone()); break; } @@ -568,7 +571,7 @@ where for aid in auth_events { let aev = fetch_event(aid) .ok_or_else(|| Error::NotFound(format!("Failed to find {}", aid)))?; - if is_type_and_key(&aev, EventType::RoomPowerLevels, "") { + if is_type_and_key(&aev, &EventType::RoomPowerLevels, "") { event = Some(aev); break; } @@ -615,19 +618,19 @@ where } } -pub fn is_type_and_key(ev: &Arc, ev_type: EventType, state_key: &str) -> bool { - ev.event_type() == ev_type && ev.state_key().as_deref() == Some(state_key) +pub fn is_type_and_key(ev: &Arc, ev_type: &EventType, state_key: &str) -> bool { + ev.event_type() == ev_type && ev.state_key() == Some(state_key) } pub fn is_power_event(event: &Arc) -> bool { match event.event_type() { EventType::RoomPowerLevels | EventType::RoomJoinRules | EventType::RoomCreate => { - event.state_key() == Some("".into()) + event.state_key() == Some("") } EventType::RoomMember => { if let Ok(content) = serde_json::from_value::(event.content()) { if [MembershipState::Leave, MembershipState::Ban].contains(&content.membership) { - return Some(event.sender().as_str()) != event.state_key().as_deref(); + return Some(event.sender().as_str()) != event.state_key(); } } diff --git a/crates/ruma-state-res/src/state_event.rs b/crates/ruma-state-res/src/state_event.rs index 40c3b9ec..4b632dc7 100644 --- a/crates/ruma-state-res/src/state_event.rs +++ b/crates/ruma-state-res/src/state_event.rs @@ -21,13 +21,13 @@ pub trait Event { fn origin_server_ts(&self) -> MilliSecondsSinceUnixEpoch; /// The kind of event. - fn event_type(&self) -> EventType; + fn event_type(&self) -> &EventType; /// The `UserId` of this PDU. fn content(&self) -> serde_json::Value; /// The state key for this event. - fn state_key(&self) -> Option; + fn state_key(&self) -> Option<&str>; /// The events before this event. fn prev_events(&self) -> Vec; diff --git a/crates/ruma-state-res/src/test_utils.rs b/crates/ruma-state-res/src/test_utils.rs index fc50bdf3..06d86b4d 100644 --- a/crates/ruma-state-res/src/test_utils.rs +++ b/crates/ruma-state-res/src/test_utils.rs @@ -129,12 +129,12 @@ pub fn do_check( let mut state_after = state_before.clone(); - let ty = fake_event.event_type(); - let key = fake_event.state_key(); + let ty = fake_event.event_type().to_owned(); + let key = fake_event.state_key().to_owned(); state_after.insert((ty, key), event_id.clone()); let auth_types = auth_types_for_event( - &fake_event.event_type(), + fake_event.event_type(), fake_event.sender(), Some(fake_event.state_key()), fake_event.content(), @@ -155,7 +155,7 @@ pub fn do_check( e.event_id().as_str(), e.sender().clone(), e.event_type().clone(), - Some(&e.state_key()), + Some(e.state_key()), e.content(), &auth_events, &prev_events.iter().cloned().collect::>(), @@ -179,7 +179,7 @@ pub fn do_check( ) }); - let key = (ev.event_type(), ev.state_key()); + let key = (ev.event_type().to_owned(), ev.state_key().to_owned()); expected_state.insert(key, node); } @@ -339,17 +339,17 @@ impl TestStore { let state_at_bob = [&create_event, &alice_mem, &join_rules, &bob_mem] .iter() - .map(|e| ((e.event_type(), e.state_key()), e.event_id().clone())) + .map(|e| ((e.event_type().to_owned(), e.state_key().to_owned()), e.event_id().clone())) .collect::>(); let state_at_charlie = [&create_event, &alice_mem, &join_rules, &charlie_mem] .iter() - .map(|e| ((e.event_type(), e.state_key()), e.event_id().clone())) + .map(|e| ((e.event_type().to_owned(), e.state_key().to_owned()), e.event_id().clone())) .collect::>(); let expected = [&create_event, &alice_mem, &join_rules, &bob_mem, &charlie_mem] .iter() - .map(|e| ((e.event_type(), e.state_key()), e.event_id().clone())) + .map(|e| ((e.event_type().to_owned(), e.state_key().to_owned()), e.event_id().clone())) .collect::>(); (state_at_bob, state_at_charlie, expected) @@ -585,7 +585,7 @@ pub mod event { fn sender(&self) -> &UserId { self.sender() } - fn event_type(&self) -> EventType { + fn event_type(&self) -> &EventType { self.event_type() } @@ -596,7 +596,7 @@ pub mod event { *self.origin_server_ts() } - fn state_key(&self) -> Option { + fn state_key(&self) -> Option<&str> { Some(self.state_key()) } fn prev_events(&self) -> Vec { @@ -721,18 +721,18 @@ pub mod event { _ => unreachable!("new PDU version"), } } - pub fn event_type(&self) -> EventType { + pub fn event_type(&self) -> &EventType { match &self.rest { - Pdu::RoomV1Pdu(ev) => ev.kind.clone(), - Pdu::RoomV3Pdu(ev) => ev.kind.clone(), + Pdu::RoomV1Pdu(ev) => &ev.kind, + Pdu::RoomV3Pdu(ev) => &ev.kind, #[cfg(not(feature = "unstable-exhaustive-types"))] _ => unreachable!("new PDU version"), } } - pub fn state_key(&self) -> String { + pub fn state_key(&self) -> &str { match &self.rest { - Pdu::RoomV1Pdu(ev) => ev.state_key.clone().unwrap(), - Pdu::RoomV3Pdu(ev) => ev.state_key.clone().unwrap(), + Pdu::RoomV1Pdu(ev) => ev.state_key.as_ref().unwrap(), + Pdu::RoomV3Pdu(ev) => ev.state_key.as_ref().unwrap(), #[cfg(not(feature = "unstable-exhaustive-types"))] _ => unreachable!("new PDU version"), } diff --git a/crates/ruma-state-res/tests/event_sorting.rs b/crates/ruma-state-res/tests/event_sorting.rs index 0df67e96..997ce3e0 100644 --- a/crates/ruma-state-res/tests/event_sorting.rs +++ b/crates/ruma-state-res/tests/event_sorting.rs @@ -16,7 +16,7 @@ fn test_event_sort() { let event_map = events .values() - .map(|ev| ((ev.event_type(), ev.state_key()), ev.clone())) + .map(|ev| ((ev.event_type().to_owned(), ev.state_key().to_owned()), ev.clone())) .collect::>(); let auth_chain = HashSet::new(); diff --git a/crates/ruma-state-res/tests/res_with_auth_ids.rs b/crates/ruma-state-res/tests/res_with_auth_ids.rs index dab48f1f..ab96fbf5 100644 --- a/crates/ruma-state-res/tests/res_with_auth_ids.rs +++ b/crates/ruma-state-res/tests/res_with_auth_ids.rs @@ -50,7 +50,7 @@ fn ban_with_auth_chains2() { inner.get(&event_id("PA")).unwrap(), ] .iter() - .map(|ev| ((ev.event_type(), ev.state_key()), ev.event_id().clone())) + .map(|ev| ((ev.event_type().to_owned(), ev.state_key().to_owned()), ev.event_id().clone())) .collect::>(); let state_set_b = [ @@ -63,7 +63,7 @@ fn ban_with_auth_chains2() { inner.get(&event_id("PA")).unwrap(), ] .iter() - .map(|ev| ((ev.event_type(), ev.state_key()), ev.event_id().clone())) + .map(|ev| ((ev.event_type().to_owned(), ev.state_key().to_owned()), ev.event_id().clone())) .collect::>(); let ev_map: EventMap> = store.0.clone();