From e1e5803067289f1e8f1147b534e64112e0a1006c Mon Sep 17 00:00:00 2001 From: bjoern Date: Wed, 1 Jun 2022 21:38:15 +0200 Subject: [PATCH] do not add legacy info-messages on resending (#3389) * do not wipe info for drafts * drafts and received webxdc-instances, resent or not, do not trigger info-messages * test that resending a webxdc does not not add legacy info-messages * make clippy happy * update CHANGELOG --- CHANGELOG.md | 1 + src/webxdc.rs | 180 +++++++++++++++++++++++++++++++------------------- 2 files changed, 114 insertions(+), 67 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1b2420efd..8565cb438 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ ### Fixes - delete outgoing MDNs found in the Sent folder on Gmail #3372 - fix searching one-to-one chats #3377 +- do not add legacy info-messages on resending webxdc #3389 ## 1.84.0 diff --git a/src/webxdc.rs b/src/webxdc.rs index 866ae4098..fe2c6d874 100644 --- a/src/webxdc.rs +++ b/src/webxdc.rs @@ -202,41 +202,33 @@ impl Context { instance: &mut Message, update_str: &str, timestamp: i64, + can_info_msg: bool, ) -> Result { let update_str = update_str.trim(); if update_str.is_empty() { bail!("create_status_update_record: empty update."); } - let status_update_item: StatusUpdateItem = { + let status_update_item: StatusUpdateItem = if let Ok(item) = serde_json::from_str::(update_str) { - match instance.state { - MessageState::Undefined - | MessageState::OutPreparing - | MessageState::OutDraft => StatusUpdateItem { - payload: item.payload, - info: None, // no info-messages in draft mode - document: item.document, - summary: item.summary, - }, - _ => item, - } + item } else { bail!("create_status_update_record: no valid update item."); - } - }; + }; - if let Some(ref info) = status_update_item.info { - chat::add_info_msg_with_cmd( - self, - instance.chat_id, - info.as_str(), - SystemMessage::Unknown, - timestamp, - None, - Some(instance), - ) - .await?; + if can_info_msg { + if let Some(ref info) = status_update_item.info { + chat::add_info_msg_with_cmd( + self, + instance.chat_id, + info.as_str(), + SystemMessage::Unknown, + timestamp, + None, + Some(instance), + ) + .await?; + } } let mut param_changed = false; @@ -305,46 +297,50 @@ impl Context { let chat = Chat::load_from_db(self, instance.chat_id).await?; ensure!(chat.can_send(self).await?, "cannot send to {}", chat.id); + let send_now = !matches!( + instance.state, + MessageState::Undefined | MessageState::OutPreparing | MessageState::OutDraft + ); + let status_update_serial = self .create_status_update_record( &mut instance, update_str, dc_create_smeared_timestamp(self).await, + send_now, ) .await?; - match instance.state { - MessageState::Undefined | MessageState::OutPreparing | MessageState::OutDraft => { - // send update once the instance is actually send - Ok(None) - } - _ => { - // send update now - // (also send updates on MessagesState::Failed, maybe only one member cannot receive) - let mut status_update = Message { - chat_id: instance.chat_id, - viewtype: Viewtype::Text, - text: Some(descr.to_string()), - hidden: true, - ..Default::default() - }; - status_update - .param - .set_cmd(SystemMessage::WebxdcStatusUpdate); - status_update.param.set( - Param::Arg, - self.render_webxdc_status_update_object( - instance_msg_id, - Some(status_update_serial), - ) - .await? - .ok_or_else(|| format_err!("Status object expected."))?, - ); - status_update.set_quote(self, Some(&instance)).await?; - status_update.param.remove(Param::GuaranteeE2ee); // may be set by set_quote(), if #2985 is done, this line can be removed - let status_update_msg_id = - chat::send_msg(self, instance.chat_id, &mut status_update).await?; - Ok(Some(status_update_msg_id)) - } + + if send_now { + // send update now + // (also send updates on MessagesState::Failed, maybe only one member cannot receive) + let mut status_update = Message { + chat_id: instance.chat_id, + viewtype: Viewtype::Text, + text: Some(descr.to_string()), + hidden: true, + ..Default::default() + }; + status_update + .param + .set_cmd(SystemMessage::WebxdcStatusUpdate); + status_update.param.set( + Param::Arg, + self.render_webxdc_status_update_object( + instance_msg_id, + Some(status_update_serial), + ) + .await? + .ok_or_else(|| format_err!("Status object expected."))?, + ); + status_update.set_quote(self, Some(&instance)).await?; + status_update.param.remove(Param::GuaranteeE2ee); // may be set by set_quote(), if #2985 is done, this line can be removed + let status_update_msg_id = + chat::send_msg(self, instance.chat_id, &mut status_update).await?; + Ok(Some(status_update_msg_id)) + } else { + // send update once the instance is actually send + Ok(None) } } @@ -368,11 +364,11 @@ impl Context { /// the array is parsed using serde, the single payloads are used as is. pub(crate) async fn receive_status_update(&self, msg_id: MsgId, json: &str) -> Result<()> { let msg = Message::load_from_db(self, msg_id).await?; - let (timestamp, mut instance) = if msg.viewtype == Viewtype::Webxdc { - (msg.timestamp_sort, msg) + let (timestamp, mut instance, can_info_msg) = if msg.viewtype == Viewtype::Webxdc { + (msg.timestamp_sort, msg, false) } else if let Some(parent) = msg.parent(self).await? { if parent.viewtype == Viewtype::Webxdc { - (msg.timestamp_sort, parent) + (msg.timestamp_sort, parent, true) } else { bail!("receive_status_update: message is not the child of a webxdc message.") } @@ -386,6 +382,7 @@ impl Context { &mut instance, &*serde_json::to_string(&update_item)?, timestamp, + can_info_msg, ) .await?; } @@ -620,8 +617,8 @@ mod tests { use async_std::io::WriteExt; use crate::chat::{ - add_contact_to_chat, create_group_chat, forward_msgs, send_msg, send_text_msg, ChatId, - ProtectionStatus, + add_contact_to_chat, create_group_chat, forward_msgs, resend_msgs, send_msg, send_text_msg, + ChatId, ProtectionStatus, }; use crate::chatlist::Chatlist; use crate::config::Config; @@ -801,6 +798,51 @@ mod tests { Ok(()) } + #[async_std::test] + async fn test_resend_webxdc_instance_and_info() -> Result<()> { + // Alice uses webxdc in a group + let alice = TestContext::new_alice().await; + let alice_grp = create_group_chat(&alice, ProtectionStatus::Unprotected, "grp").await?; + let alice_instance = send_webxdc_instance(&alice, alice_grp).await?; + assert_eq!(alice_grp.get_msg_cnt(&alice).await?, 1); + alice + .send_webxdc_status_update( + alice_instance.id, + r#"{"payload":7,"info": "i","summary":"s"}"#, + "d", + ) + .await?; + assert_eq!(alice_grp.get_msg_cnt(&alice).await?, 2); + assert!(alice.get_last_msg_in(alice_grp).await.is_info()); + + // Alice adds Bob and resend already used webxdc + add_contact_to_chat( + &alice, + alice_grp, + Contact::create(&alice, "", "bob@example.net").await?, + ) + .await?; + assert_eq!(alice_grp.get_msg_cnt(&alice).await?, 3); + resend_msgs(&alice, &[alice_instance.id]).await?; + let sent1 = alice.pop_sent_msg().await; + + // Bob received webxdc, legacy info-messages updates are received but not added to the chat + let bob = TestContext::new_bob().await; + let bob_instance = bob.recv_msg(&sent1).await; + assert_eq!(bob_instance.viewtype, Viewtype::Webxdc); + assert!(!bob_instance.is_info()); + assert_eq!( + bob.get_webxdc_status_updates(bob_instance.id, StatusUpdateSerial(0)) + .await?, + r#"[{"payload":7,"info":"i","summary":"s","serial":1,"max_serial":1}]"# + ); + let bob_grp = bob_instance.chat_id; + assert_eq!(bob.get_last_msg_in(bob_grp).await.id, bob_instance.id); + assert_eq!(bob_grp.get_msg_cnt(&bob).await?, 1); + + Ok(()) + } + #[async_std::test] async fn test_receive_webxdc_instance() -> Result<()> { let t = TestContext::new_alice().await; @@ -926,6 +968,7 @@ mod tests { &mut instance, "\n\n{\"payload\": {\"foo\":\"bar\"}}\n", 1640178619, + true, ) .await?; assert_eq!( @@ -935,11 +978,11 @@ mod tests { ); assert!(t - .create_status_update_record(&mut instance, "\n\n\n", 1640178619) + .create_status_update_record(&mut instance, "\n\n\n", 1640178619, true) .await .is_err()); assert!(t - .create_status_update_record(&mut instance, "bad json", 1640178619) + .create_status_update_record(&mut instance, "bad json", 1640178619, true) .await .is_err()); assert_eq!( @@ -953,13 +996,14 @@ mod tests { &mut instance, r#"{"payload" : { "foo2":"bar2"}}"#, 1640178619, + true, ) .await?; assert_eq!( t.get_webxdc_status_updates(instance.id, update_id1).await?, r#"[{"payload":{"foo2":"bar2"},"serial":2,"max_serial":2}]"# ); - t.create_status_update_record(&mut instance, r#"{"payload":true}"#, 1640178619) + t.create_status_update_record(&mut instance, r#"{"payload":true}"#, 1640178619, true) .await?; assert_eq!( t.get_webxdc_status_updates(instance.id, StatusUpdateSerial(0)) @@ -974,6 +1018,7 @@ mod tests { &mut instance, r#"{"payload" : 1, "sender": "that is not used"}"#, 1640178619, + true, ) .await?; assert_eq!( @@ -1243,8 +1288,9 @@ mod tests { bob.get_webxdc_status_updates(bob_instance.id, StatusUpdateSerial(0)) .await?, r#"[{"payload":{"foo":"bar"},"serial":1,"max_serial":2}, -{"payload":42,"serial":2,"max_serial":2}]"# // 'info: "i"' ignored as sent in draft mode +{"payload":42,"info":"i","serial":2,"max_serial":2}]"# ); + assert!(!bob.get_last_msg().await.is_info()); // 'info: "i"' message not added in draft mode Ok(()) }