events: Don't fail event content parsing on invalid relation

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:

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 {
+#[serde(transparent)]
-    /// A custom relation type.
+pub struct CustomRelation(pub(super) JsonObject);
-    pub(super) rel_type: String,
 
-    /// The ID of the event this relation applies to.
+impl CustomRelation {
-    pub(super) event_id: OwnedEventId,
+    pub(super) fn rel_type(&self) -> Option<RelationType> {
-
+        Some(self.0.get("rel_type")?.as_str()?.into())
-    /// Remaining event content.
+    }
-    #[serde(flatten)]
-    pub(super) data: JsonObject,
 }

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()),
+            Relation::_Custom(c) => c.rel_type(),
-        }
-    }
-
-    /// 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,
         }
     }
 
     /// The associated data.
     ///
-    /// The returned JSON object won't contain the `rel_type` field, use
+    /// The returned JSON object holds the contents of `m.relates_to`, including `rel_type` and
-    /// [`.rel_type()`][Self::rel_type] to access it. It also won't contain data
+    /// `event_id` if present, but not things like `m.new_content` for `m.replace` relations that
-    /// outside of `m.relates_to` (e.g. `m.new_content` for `m.replace` relations).
+    /// 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 {
+        if let Relation::_Custom(CustomRelation(data)) = self {
-            Cow::Borrowed(&c.data)
+            Cow::Borrowed(data)
         } else {
             Cow::Owned(self.serialize_data())
         }

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 } =

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<C> Relation<C> {
             Relation::Reply { .. } => None,
             Relation::Replacement(_) => Some(RelationType::Replacement),
             Relation::Thread(_) => Some(RelationType::Thread),
-            Relation::_Custom(c) => Some(c.rel_type.as_str().into()),
+            Relation::_Custom(c) => c.rel_type(),
-        }
-    }
-
-    /// 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,
         }
     }
 
     /// The associated data.
     ///
-    /// The returned JSON object won't contain the `rel_type` field, use
+    /// The returned JSON object holds the contents of `m.relates_to`, including `rel_type` and
-    /// [`.rel_type()`][Self::rel_type] to access it. It also won't contain data
+    /// `event_id` if present, but not things like `m.new_content` for `m.replace` relations that
-    /// outside of `m.relates_to` (e.g. `m.new_content` for `m.replace` relations).
+    /// 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<C> Relation<C> {
     where
         C: Clone,
     {
-        if let Relation::_Custom(c) = self {
+        if let Relation::_Custom(CustomRelation(data)) = self {
-            Cow::Borrowed(&c.data)
+            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()),
+            Self::_Custom(c) => c.rel_type(),
-        }
-    }
-
-    /// 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,
         }
     }
 
     /// The associated data.
     ///
-    /// The returned JSON object won't contain the `rel_type` field, use
+    /// The returned JSON object holds the contents of `m.relates_to`, including `rel_type` and
-    /// [`.rel_type()`][Self::rel_type] to access it.
+    /// `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 {
+        if let Self::_Custom(CustomRelation(data)) = self {
-            Cow::Borrowed(&c.data)
+            Cow::Borrowed(data)
         } else {
             Cow::Owned(self.serialize_data())
         }

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 {
+    let rel = match relation {
-        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<InReplyTo>,
 
     #[serde(flatten)]
-    relation: Option<RelationDeHelper>,
+    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<InReplyTo>,
 
-    #[serde(skip_serializing_if = "Option::is_none")]
-    rel_type: Option<String>,
-
-    #[serde(skip_serializing_if = "Option::is_none")]
-    event_id: Option<OwnedEventId>,
-
     #[serde(flatten, skip_serializing_if = "JsonObject::is_empty")]
     data: JsonObject,
 }
 
 impl From<InReplyTo> 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<CustomRelation> for CustomSerHelper {
-    fn from(value: CustomRelation) -> Self {
+    fn from(CustomRelation(data): CustomRelation) -> Self {
-        let CustomRelation { rel_type, event_id, data } = value;
+        Self { in_reply_to: None, data }
-        Self { rel_type: Some(rel_type), event_id: Some(event_id), data, ..Default::default() }
     }
 }
 

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": {
+        "m.relates_to": relation_json,
-            "rel_type": "io.ruma.custom",
-            "event_id": "$related_event",
-            "field": "value",
-        },
     });
 
-    let content = from_json_value::<RoomEncryptedEventContent>(json).unwrap();
+    let content = from_json_value::<RoomEncryptedEventContent>(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");
+    assert_eq!(JsonValue::Object(relation.data().into_owned()), relation_json);
-    let data = relation.data();
-    assert_eq!(data.get("field").unwrap().as_str(), Some("value"));
 }
 
 #[test]

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": "<text msg>",
-        "m.relates_to": {
+        "m.relates_to": relation_json,
-            "rel_type": "io.ruma.unknown",
-            "event_id": "$related_event",
-            "key": "value",
-        },
     });
 
     assert_matches!(
-        from_json_value::<RoomMessageEventContent>(json),
+        from_json_value::<RoomMessageEventContent>(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");
+    assert_eq!(JsonValue::Object(relation.data().into_owned()), relation_json);
-    let data = relation.data();
-    assert_eq!(data.get("key").unwrap().as_str(), Some("value"));
 }
 
 #[test]

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::<RoomMessageEventContent>(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);
+}