From 93d9a4be1c6f9ef2d7ccfe282acc6edce7585fb2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?bi=C3=B8rn?= Date: Wed, 5 Aug 2026 10:22:19 +0200 Subject: [PATCH] feat: allow only default reactions (#8545) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit the UIs are currently advised to only allow the five default reactions in broadcast channels. this PR ensures that from sending site as well as receiving site. soon, we probably want to make the possible reactions configurable, this PR is mainly for some safety until then. once we have an API to change default reactions, we can also easily test "invalid" reactions on the receiving side; currently only sending is tested successor of https://github.com/chatmail/core/pull/8450 --------- Co-authored-by: Jagoda Estera Ślązak <128227338+j-g00da@users.noreply.github.com> --- src/reaction.rs | 18 +++++++- src/reaction/broadcast_reactions.rs | 64 ++++++++++++++++++++++------- 2 files changed, 66 insertions(+), 16 deletions(-) diff --git a/src/reaction.rs b/src/reaction.rs index e129c766c..f51d01463 100644 --- a/src/reaction.rs +++ b/src/reaction.rs @@ -32,7 +32,8 @@ use crate::events::EventType; use crate::message::{Message, MsgId, rfc724_mid_exists}; use crate::param::Param; use crate::reaction::broadcast_reactions::{ - load_broadcast_reactions, modify_frequencies, refine_frequencies, save_broadcast_reactions, + is_allowed_reaction, load_broadcast_reactions, modify_frequencies, refine_frequencies, + save_broadcast_reactions, }; /// A single reaction. @@ -133,6 +134,12 @@ async fn set_msg_id_reaction( let old_reactions = get_msg_reactions(context, msg_id).await?; let old_self_reaction = old_reactions.by_contact.get(&ContactId::SELF); + if matches!(chat.typ, Chattype::OutBroadcast | Chattype::InBroadcast) + && !is_allowed_reaction(reaction) + { + bail!("Reaction not allowed: {}", reaction.as_str()); + } + if reaction.is_empty() { // Simply remove the record instead of setting it to empty string. context @@ -233,10 +240,17 @@ async fn set_pending_reaction( /// `reaction` is a string consisting of a single emoji. Use /// empty string to retract a reaction. pub async fn send_reaction(context: &Context, msg_id: MsgId, reaction: &str) -> Result { + let reaction = Reaction::new(reaction); let msg = Message::load_from_db(context, msg_id).await?; let chat_id = msg.chat_id; + let chat = Chat::load_from_db(context, chat_id).await?; + + if matches!(chat.typ, Chattype::OutBroadcast | Chattype::InBroadcast) + && !is_allowed_reaction(&reaction) + { + bail!("Reaction not allowed: {}", reaction.as_str()); + } - let reaction = Reaction::new(reaction); let mut reaction_msg = Message::new_text(reaction.as_str().to_string()); reaction_msg.set_reaction(); reaction_msg.in_reply_to = Some(msg.rfc724_mid); diff --git a/src/reaction/broadcast_reactions.rs b/src/reaction/broadcast_reactions.rs index a1d2079bb..3f6a0e0f0 100644 --- a/src/reaction/broadcast_reactions.rs +++ b/src/reaction/broadcast_reactions.rs @@ -73,6 +73,14 @@ pub(crate) async fn render_json(context: &Context, msg_ids: &[MsgId]) -> Result< Ok(Some(json)) } +/// Emojis allowed as reactions in broadcast channels. +const ALLOWED_REACTIONS: [&str; 5] = ["👍", "👎", "❤️", "😂", "🙁"]; + +/// Check if a reaction is an allowed reaction in a broadcast channel. +pub(crate) fn is_allowed_reaction(reaction: &Reaction) -> bool { + reaction.is_empty() || ALLOWED_REACTIONS.contains(&reaction.as_str()) +} + /// Seconds between sending out accumulated reaction updates for broadcast channels from `reactions_need_broadcast` table const REACTION_BROADCAST_PERIOD: i64 = 10 * 60; @@ -593,19 +601,19 @@ mod tests { assert_eq!(claire_msg.get_text(), "hi channel!"); // Bob reacts to the message - send_reaction(bob, bob_msg.id, "🏳️‍🌈").await?; + send_reaction(bob, bob_msg.id, "❤️").await?; let sent_msg = bob.pop_sent_msg().await; let reactions = get_msg_reactions(bob, bob_msg.id).await?; - assert_eq!(reactions.to_string(), "🏳️‍🌈1"); + assert_eq!(reactions.to_string(), "❤️1"); // Alice receives Bob's reaction alice.recv_msg_hidden(&sent_msg).await; let reactions = get_msg_reactions(alice, alice_msg_id).await?; - assert_eq!(reactions.to_string(), "🏳️‍🌈1"); + assert_eq!(reactions.to_string(), "❤️1"); // Alice broadcasts recent reaction changes to Bob and Claire. // On the wire, the hidden message has a header like - // `Chat-Broadcast-Reactions: {"messages":[{"id":"123@adc","reactions":[{"emoji":"🏳️‍🌈","count":1}]}]}` + // `Chat-Broadcast-Reactions: {"messages":[{"id":"123@adc","reactions":[{"emoji":"❤️","count":1}]}]}` maybe_broadcast_reactions(alice).await?; let sent_msg = alice.pop_sent_msg().await; bob.recv_msg_hidden(&sent_msg).await; @@ -619,13 +627,13 @@ mod tests { // Claire got the broadcasted reaction, and then reacts herself. // This means, her local view on reactions are a mix `broadcasted_reactions`and `reactions`. let reactions = get_msg_reactions(claire, claire_msg.id).await?; - assert_eq!(reactions.to_string(), "🏳️‍🌈1"); + assert_eq!(reactions.to_string(), "❤️1"); assert_eq!(reactions.frequencies.len(), 1); assert_eq!(reactions.by_contact.len(), 0); - send_reaction(claire, claire_msg.id, "💪").await?; + send_reaction(claire, claire_msg.id, "👍").await?; let reactions = get_msg_reactions(claire, claire_msg.id).await?; - assert_eq!(reactions.to_string(), "🏳️‍🌈1 💪1"); + assert_eq!(reactions.to_string(), "❤️1 👍1"); assert_eq!(reactions.frequencies.len(), 2); assert_eq!(reactions.frequencies[0].is_from_self, false); assert_eq!(reactions.frequencies[1].is_from_self, true); @@ -635,7 +643,7 @@ mod tests { let sent_msg = claire.pop_sent_msg().await; alice.recv_msg_hidden(&sent_msg).await; let reactions = get_msg_reactions(alice, alice_msg_id).await?; - assert_eq!(reactions.to_string(), "🏳️‍🌈1 💪1"); + assert_eq!(reactions.to_string(), "❤️1 👍1"); broadcast_reactions_for_all_chats(alice).await?; // bypass timer in maybe_broadcast_reactions() let sent_msg = alice.pop_sent_msg().await; @@ -643,22 +651,22 @@ mod tests { claire.recv_msg_hidden(&sent_msg).await; let reactions = get_msg_reactions(claire, claire_msg.id).await?; - assert_eq!(reactions.to_string(), "🏳️‍🌈1 💪1"); + assert_eq!(reactions.to_string(), "❤️1 👍1"); assert_eq!(reactions.frequencies.len(), 2); assert_eq!(reactions.frequencies[0].is_from_self, false); assert_eq!(reactions.frequencies[1].is_from_self, true); - // Claire removes her 💪 reaction, and also reactios with 🏳️‍🌈; + // Claire removes her 👍 reaction, and also reactios with ❤️; // SELF-changes are immediate even tho not broadcasted yet, the bring broadcasted reactions table to a "dirty state" ... send_reaction(claire, claire_msg.id, "").await?; let sent_msg = claire.pop_sent_msg().await; let reactions = get_msg_reactions(claire, claire_msg.id).await?; - assert_eq!(reactions.to_string(), "🏳️‍🌈1"); + assert_eq!(reactions.to_string(), "❤️1"); - send_reaction(claire, claire_msg.id, "🏳️‍🌈").await?; + send_reaction(claire, claire_msg.id, "❤️").await?; let sent_msg2 = claire.pop_sent_msg().await; let reactions = get_msg_reactions(claire, claire_msg.id).await?; - assert_eq!(reactions.to_string(), "🏳️‍🌈2"); + assert_eq!(reactions.to_string(), "❤️2"); // ... "dirty state" is fixed after next broadcast then, counters should stay the same alice.recv_msg_hidden(&sent_msg).await; @@ -667,7 +675,7 @@ mod tests { let sent_msg = alice.pop_sent_msg().await; claire.recv_msg_hidden(&sent_msg).await; let reactions = get_msg_reactions(claire, claire_msg.id).await?; - assert_eq!(reactions.to_string(), "🏳️‍🌈2"); + assert_eq!(reactions.to_string(), "❤️2"); Ok(()) } @@ -702,4 +710,32 @@ mod tests { Ok(()) } + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn test_broadcast_subscriber_sends_unallowed_reaction() -> Result<()> { + let mut tcm = TestContextManager::new(); + let alice = &tcm.alice().await; + let bob = &tcm.bob().await; + + // Alice creates a channel, Bob joins + let alice_chat_id = create_broadcast(alice, "Channel".to_string()).await?; + let qr = get_securejoin_qr(alice, Some(alice_chat_id)).await?; + let bob_chat_id = tcm.exec_securejoin_qr(bob, alice, &qr).await; + bob_chat_id.accept(bob).await?; + + // Alice sends a message to the channel, Alice cannot react to her own message with unallowed emoji + let sent_msg = alice.send_text(alice_chat_id, "hi channel!").await; + let alice_msg_id = sent_msg.load_from_db().await.id; + let res = send_reaction(alice, alice_msg_id, "💩").await; + assert!(res.is_err()); + assert!(alice.pop_sent_msg_opt().await.is_none()); + + // Bob receives the message and reacts unallowed + let bob_msg = bob.recv_msg(&sent_msg).await; + let res = send_reaction(bob, bob_msg.id, "🤮").await; + assert!(res.is_err()); + assert!(bob.pop_sent_msg_opt().await.is_none()); + + Ok(()) + } }