xmpp: Receive message corrections, unchecked

xmppftw created

Change summary

xmpp/ChangeLog                         |  3 +
xmpp/src/event.rs                      | 20 +++++++
xmpp/src/message/receive/chat.rs       | 76 ++++++++++++++++-----------
xmpp/src/message/receive/group_chat.rs | 67 ++++++++++++++++++++----
xmpp/src/message/receive/mod.rs        | 10 ++-
5 files changed, 130 insertions(+), 46 deletions(-)

Detailed changes

xmpp/ChangeLog 🔗

@@ -16,6 +16,9 @@ XXXX-YY-ZZ [ RELEASER <admin@localhost> ]
     * Added:
       - Agent::send_room_message takes RoomMessageSettings argument (!483)
       - Agent::send_raw_message takes RawMessageSettings for any message type (!487)
+      - Event::ChatMessageCorrection, Event::RoomMessageCorrection, and
+        Event::RoomPrivateMessageCorrection signal XEP-0308 message corrections; they're
+        not checked how old the corrected entry is, which has security concerns (!496)
     * Fixes:
       - Use tokio::sync::RwLock not std::sync::RwLock (!432)
       - Agent::wait_for_events now return Vec<Event> and sets inner tokio_xmpp Client

xmpp/src/event.rs 🔗

@@ -26,9 +26,22 @@ pub enum Event {
     /// - The [`Body`] is the message body.
     /// - The [`StanzaTimeInfo`] about when message was received, and when the message was claimed sent.
     ChatMessage(Id, BareJid, Body, StanzaTimeInfo),
+    /// A message in a one-to-one chat was corrected/edited.
+    /// - The [`Id`] is the ID of the message that was corrected (always Some)
+    /// - The [`BareJid`] is the JID of the other participant in the chat.
+    /// - The [`Body`] is the new body of the message, to replace the old one.
+    /// - The [`StanzaTimeInfo`] is the time the message correction was sent/received
+    ChatMessageCorrection(Id, BareJid, Body, StanzaTimeInfo),
     RoomJoined(BareJid),
     RoomLeft(BareJid),
     RoomMessage(Id, BareJid, RoomNick, Body, StanzaTimeInfo),
+    /// A message in a MUC was corrected/edited.
+    /// - The [`Id`] is the ID of the message that was corrected (always Some)
+    /// - The [`BareJid`] is the JID of the room where the message was sent.
+    /// - The [`RoomNick`] is the nickname of the sender of the message.
+    /// - The [`Body`] is the new body of the message, to replace the old one.
+    /// - The [`StanzaTimeInfo`] is the time the message correction was sent/received
+    RoomMessageCorrection(Id, BareJid, RoomNick, Body, StanzaTimeInfo),
     /// The subject of a room was received.
     /// - The BareJid is the room's address.
     /// - The RoomNick is the nickname of the room member who set the subject.
@@ -37,6 +50,13 @@ pub enum Event {
     /// A private message received from a room, containing the message ID, the room's BareJid,
     /// the sender's nickname, and the message body.
     RoomPrivateMessage(Id, BareJid, RoomNick, Body, StanzaTimeInfo),
+    /// A private message in a MUC was corrected/edited.
+    /// - The [`Id`] is the ID of the message that was corrected (always Some)
+    /// - The [`BareJid`] is the JID of the room where the message was sent.
+    /// - The [`RoomNick`] is the nickname of the sender of the message.
+    /// - The [`Body`] is the new body of the message, to replace the old one.
+    /// - The [`StanzaTimeInfo`] is the time the message correction was sent/received
+    RoomPrivateMessageCorrection(Id, BareJid, RoomNick, Body, StanzaTimeInfo),
     ServiceMessage(Id, BareJid, Body, StanzaTimeInfo),
     HttpUploadedFile(String),
 }

xmpp/src/message/receive/chat.rs 🔗

@@ -6,7 +6,7 @@
 
 use tokio_xmpp::{
     jid::Jid,
-    parsers::{message::Message, muc::user::MucUser},
+    parsers::{message::Message, message_correct::Replace, muc::user::MucUser},
 };
 
 use crate::{delay::StanzaTimeInfo, Agent, Event, RoomNick};
@@ -15,44 +15,56 @@ pub async fn handle_message_chat(
     agent: &mut Agent,
     events: &mut Vec<Event>,
     from: Jid,
-    message: &Message,
+    message: &mut Message,
     time_info: StanzaTimeInfo,
 ) {
     let langs: Vec<&str> = agent.lang.iter().map(String::as_str).collect();
-    if let Some((_lang, body)) = message.get_best_body(langs) {
-        let mut found_special_message = false;
 
-        for payload in &message.payloads {
-            if let Ok(_) = MucUser::try_from(payload.clone()) {
-                let event = match from.clone().try_into_full() {
-                    Err(bare) => {
-                        // TODO: Can a service message be of type Chat/Normal and not Groupchat?
-                        warn!("Received malformed MessageType::Chat in muc#user namespace from a bare JID.");
-                        Event::ServiceMessage(
-                            message.id.clone(),
-                            bare,
-                            body.clone(),
-                            time_info.clone(),
-                        )
-                    }
-                    Ok(full) => Event::RoomPrivateMessage(
-                        message.id.clone(),
-                        full.to_bare(),
-                        RoomNick::from_resource_ref(full.resource()),
-                        body.clone(),
-                        time_info.clone(),
-                    ),
-                };
+    let Some((_lang, body)) = message.get_best_body_cloned(langs) else {
+        debug!("Received normal/chat message without body:\n{:#?}", message);
+        return;
+    };
 
-                found_special_message = true;
-                events.push(event);
-            }
-        }
+    let is_muc_pm = message.extract_valid_payload::<MucUser>().is_some();
+    let correction = message.extract_valid_payload::<Replace>();
+
+    if is_muc_pm {
+        if from.resource().is_none() {
+            warn!("Received malformed MessageType::Chat in muc#user namespace from a bare JID:\n{:#?}", message);
+        } else {
+            let full_from = from.clone().try_into_full().unwrap();
 
-        if !found_special_message {
-            let event =
-                Event::ChatMessage(message.id.clone(), from.to_bare(), body.clone(), time_info);
+            let event = if let Some(correction) = correction {
+                Event::RoomPrivateMessageCorrection(
+                    Some(correction.id),
+                    full_from.to_bare(),
+                    RoomNick::from_resource_ref(full_from.resource()),
+                    body.clone(),
+                    time_info,
+                )
+            } else {
+                Event::RoomPrivateMessage(
+                    message.id.clone(),
+                    from.to_bare(),
+                    RoomNick::from_resource_ref(full_from.resource()),
+                    body.clone(),
+                    time_info,
+                )
+            };
             events.push(event);
         }
+    } else {
+        let event = if let Some(correction) = correction {
+            // TODO: Check that correction is valid (only for last N minutes or last N messages)
+            Event::ChatMessageCorrection(
+                Some(correction.id),
+                from.to_bare(),
+                body.clone(),
+                time_info,
+            )
+        } else {
+            Event::ChatMessage(message.id.clone(), from.to_bare(), body.clone(), time_info)
+        };
+        events.push(event);
     }
 }

xmpp/src/message/receive/group_chat.rs 🔗

@@ -4,18 +4,22 @@
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
-use tokio_xmpp::{jid::Jid, parsers::message::Message};
-
-use crate::{delay::StanzaTimeInfo, Agent, Event, RoomNick};
+use crate::{
+    delay::StanzaTimeInfo,
+    jid::Jid,
+    parsers::{message::Message, message_correct::Replace},
+    Agent, Event, RoomNick,
+};
 
 pub async fn handle_message_group_chat(
     agent: &mut Agent,
     events: &mut Vec<Event>,
     from: Jid,
-    message: &Message,
+    message: &mut Message,
     time_info: StanzaTimeInfo,
 ) {
     let langs: Vec<&str> = agent.lang.iter().map(String::as_str).collect();
+    let mut found_subject = false;
 
     if let Some((_lang, subject)) = message.get_best_subject(langs.clone()) {
         events.push(Event::RoomSubject(
@@ -24,19 +28,60 @@ pub async fn handle_message_group_chat(
             subject.0.clone(),
             time_info.clone(),
         ));
+        found_subject = true;
     }
 
-    if let Some((_lang, body)) = message.get_best_body(langs) {
-        let event = match from.clone().try_into_full() {
-            Ok(full) => Event::RoomMessage(
+    let Some((_lang, body)) = message.get_best_body_cloned(langs) else {
+        if !found_subject {
+            debug!(
+                "Received groupchat message without body/subject:\n{:#?}",
+                message
+            );
+        }
+        return;
+    };
+
+    let correction = message.extract_payload::<Replace>().unwrap_or_else(|e| {
+        warn!("Failed to parse <replace> payload: {e}");
+        None
+    });
+
+    // Now we have a groupchat message... which can be:
+    //
+    // - a normal MUC message from a user in a room
+    // - a MUC message correction from a user in a room
+    // - a service message from a MUC channel (barejid)
+    //
+    // In theory we can have service message correction but nope nope nope
+
+    if let Some(resource) = from.resource() {
+        // User message/correction
+
+        let event = if let Some(correction) = correction {
+            Event::RoomMessageCorrection(
+                Some(correction.id),
+                from.to_bare(),
+                RoomNick::from_resource_ref(resource),
+                body.clone(),
+                time_info,
+            )
+        } else {
+            Event::RoomMessage(
                 message.id.clone(),
                 from.to_bare(),
-                RoomNick::from_resource_ref(full.resource()),
+                RoomNick::from_resource_ref(resource),
                 body.clone(),
                 time_info,
-            ),
-            Err(bare) => Event::ServiceMessage(message.id.clone(), bare, body.clone(), time_info),
+            )
         };
-        events.push(event)
+        events.push(event);
+    } else {
+        // Service message
+        if correction.is_some() {
+            warn!("Found correction in service message:\n{:#?}", message);
+        } else {
+            let event = Event::ServiceMessage(message.id.clone(), from.to_bare(), body, time_info);
+            events.push(event);
+        }
     }
 }

xmpp/src/message/receive/mod.rs 🔗

@@ -14,7 +14,7 @@ use crate::{delay::message_time_info, pubsub, Agent, Event};
 pub mod chat;
 pub mod group_chat;
 
-pub async fn handle_message(agent: &mut Agent, message: Message) -> Vec<Event> {
+pub async fn handle_message(agent: &mut Agent, mut message: Message) -> Vec<Event> {
     let mut events = vec![];
     let from = message.from.clone().unwrap();
     let time_info = message_time_info(&message);
@@ -25,17 +25,21 @@ pub async fn handle_message(agent: &mut Agent, message: Message) -> Vec<Event> {
                 agent,
                 &mut events,
                 from.clone(),
-                &message,
+                &mut message,
                 time_info,
             )
             .await;
         }
         MessageType::Chat | MessageType::Normal => {
-            chat::handle_message_chat(agent, &mut events, from.clone(), &message, time_info).await;
+            chat::handle_message_chat(agent, &mut events, from.clone(), &mut message, time_info)
+                .await;
         }
         _ => {}
     }
 
+    // TODO: should this be here or in specific branch of messagetype?
+    // We may drop some payloads in branches before (&mut message), but
+    // that's ok because pubsub only wants the pubsub payload.
     for child in message.payloads {
         if child.is("event", ns::PUBSUB_EVENT) {
             let new_events = pubsub::handle_event(&from, child, agent).await;