diff --git a/crates/ruma-events/CHANGELOG.md b/crates/ruma-events/CHANGELOG.md index 371ddb13..2b14d6a8 100644 --- a/crates/ruma-events/CHANGELOG.md +++ b/crates/ruma-events/CHANGELOG.md @@ -1,5 +1,15 @@ # [unreleased] +Breaking changes: + +- Remove `event_id` methods from relation types + +Improvements: + +- Don't fail event content parsing on invalid relation + - We previously already accepted custom or slightly malformed relations + - Now, even invalid / missing `rel_type` and `event_id` are accepted + # 0.27.9 Bug fixes: diff --git a/crates/ruma-events/src/relation.rs b/crates/ruma-events/src/relation.rs index c560cddb..75d453d3 100644 --- a/crates/ruma-events/src/relation.rs +++ b/crates/ruma-events/src/relation.rs @@ -316,14 +316,11 @@ pub enum RelationType { /// The payload for a custom relation. #[doc(hidden)] #[derive(Clone, Debug, Deserialize, Serialize)] -pub struct CustomRelation { - /// A custom relation type. - pub(super) rel_type: String, +#[serde(transparent)] +pub struct CustomRelation(pub(super) JsonObject); - /// The ID of the event this relation applies to. - pub(super) event_id: OwnedEventId, - - /// Remaining event content. - #[serde(flatten)] - pub(super) data: JsonObject, +impl CustomRelation { + pub(super) fn rel_type(&self) -> Option { + Some(self.0.get("rel_type")?.as_str()?.into()) + } } diff --git a/crates/ruma-events/src/room/encrypted.rs b/crates/ruma-events/src/room/encrypted.rs index 8942f1fc..2bef8535 100644 --- a/crates/ruma-events/src/room/encrypted.rs +++ b/crates/ruma-events/src/room/encrypted.rs @@ -5,7 +5,7 @@ use std::{borrow::Cow, collections::BTreeMap}; use js_int::UInt; -use ruma_common::{serde::JsonObject, EventId, OwnedDeviceId, OwnedEventId}; +use ruma_common::{serde::JsonObject, OwnedDeviceId, OwnedEventId}; use ruma_macros::EventContent; use serde::{Deserialize, Serialize}; @@ -118,36 +118,21 @@ impl Relation { Relation::Reference(_) => Some(RelationType::Reference), Relation::Annotation(_) => Some(RelationType::Annotation), Relation::Thread(_) => Some(RelationType::Thread), - Relation::_Custom(c) => Some(c.rel_type.as_str().into()), - } - } - - /// The ID of the event this relates to. - /// - /// This is the `event_id` field at the root of an `m.relates_to` object, except in the case of - /// a reply relation where it's the `event_id` field in the `m.in_reply_to` object. - pub fn event_id(&self) -> &EventId { - match self { - Relation::Reply { in_reply_to } => &in_reply_to.event_id, - Relation::Replacement(r) => &r.event_id, - Relation::Reference(r) => &r.event_id, - Relation::Annotation(a) => &a.event_id, - Relation::Thread(t) => &t.event_id, - Relation::_Custom(c) => &c.event_id, + Relation::_Custom(c) => c.rel_type(), } } /// The associated data. /// - /// The returned JSON object won't contain the `rel_type` field, use - /// [`.rel_type()`][Self::rel_type] to access it. It also won't contain data - /// outside of `m.relates_to` (e.g. `m.new_content` for `m.replace` relations). + /// The returned JSON object holds the contents of `m.relates_to`, including `rel_type` and + /// `event_id` if present, but not things like `m.new_content` for `m.replace` relations that + /// live next to `m.relates_to`. /// /// Prefer to use the public variants of `Relation` where possible; this method is meant to /// be used for custom relations only. pub fn data(&self) -> Cow<'_, JsonObject> { - if let Relation::_Custom(c) = self { - Cow::Borrowed(&c.data) + if let Relation::_Custom(CustomRelation(data)) = self { + Cow::Borrowed(data) } else { Cow::Owned(self.serialize_data()) } diff --git a/crates/ruma-events/src/room/encrypted/relation_serde.rs b/crates/ruma-events/src/room/encrypted/relation_serde.rs index 51bdf954..89516fab 100644 --- a/crates/ruma-events/src/room/encrypted/relation_serde.rs +++ b/crates/ruma-events/src/room/encrypted/relation_serde.rs @@ -2,7 +2,7 @@ use ruma_common::{ serde::{from_raw_json_value, JsonObject}, OwnedEventId, }; -use serde::{de, ser::SerializeStruct, Deserialize, Deserializer, Serialize}; +use serde::{ser::SerializeStruct, Deserialize, Deserializer, Serialize}; use serde_json::{value::RawValue as RawJsonValue, Value as JsonValue}; use super::{InReplyTo, Relation, Thread}; @@ -17,7 +17,6 @@ impl<'de> Deserialize<'de> for Relation { let RelationDeHelper { in_reply_to, rel_type } = from_raw_json_value(&json)?; let rel = match (in_reply_to, rel_type.as_deref()) { - (None, None) => return Err(de::Error::missing_field("m.in_reply_to or rel_type")), (_, Some("m.thread")) => Relation::Thread(from_raw_json_value(&json)?), (in_reply_to, Some("io.element.thread")) => { let ThreadUnstableDeHelper { event_id, is_falling_back } = diff --git a/crates/ruma-events/src/room/message/relation.rs b/crates/ruma-events/src/room/message/relation.rs index 6f089866..58654516 100644 --- a/crates/ruma-events/src/room/message/relation.rs +++ b/crates/ruma-events/src/room/message/relation.rs @@ -1,6 +1,6 @@ use std::borrow::Cow; -use ruma_common::{serde::JsonObject, EventId}; +use ruma_common::serde::JsonObject; use crate::relation::{CustomRelation, InReplyTo, RelationType, Replacement, Thread}; @@ -34,28 +34,15 @@ impl Relation { Relation::Reply { .. } => None, Relation::Replacement(_) => Some(RelationType::Replacement), Relation::Thread(_) => Some(RelationType::Thread), - Relation::_Custom(c) => Some(c.rel_type.as_str().into()), - } - } - - /// The ID of the event this relates to. - /// - /// This is the `event_id` field at the root of an `m.relates_to` object, except in the case of - /// a reply relation where it's the `event_id` field in the `m.in_reply_to` object. - pub fn event_id(&self) -> &EventId { - match self { - Relation::Reply { in_reply_to } => &in_reply_to.event_id, - Relation::Replacement(r) => &r.event_id, - Relation::Thread(t) => &t.event_id, - Relation::_Custom(c) => &c.event_id, + Relation::_Custom(c) => c.rel_type(), } } /// The associated data. /// - /// The returned JSON object won't contain the `rel_type` field, use - /// [`.rel_type()`][Self::rel_type] to access it. It also won't contain data - /// outside of `m.relates_to` (e.g. `m.new_content` for `m.replace` relations). + /// The returned JSON object holds the contents of `m.relates_to`, including `rel_type` and + /// `event_id` if present, but not things like `m.new_content` for `m.replace` relations that + /// live next to `m.relates_to`. /// /// Prefer to use the public variants of `Relation` where possible; this method is meant to /// be used for custom relations only. @@ -63,8 +50,8 @@ impl Relation { where C: Clone, { - if let Relation::_Custom(c) = self { - Cow::Borrowed(&c.data) + if let Relation::_Custom(CustomRelation(data)) = self { + Cow::Borrowed(data) } else { Cow::Owned(self.serialize_data()) } @@ -97,32 +84,21 @@ impl RelationWithoutReplacement { match self { Self::Reply { .. } => None, Self::Thread(_) => Some(RelationType::Thread), - Self::_Custom(c) => Some(c.rel_type.as_str().into()), - } - } - - /// The ID of the event this relates to. - /// - /// This is the `event_id` field at the root of an `m.relates_to` object, except in the case of - /// a reply relation where it's the `event_id` field in the `m.in_reply_to` object. - pub fn event_id(&self) -> &EventId { - match self { - Self::Reply { in_reply_to } => &in_reply_to.event_id, - Self::Thread(t) => &t.event_id, - Self::_Custom(c) => &c.event_id, + Self::_Custom(c) => c.rel_type(), } } /// The associated data. /// - /// The returned JSON object won't contain the `rel_type` field, use - /// [`.rel_type()`][Self::rel_type] to access it. + /// The returned JSON object holds the contents of `m.relates_to`, including `rel_type` and + /// `event_id` if present, but not things like `m.new_content` for `m.replace` relations that + /// live next to `m.relates_to`. /// /// Prefer to use the public variants of `Relation` where possible; this method is meant to /// be used for custom relations only. pub fn data(&self) -> Cow<'_, JsonObject> { - if let Self::_Custom(c) = self { - Cow::Borrowed(&c.data) + if let Self::_Custom(CustomRelation(data)) = self { + Cow::Borrowed(data) } else { Cow::Owned(self.serialize_data()) } diff --git a/crates/ruma-events/src/room/message/relation_serde.rs b/crates/ruma-events/src/room/message/relation_serde.rs index 6d97d454..e5bfb657 100644 --- a/crates/ruma-events/src/room/message/relation_serde.rs +++ b/crates/ruma-events/src/room/message/relation_serde.rs @@ -35,8 +35,8 @@ where let RelatesToDeHelper { in_reply_to, relation } = relates_to; - let rel = if let Some(RelationDeHelper::Known(relation)) = relation { - match relation { + let rel = match relation { + RelationDeHelper::Known(relation) => match relation { KnownRelationDeHelper::Replacement(ReplacementJsonRepr { event_id }) => { match new_content { Some(new_content) => { @@ -50,13 +50,14 @@ where event_id, is_falling_back, }) => Relation::Thread(Thread { event_id, in_reply_to, is_falling_back }), + }, + RelationDeHelper::Unknown(c) => { + if let Some(in_reply_to) = in_reply_to { + Relation::Reply { in_reply_to } + } else { + Relation::_Custom(c) + } } - } else if let Some(in_reply_to) = in_reply_to { - Relation::Reply { in_reply_to } - } else if let Some(RelationDeHelper::Unknown(c)) = relation { - Relation::_Custom(c) - } else { - return Err(de::Error::missing_field("m.in_reply_to or rel_type")); }; Ok(Some(rel)) @@ -91,7 +92,7 @@ pub(crate) struct RelatesToDeHelper { in_reply_to: Option, #[serde(flatten)] - relation: Option, + relation: RelationDeHelper, } #[derive(Deserialize)] @@ -200,26 +201,19 @@ pub(super) struct CustomSerHelper { #[serde(rename = "m.in_reply_to", skip_serializing_if = "Option::is_none")] in_reply_to: Option, - #[serde(skip_serializing_if = "Option::is_none")] - rel_type: Option, - - #[serde(skip_serializing_if = "Option::is_none")] - event_id: Option, - #[serde(flatten, skip_serializing_if = "JsonObject::is_empty")] data: JsonObject, } impl From for CustomSerHelper { fn from(value: InReplyTo) -> Self { - Self { in_reply_to: Some(value), ..Default::default() } + Self { in_reply_to: Some(value), data: JsonObject::new() } } } impl From for CustomSerHelper { - fn from(value: CustomRelation) -> Self { - let CustomRelation { rel_type, event_id, data } = value; - Self { rel_type: Some(rel_type), event_id: Some(event_id), data, ..Default::default() } + fn from(CustomRelation(data): CustomRelation) -> Self { + Self { in_reply_to: None, data } } } diff --git a/crates/ruma-events/tests/it/encrypted.rs b/crates/ruma-events/tests/it/encrypted.rs index a3c6b618..8778f5bb 100644 --- a/crates/ruma-events/tests/it/encrypted.rs +++ b/crates/ruma-events/tests/it/encrypted.rs @@ -7,7 +7,9 @@ use ruma_events::{ RoomEncryptedEventContent, }, }; -use serde_json::{from_value as from_json_value, json, to_value as to_json_value}; +use serde_json::{ + from_value as from_json_value, json, to_value as to_json_value, Value as JsonValue, +}; fn encrypted_scheme() -> EncryptedEventScheme { EncryptedEventScheme::MegolmV1AesSha2( @@ -429,7 +431,12 @@ fn content_annotation_deserialization() { #[test] fn custom_relation_deserialization() { - let json = json!({ + let relation_json = json!({ + "rel_type": "io.ruma.custom", + "event_id": "$related_event", + "field": "value", + }); + let content_json = json!({ "algorithm": "m.megolm.v1.aes-sha2", "sender_key": "aV9BpqYFqJpKYmgERyGv/6QyKMcgLqxM05V0gvzg9Yk", "ciphertext": "AwgAEpABjy6BHczo7UZE3alyej6y2YQ5v+L9eB+fBqL7yteCPv8Jig\ @@ -440,14 +447,10 @@ fn custom_relation_deserialization() { lDl5mzVO3tPnJMKZ0hn+AF", "session_id": "IkwqWxT2zy3DI1E/zM2Wq+CE8tr3eEpsxsVGjGrMPdw", "device_id": "DEVICE", - "m.relates_to": { - "rel_type": "io.ruma.custom", - "event_id": "$related_event", - "field": "value", - }, + "m.relates_to": relation_json, }); - let content = from_json_value::(json).unwrap(); + let content = from_json_value::(content_json).unwrap(); assert_matches!(content.scheme, EncryptedEventScheme::MegolmV1AesSha2(encrypted_content)); assert_eq!(encrypted_content.session_id, "IkwqWxT2zy3DI1E/zM2Wq+CE8tr3eEpsxsVGjGrMPdw"); @@ -463,9 +466,7 @@ fn custom_relation_deserialization() { let relation = content.relates_to.unwrap(); assert_eq!(relation.rel_type().unwrap().as_str(), "io.ruma.custom"); - assert_eq!(relation.event_id(), "$related_event"); - let data = relation.data(); - assert_eq!(data.get("field").unwrap().as_str(), Some("value")); + assert_eq!(JsonValue::Object(relation.data().into_owned()), relation_json); } #[test] diff --git a/crates/ruma-events/tests/it/relations.rs b/crates/ruma-events/tests/it/relations.rs index ab2c25a9..50b6a0fc 100644 --- a/crates/ruma-events/tests/it/relations.rs +++ b/crates/ruma-events/tests/it/relations.rs @@ -5,7 +5,9 @@ use ruma_events::{ relation::{CustomRelation, InReplyTo, Replacement, Thread}, room::message::{MessageType, Relation, RoomMessageEventContent}, }; -use serde_json::{from_value as from_json_value, json, to_value as to_json_value}; +use serde_json::{ + from_value as from_json_value, json, to_value as to_json_value, Value as JsonValue, +}; #[test] fn reply_deserialize() { @@ -250,18 +252,19 @@ fn thread_unstable_deserialize() { #[test] fn custom_deserialize() { - let json = json!({ + let relation_json = json!({ + "rel_type": "io.ruma.unknown", + "event_id": "$related_event", + "key": "value", + }); + let content_json = json!({ "msgtype": "m.text", "body": "", - "m.relates_to": { - "rel_type": "io.ruma.unknown", - "event_id": "$related_event", - "key": "value", - }, + "m.relates_to": relation_json, }); assert_matches!( - from_json_value::(json), + from_json_value::(content_json), Ok(RoomMessageEventContent { msgtype: MessageType::Text(_), relates_to: Some(relation), @@ -269,9 +272,7 @@ fn custom_deserialize() { }) ); assert_eq!(relation.rel_type().unwrap().as_str(), "io.ruma.unknown"); - assert_eq!(relation.event_id().as_str(), "$related_event"); - let data = relation.data(); - assert_eq!(data.get("key").unwrap().as_str(), Some("value")); + assert_eq!(JsonValue::Object(relation.data().into_owned()), relation_json); } #[test] diff --git a/crates/ruma-events/tests/it/room_message.rs b/crates/ruma-events/tests/it/room_message.rs index 548b500f..c72008e3 100644 --- a/crates/ruma-events/tests/it/room_message.rs +++ b/crates/ruma-events/tests/it/room_message.rs @@ -21,7 +21,9 @@ use ruma_events::{ }, AnySyncTimelineEvent, Mentions, MessageLikeUnsigned, }; -use serde_json::{from_value as from_json_value, json, to_value as to_json_value}; +use serde_json::{ + from_value as from_json_value, json, to_value as to_json_value, Value as JsonValue, +}; macro_rules! json_object { ( $($tt:tt)+ ) => { @@ -1230,3 +1232,29 @@ fn mentions_room_deserialization() { let mentions = content.mentions.unwrap(); assert!(mentions.room); } + +#[test] +fn invalid_replacement() { + // As generated by Element Web: https://github.com/vector-im/element-web/issues/26554 + let relation = json!({ + "rel_type": "m.replace", + "event_id": "~!kCCQTCfnABLKGGvQjo:matrix.org:m1699715385559.77", + }); + let json_data = json!({ + "msgtype": "m.text", + "body": " * edited text", + "m.new_content": { + "msgtype": "m.text", + "body": "edited text", + "m.mentions": {}, + }, + "m.mentions": {}, + "m.relates_to": relation + }); + + let content = from_json_value::(json_data).unwrap(); + let relates_to = content.relates_to.unwrap(); + let data = relates_to.data(); + assert_matches!(&data, Cow::Borrowed(_)); // data is stored in JSON form because it's invalid + assert_eq!(JsonValue::Object(data.into_owned()), relation); +}