From 0999e420ae1df00dc9c0580dc0dea937b8cd8d9c Mon Sep 17 00:00:00 2001 From: Jonas Platte Date: Mon, 13 Sep 2021 18:51:37 +0200 Subject: [PATCH] state-res: Return borrowed content in Event method --- .../ruma-state-res/benches/state_res_bench.rs | 6 ++-- crates/ruma-state-res/src/event_auth.rs | 36 ++++++++++++------- crates/ruma-state-res/src/lib.rs | 10 +++--- crates/ruma-state-res/src/state_event.rs | 10 ++++-- crates/ruma-state-res/src/test_utils.rs | 8 ++--- 5 files changed, 43 insertions(+), 27 deletions(-) diff --git a/crates/ruma-state-res/benches/state_res_bench.rs b/crates/ruma-state-res/benches/state_res_bench.rs index 3cf27ca0..87ea416e 100644 --- a/crates/ruma-state-res/benches/state_res_bench.rs +++ b/crates/ruma-state-res/benches/state_res_bench.rs @@ -573,10 +573,10 @@ mod event { } } - fn content(&self) -> serde_json::Value { + fn content(&self) -> &serde_json::Value { match &self.rest { - Pdu::RoomV1Pdu(ev) => ev.content.clone(), - Pdu::RoomV3Pdu(ev) => ev.content.clone(), + Pdu::RoomV1Pdu(ev) => &ev.content, + Pdu::RoomV3Pdu(ev) => &ev.content, #[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 54ef0840..5aa914ca 100644 --- a/crates/ruma-state-res/src/event_auth.rs +++ b/crates/ruma-state-res/src/event_auth.rs @@ -22,7 +22,7 @@ pub fn auth_types_for_event( kind: &EventType, sender: &UserId, state_key: Option<&str>, - content: serde_json::Value, + content: &serde_json::Value, ) -> Vec<(EventType, String)> { if kind == &EventType::RoomCreate { return vec![]; @@ -269,7 +269,9 @@ where } let sender_power_level = if let Some(pl) = &power_levels_event { - if let Ok(content) = serde_json::from_value::(pl.content()) { + if let Ok(content) = + serde_json::from_value::(pl.content().to_owned()) + { if let Some(level) = content.users.get(sender) { *level } else { @@ -281,7 +283,9 @@ where } else { // If no power level event found the creator gets 100 everyone else gets 0 room_create_event - .and_then(|create| serde_json::from_value::(create.content()).ok()) + .and_then(|create| { + serde_json::from_value::(create.content().to_owned()).ok() + }) .and_then(|create| (create.creator == *sender).then(|| int!(100))) .unwrap_or_default() }; @@ -291,7 +295,10 @@ where 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 + serde_json::from_value::( + power_levels.content().to_owned(), + )? + .invite } None => int!(50), }; @@ -376,7 +383,7 @@ fn valid_membership_change( target_user_membership_event: Option<&E>, sender: &UserId, sender_membership_event: Option<&E>, - content: serde_json::Value, + content: &serde_json::Value, prev_event: Option<&E>, current_third_party_invite: Option<&E>, power_levels_event: Option<&E>, @@ -412,7 +419,7 @@ fn valid_membership_change( }; let power_levels: PowerLevelsEventContent = match &power_levels_event { - Some(ev) => serde_json::from_value(ev.content())?, + Some(ev) => serde_json::from_value(ev.content().to_owned())?, None => PowerLevelsEventContent::default(), }; @@ -427,7 +434,8 @@ fn valid_membership_change( let mut join_rules = JoinRule::Invite; if let Some(jr) = &join_rules_event { - join_rules = serde_json::from_value::(jr.content())?.join_rule; + join_rules = + serde_json::from_value::(jr.content().to_owned())?.join_rule; } if let Some(prev) = prev_event { @@ -613,10 +621,12 @@ where // If users key in content is not a dictionary with keys that are valid user IDs // with values that are integers (or a string that is an integer), reject. let user_content = - serde_json::from_value::(power_event.content()).unwrap(); + serde_json::from_value::(power_event.content().to_owned()) + .unwrap(); let current_content = - serde_json::from_value::(current_state.content()).unwrap(); + serde_json::from_value::(current_state.content().to_owned()) + .unwrap(); // Validation of users is done in Ruma, synapse for loops validating user_ids and integers here info!("validation of power event finished"); @@ -768,7 +778,7 @@ fn get_send_level( ) -> Int { power_lvl .and_then(|ple| { - serde_json::from_value::(ple.content()) + serde_json::from_value::(ple.content().to_owned()) .map(|content| { content.events.get(e_type).copied().unwrap_or_else(|| { if state_key.is_some() { @@ -810,9 +820,9 @@ fn verify_third_party_invite( // If any signature in signed matches any public key in the m.room.third_party_invite event, // allow - if let Ok(tpid_ev) = - serde_json::from_value::(current_tpid.content()) - { + if let Ok(tpid_ev) = serde_json::from_value::( + current_tpid.content().to_owned(), + ) { // A list of public keys in the public_keys field for key in tpid_ev.public_keys.unwrap_or_default() { if key.public_key == tp_id.signed.token { diff --git a/crates/ruma-state-res/src/lib.rs b/crates/ruma-state-res/src/lib.rs index 98c34526..081777e3 100644 --- a/crates/ruma-state-res/src/lib.rs +++ b/crates/ruma-state-res/src/lib.rs @@ -343,9 +343,9 @@ where return 0; } - if let Some(content) = - pl.and_then(|pl| serde_json::from_value::(pl.content()).ok()) - { + if let Some(content) = pl.and_then(|pl| { + serde_json::from_value::(pl.content().to_owned()).ok() + }) { if let Some(ev) = event { if let Some(user) = content.users.get(ev.sender()) { debug!("found {} at power_level {}", ev.sender(), user); @@ -615,7 +615,9 @@ fn is_power_event(event: &E) -> bool { event.state_key() == Some("") } EventType::RoomMember => { - if let Ok(content) = serde_json::from_value::(event.content()) { + if let Ok(content) = + serde_json::from_value::(event.content().to_owned()) + { if [MembershipState::Leave, MembershipState::Ban].contains(&content.membership) { 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 6f49ae0e..3689c3f4 100644 --- a/crates/ruma-state-res/src/state_event.rs +++ b/crates/ruma-state-res/src/state_event.rs @@ -24,7 +24,11 @@ pub trait Event { fn event_type(&self) -> &EventType; /// The event's content. - fn content(&self) -> serde_json::Value; + // FIXME: This forces a serde_json::Value to be stored, which the previous solution of returning + // an owned one did not. However, the previous signature was even less efficient and also + // heavily encouraged storing `serde_json::Value`. We should likely force usage of `RawValue` + // instead, or somehow allow different storage without pessimizing all but one. + fn content(&self) -> &serde_json::Value; /// The state key for this event. fn state_key(&self) -> Option<&str>; @@ -77,7 +81,7 @@ impl Event for &T { (*self).event_type() } - fn content(&self) -> serde_json::Value { + fn content(&self) -> &serde_json::Value { (*self).content() } @@ -135,7 +139,7 @@ impl Event for Arc { (&**self).event_type() } - fn content(&self) -> serde_json::Value { + fn content(&self) -> &serde_json::Value { (&**self).content() } diff --git a/crates/ruma-state-res/src/test_utils.rs b/crates/ruma-state-res/src/test_utils.rs index 21532923..de470e4f 100644 --- a/crates/ruma-state-res/src/test_utils.rs +++ b/crates/ruma-state-res/src/test_utils.rs @@ -149,7 +149,7 @@ pub fn do_check( e.sender().clone(), e.event_type().clone(), e.state_key(), - e.content(), + e.content().to_owned(), &auth_events, &prev_events.iter().cloned().collect::>(), ); @@ -575,10 +575,10 @@ pub mod event { } } - fn content(&self) -> serde_json::Value { + fn content(&self) -> &serde_json::Value { match &self.rest { - Pdu::RoomV1Pdu(ev) => ev.content.clone(), - Pdu::RoomV3Pdu(ev) => ev.content.clone(), + Pdu::RoomV1Pdu(ev) => &ev.content, + Pdu::RoomV3Pdu(ev) => &ev.content, #[cfg(not(feature = "unstable-exhaustive-types"))] _ => unreachable!("new PDU version"), }