diff --git a/crates/ruma-common/CHANGELOG.md b/crates/ruma-common/CHANGELOG.md index fd537940..aaa8c30d 100644 --- a/crates/ruma-common/CHANGELOG.md +++ b/crates/ruma-common/CHANGELOG.md @@ -6,6 +6,8 @@ Bug fixes: during creation of the rich reply * Don't include sensitive information in `Debug`-format of types from the `events::key` and `events::secret` modules +* Fix deserialization of `RoomMessageEventContent` and `RoomEncryptedEventContent` when there + is no relation Breaking changes: diff --git a/crates/ruma-common/src/events/audio.rs b/crates/ruma-common/src/events/audio.rs index 26b1bb33..bbb79f42 100644 --- a/crates/ruma-common/src/events/audio.rs +++ b/crates/ruma-common/src/events/audio.rs @@ -39,7 +39,11 @@ pub struct AudioEventContent { pub audio: AudioContent, /// Information about related messages. - #[serde(flatten, skip_serializing_if = "Option::is_none")] + #[serde( + flatten, + skip_serializing_if = "Option::is_none", + deserialize_with = "crate::events::room::message::relation_serde::deserialize_relation" + )] pub relates_to: Option>, } diff --git a/crates/ruma-common/src/events/emote.rs b/crates/ruma-common/src/events/emote.rs index 5f87f66f..b992da45 100644 --- a/crates/ruma-common/src/events/emote.rs +++ b/crates/ruma-common/src/events/emote.rs @@ -23,7 +23,11 @@ pub struct EmoteEventContent { pub message: MessageContent, /// Information about related messages. - #[serde(flatten, skip_serializing_if = "Option::is_none")] + #[serde( + flatten, + skip_serializing_if = "Option::is_none", + deserialize_with = "crate::events::room::message::relation_serde::deserialize_relation" + )] pub relates_to: Option>, } diff --git a/crates/ruma-common/src/events/file.rs b/crates/ruma-common/src/events/file.rs index 0a95c56f..c48ff6fe 100644 --- a/crates/ruma-common/src/events/file.rs +++ b/crates/ruma-common/src/events/file.rs @@ -34,7 +34,11 @@ pub struct FileEventContent { pub file: FileContent, /// Information about related messages. - #[serde(flatten, skip_serializing_if = "Option::is_none")] + #[serde( + flatten, + skip_serializing_if = "Option::is_none", + deserialize_with = "crate::events::room::message::relation_serde::deserialize_relation" + )] pub relates_to: Option>, } diff --git a/crates/ruma-common/src/events/image.rs b/crates/ruma-common/src/events/image.rs index 03124f16..e19081dd 100644 --- a/crates/ruma-common/src/events/image.rs +++ b/crates/ruma-common/src/events/image.rs @@ -50,7 +50,11 @@ pub struct ImageEventContent { pub caption: Option, /// Information about related messages. - #[serde(flatten, skip_serializing_if = "Option::is_none")] + #[serde( + flatten, + skip_serializing_if = "Option::is_none", + deserialize_with = "crate::events::room::message::relation_serde::deserialize_relation" + )] pub relates_to: Option>, } diff --git a/crates/ruma-common/src/events/location.rs b/crates/ruma-common/src/events/location.rs index c7e180dd..0e7b5463 100644 --- a/crates/ruma-common/src/events/location.rs +++ b/crates/ruma-common/src/events/location.rs @@ -39,7 +39,11 @@ pub struct LocationEventContent { pub ts: Option, /// Information about related messages. - #[serde(flatten, skip_serializing_if = "Option::is_none")] + #[serde( + flatten, + skip_serializing_if = "Option::is_none", + deserialize_with = "crate::events::room::message::relation_serde::deserialize_relation" + )] pub relates_to: Option>, } diff --git a/crates/ruma-common/src/events/message.rs b/crates/ruma-common/src/events/message.rs index 9351d3ad..25486632 100644 --- a/crates/ruma-common/src/events/message.rs +++ b/crates/ruma-common/src/events/message.rs @@ -72,7 +72,11 @@ pub struct MessageEventContent { pub message: MessageContent, /// Information about related messages. - #[serde(flatten, skip_serializing_if = "Option::is_none")] + #[serde( + flatten, + skip_serializing_if = "Option::is_none", + deserialize_with = "crate::events::room::message::relation_serde::deserialize_relation" + )] pub relates_to: Option>, } diff --git a/crates/ruma-common/src/events/notice.rs b/crates/ruma-common/src/events/notice.rs index f593ef9b..2a0449f3 100644 --- a/crates/ruma-common/src/events/notice.rs +++ b/crates/ruma-common/src/events/notice.rs @@ -23,7 +23,11 @@ pub struct NoticeEventContent { pub message: MessageContent, /// Information about related messages. - #[serde(flatten, skip_serializing_if = "Option::is_none")] + #[serde( + flatten, + skip_serializing_if = "Option::is_none", + deserialize_with = "crate::events::room::message::relation_serde::deserialize_relation" + )] pub relates_to: Option>, } diff --git a/crates/ruma-common/src/events/room/encrypted.rs b/crates/ruma-common/src/events/room/encrypted.rs index e2790b2b..c8d22f9e 100644 --- a/crates/ruma-common/src/events/room/encrypted.rs +++ b/crates/ruma-common/src/events/room/encrypted.rs @@ -23,7 +23,11 @@ pub struct RoomEncryptedEventContent { pub scheme: EncryptedEventScheme, /// Information about related events. - #[serde(flatten, skip_serializing_if = "Option::is_none")] + #[serde( + flatten, + skip_serializing_if = "Option::is_none", + deserialize_with = "relation_serde::deserialize_relation" + )] pub relates_to: Option, } @@ -400,6 +404,8 @@ mod tests { assert_eq!(c.ciphertext.len(), 1); assert_eq!(c.ciphertext["test_curve_key"].body, "encrypted_body"); assert_eq!(c.ciphertext["test_curve_key"].message_type, uint!(1)); + + assert_matches!(content.relates_to, None); } #[test] diff --git a/crates/ruma-common/src/events/room/encrypted/relation_serde.rs b/crates/ruma-common/src/events/room/encrypted/relation_serde.rs index 84f77ab0..7cdd63d7 100644 --- a/crates/ruma-common/src/events/room/encrypted/relation_serde.rs +++ b/crates/ruma-common/src/events/room/encrypted/relation_serde.rs @@ -5,47 +5,43 @@ use super::Annotation; use super::{InReplyTo, Reference, Relation, Replacement, Thread}; use crate::OwnedEventId; -impl<'de> Deserialize<'de> for Relation { - fn deserialize(deserializer: D) -> Result - where - D: Deserializer<'de>, +pub(super) fn deserialize_relation<'de, D>(deserializer: D) -> Result, D::Error> +where + D: Deserializer<'de>, +{ + let ev = EventWithRelatesToJsonRepr::deserialize(deserializer)?; + + if let Some( + RelationJsonRepr::ThreadStable(ThreadStableJsonRepr { event_id, is_falling_back }) + | RelationJsonRepr::ThreadUnstable(ThreadUnstableJsonRepr { event_id, is_falling_back }), + ) = ev.relates_to.relation { - let ev = EventWithRelatesToJsonRepr::deserialize(deserializer)?; - - if let Some( - RelationJsonRepr::ThreadStable(ThreadStableJsonRepr { event_id, is_falling_back }) - | RelationJsonRepr::ThreadUnstable(ThreadUnstableJsonRepr { event_id, is_falling_back }), - ) = ev.relates_to.relation - { - let in_reply_to = ev - .relates_to - .in_reply_to - .ok_or_else(|| serde::de::Error::missing_field("m.in_reply_to"))?; - return Ok(Relation::Thread(Thread { event_id, in_reply_to, is_falling_back })); - } - let rel = if let Some(in_reply_to) = ev.relates_to.in_reply_to { - Relation::Reply { in_reply_to } - } else if let Some(relation) = ev.relates_to.relation { - match relation { - #[cfg(feature = "unstable-msc2677")] - RelationJsonRepr::Annotation(a) => Relation::Annotation(a), - RelationJsonRepr::Reference(r) => Relation::Reference(r), - RelationJsonRepr::Replacement(Replacement { event_id }) => { - Relation::Replacement(Replacement { event_id }) - } - RelationJsonRepr::ThreadStable(_) | RelationJsonRepr::ThreadUnstable(_) => { - unreachable!() - } - // FIXME: Maybe we should log this, though at this point we don't even have - // access to the rel_type of the unknown relation. - RelationJsonRepr::Unknown => Relation::_Custom, - } - } else { - Relation::_Custom - }; - - Ok(rel) + let in_reply_to = ev + .relates_to + .in_reply_to + .ok_or_else(|| serde::de::Error::missing_field("m.in_reply_to"))?; + return Ok(Some(Relation::Thread(Thread { event_id, in_reply_to, is_falling_back }))); } + let rel = if let Some(in_reply_to) = ev.relates_to.in_reply_to { + Some(Relation::Reply { in_reply_to }) + } else { + ev.relates_to.relation.map(|relation| match relation { + #[cfg(feature = "unstable-msc2677")] + RelationJsonRepr::Annotation(a) => Relation::Annotation(a), + RelationJsonRepr::Reference(r) => Relation::Reference(r), + RelationJsonRepr::Replacement(Replacement { event_id }) => { + Relation::Replacement(Replacement { event_id }) + } + RelationJsonRepr::ThreadStable(_) | RelationJsonRepr::ThreadUnstable(_) => { + unreachable!() + } + // FIXME: Maybe we should log this, though at this point we don't even have + // access to the rel_type of the unknown relation. + RelationJsonRepr::Unknown => Relation::_Custom, + }) + }; + + Ok(rel) } impl Serialize for Relation { diff --git a/crates/ruma-common/src/events/room/message.rs b/crates/ruma-common/src/events/room/message.rs index fe3fb852..34d8265d 100644 --- a/crates/ruma-common/src/events/room/message.rs +++ b/crates/ruma-common/src/events/room/message.rs @@ -21,7 +21,7 @@ mod image; mod key_verification_request; mod location; mod notice; -mod relation_serde; +pub(crate) mod relation_serde; mod reply; pub mod sanitize; mod server_notice; diff --git a/crates/ruma-common/src/events/room/message/content_serde.rs b/crates/ruma-common/src/events/room/message/content_serde.rs index fe0b1ccb..e7e2d912 100644 --- a/crates/ruma-common/src/events/room/message/content_serde.rs +++ b/crates/ruma-common/src/events/room/message/content_serde.rs @@ -5,13 +5,13 @@ use serde_json::value::RawValue as RawJsonValue; #[cfg(feature = "unstable-msc3552")] use super::ImageMessageEventContent; +use super::{relation_serde::deserialize_relation, MessageType, RoomMessageEventContent}; #[cfg(feature = "unstable-msc3246")] use super::{AudioInfo, AudioMessageEventContent}; #[cfg(feature = "unstable-msc3551")] use super::{FileInfo, FileMessageEventContent}; #[cfg(feature = "unstable-msc3488")] use super::{LocationInfo, LocationMessageEventContent}; -use super::{MessageType, Relation, RoomMessageEventContent}; #[cfg(feature = "unstable-msc3553")] use super::{VideoInfo, VideoMessageEventContent}; #[cfg(feature = "unstable-msc3246")] @@ -47,8 +47,7 @@ impl<'de> Deserialize<'de> for RoomMessageEventContent { { let json = Box::::deserialize(deserializer)?; let mut deserializer = serde_json::Deserializer::from_str(json.get()); - let relates_to = Option::>::deserialize(&mut deserializer) - .map_err(de::Error::custom)?; + let relates_to = deserialize_relation(&mut deserializer).map_err(de::Error::custom)?; Ok(Self { msgtype: from_raw_json_value(&json)?, relates_to }) } diff --git a/crates/ruma-common/src/events/room/message/relation_serde.rs b/crates/ruma-common/src/events/room/message/relation_serde.rs index ce0dfb5d..280001de 100644 --- a/crates/ruma-common/src/events/room/message/relation_serde.rs +++ b/crates/ruma-common/src/events/room/message/relation_serde.rs @@ -3,51 +3,49 @@ use serde::{Deserialize, Deserializer, Serialize, Serializer}; use super::{InReplyTo, Relation, Replacement, Thread}; use crate::OwnedEventId; -impl<'de, C> Deserialize<'de> for Relation +pub(crate) fn deserialize_relation<'de, D, C>( + deserializer: D, +) -> Result>, D::Error> where + D: Deserializer<'de>, C: Deserialize<'de>, { - fn deserialize(deserializer: D) -> Result - where - D: Deserializer<'de>, + let ev = EventWithRelatesToJsonRepr::deserialize(deserializer)?; + + if let Some( + RelationJsonRepr::ThreadStable(ThreadStableJsonRepr { event_id, is_falling_back }) + | RelationJsonRepr::ThreadUnstable(ThreadUnstableJsonRepr { event_id, is_falling_back }), + ) = ev.relates_to.relation { - let ev = EventWithRelatesToJsonRepr::deserialize(deserializer)?; - - if let Some( - RelationJsonRepr::ThreadStable(ThreadStableJsonRepr { event_id, is_falling_back }) - | RelationJsonRepr::ThreadUnstable(ThreadUnstableJsonRepr { event_id, is_falling_back }), - ) = ev.relates_to.relation - { - let in_reply_to = ev - .relates_to - .in_reply_to - .ok_or_else(|| serde::de::Error::missing_field("m.in_reply_to"))?; - return Ok(Relation::Thread(Thread { event_id, in_reply_to, is_falling_back })); - } - - let rel = if let Some(in_reply_to) = ev.relates_to.in_reply_to { - Relation::Reply { in_reply_to } - } else if let Some(relation) = ev.relates_to.relation { - match relation { - RelationJsonRepr::Replacement(ReplacementJsonRepr { event_id }) => { - let new_content = ev - .new_content - .ok_or_else(|| serde::de::Error::missing_field("m.new_content"))?; - Relation::Replacement(Replacement { event_id, new_content }) - } - // FIXME: Maybe we should log this, though at this point we don't even have - // access to the rel_type of the unknown relation. - RelationJsonRepr::Unknown => Relation::_Custom, - RelationJsonRepr::ThreadStable(_) | RelationJsonRepr::ThreadUnstable(_) => { - unreachable!() - } - } - } else { - Relation::_Custom - }; - - Ok(rel) + let in_reply_to = ev + .relates_to + .in_reply_to + .ok_or_else(|| serde::de::Error::missing_field("m.in_reply_to"))?; + return Ok(Some(Relation::Thread(Thread { event_id, in_reply_to, is_falling_back }))); } + + let rel = if let Some(in_reply_to) = ev.relates_to.in_reply_to { + Some(Relation::Reply { in_reply_to }) + } else if let Some(relation) = ev.relates_to.relation { + match relation { + RelationJsonRepr::Replacement(ReplacementJsonRepr { event_id }) => { + let new_content = ev + .new_content + .ok_or_else(|| serde::de::Error::missing_field("m.new_content"))?; + Some(Relation::Replacement(Replacement { event_id, new_content })) + } + // FIXME: Maybe we should log this, though at this point we don't even have + // access to the rel_type of the unknown relation. + RelationJsonRepr::Unknown => Some(Relation::_Custom), + RelationJsonRepr::ThreadStable(_) | RelationJsonRepr::ThreadUnstable(_) => { + unreachable!() + } + } + } else { + None + }; + + Ok(rel) } impl Serialize for Relation diff --git a/crates/ruma-common/src/events/video.rs b/crates/ruma-common/src/events/video.rs index f3c3cf69..5d0d383f 100644 --- a/crates/ruma-common/src/events/video.rs +++ b/crates/ruma-common/src/events/video.rs @@ -49,7 +49,11 @@ pub struct VideoEventContent { pub caption: Option, /// Information about related messages. - #[serde(flatten, skip_serializing_if = "Option::is_none")] + #[serde( + flatten, + skip_serializing_if = "Option::is_none", + deserialize_with = "crate::events::room::message::relation_serde::deserialize_relation" + )] pub relates_to: Option>, } diff --git a/crates/ruma-common/src/events/voice.rs b/crates/ruma-common/src/events/voice.rs index 2d5b824c..a7b65383 100644 --- a/crates/ruma-common/src/events/voice.rs +++ b/crates/ruma-common/src/events/voice.rs @@ -37,7 +37,11 @@ pub struct VoiceEventContent { pub voice: VoiceContent, /// Information about related messages. - #[serde(flatten, skip_serializing_if = "Option::is_none")] + #[serde( + flatten, + skip_serializing_if = "Option::is_none", + deserialize_with = "crate::events::room::message::relation_serde::deserialize_relation" + )] pub relates_to: Option>, } diff --git a/crates/ruma-common/tests/events/room_message.rs b/crates/ruma-common/tests/events/room_message.rs index 4d26001a..4f2ab44b 100644 --- a/crates/ruma-common/tests/events/room_message.rs +++ b/crates/ruma-common/tests/events/room_message.rs @@ -367,18 +367,19 @@ fn verification_request_deserialization() { ] }); + let content = from_json_value::(json_data).unwrap(); + let verification = assert_matches!( - from_json_value::(json_data), - Ok(RoomMessageEventContent { - msgtype: MessageType::VerificationRequest(verification), - .. - }) => verification + content.msgtype, + MessageType::VerificationRequest(verification) => verification ); assert_eq!(verification.body, "@example:localhost is requesting to verify your key, ..."); assert_eq!(verification.to, user_id); assert_eq!(verification.from_device, device_id); assert_eq!(verification.methods.len(), 3); assert!(verification.methods.contains(&VerificationMethod::SasV1)); + + assert_matches!(content.relates_to, None); } #[test] @@ -413,17 +414,17 @@ fn content_deserialization() { "url": "mxc://example.org/ffed755USFFxlgbQYZGtryd" }); + let content = from_json_value::(json_data).unwrap(); let audio = assert_matches!( - from_json_value::(json_data), - Ok(RoomMessageEventContent { - msgtype: MessageType::Audio(audio), - .. - }) => audio + content.msgtype, + MessageType::Audio(audio) => audio ); assert_eq!(audio.body, "test"); assert_matches!(audio.info, None); let url = assert_matches!(audio.source, MediaSource::Plain(url) => url); assert_eq!(url, "mxc://example.org/ffed755USFFxlgbQYZGtryd"); + + assert_matches!(content.relates_to, None); } #[test]