From 4157d1986fdca5b1a370ba7a3615626c0bfff3a5 Mon Sep 17 00:00:00 2001 From: iequidoo Date: Sun, 10 Aug 2025 13:13:09 -0300 Subject: [PATCH] fix: Add messages that can't be verified as DownloadState::Available (#7059) We haven't dropped verified groups yet, so we need to do smth with messages that can't be verified yet which often occurs because of messages reordering, particularly in large groups. Apart from the reported case #7059, i had other direct reports that sometimes messages can't be verified for various reasons, but when the reason is already fixed, it should be possible to re-download failed messages and see them. Also remove the code replacing the message text with a verification error from `apply_group_changes()` as `add_parts()` already does this. --- src/receive_imf.rs | 27 ++++++++++---------- src/receive_imf/receive_imf_tests.rs | 38 ++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 14 deletions(-) diff --git a/src/receive_imf.rs b/src/receive_imf.rs index 1ab08fed2..185a2626d 100644 --- a/src/receive_imf.rs +++ b/src/receive_imf.rs @@ -1863,6 +1863,7 @@ async fn add_parts( None }; + let mut verification_failed = false; if !chat_id.is_special() && is_partial_download.is_none() { // For outgoing emails in the 1:1 chat we have an exception that // they are allowed to be unencrypted: @@ -1874,8 +1875,9 @@ async fn add_parts( // likely reinstalled DC" or similar) would be wrong. if chat.is_protected() && (mime_parser.incoming || chat.typ != Chattype::Single) { if let VerifiedEncryption::NotVerified(err) = verified_encryption { + verification_failed = true; warn!(context, "Verification problem: {err:#}."); - let s = format!("{err}. See 'Info' for more details"); + let s = format!("{err}. Re-download the message or see 'Info' for more details"); mime_parser.replace_msg_by_error(&s); } } @@ -2137,6 +2139,10 @@ RETURNING id DownloadState::Available } else if mime_parser.decrypting_failed { DownloadState::Undecipherable + } else if verification_failed { + // Verification can fail because of message reordering. Re-downloading the + // message should help if so. + DownloadState::Available } else { DownloadState::Done }, @@ -2871,20 +2877,13 @@ async fn apply_group_changes( let is_from_in_chat = !chat_contacts.contains(&ContactId::SELF) || chat_contacts.contains(&from_id); - if mime_parser.get_header(HeaderDef::ChatVerified).is_some() { + if mime_parser.get_header(HeaderDef::ChatVerified).is_some() && !chat.is_protected() { if let VerifiedEncryption::NotVerified(err) = verified_encryption { - if chat.is_protected() { - warn!(context, "Verification problem: {err:#}."); - let s = format!("{err}. See 'Info' for more details"); - mime_parser.replace_msg_by_error(&s); - } else { - warn!( - context, - "Not marking chat {} as protected due to verification problem: {err:#}.", - chat.id - ); - } - } else if !chat.is_protected() { + warn!( + context, + "Not marking chat {} as protected due to verification problem: {err:#}.", chat.id, + ); + } else { chat.id .set_protection( context, diff --git a/src/receive_imf/receive_imf_tests.rs b/src/receive_imf/receive_imf_tests.rs index ef7a31233..d6ffc48f0 100644 --- a/src/receive_imf/receive_imf_tests.rs +++ b/src/receive_imf/receive_imf_tests.rs @@ -5091,6 +5091,44 @@ async fn test_two_group_securejoins() -> Result<()> { Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_unverified_member_msg() -> Result<()> { + let mut tcm = TestContextManager::new(); + let alice = &tcm.alice().await; + let bob = &tcm.bob().await; + let fiona = &tcm.fiona().await; + + let alice_chat_id = + chat::create_group_chat(alice, ProtectionStatus::Protected, "Group").await?; + let qr = get_securejoin_qr(alice, Some(alice_chat_id)).await?; + + tcm.exec_securejoin_qr(bob, alice, &qr).await; + tcm.exec_securejoin_qr(fiona, alice, &qr).await; + + let fiona_chat_id = fiona.get_last_msg().await.chat_id; + let fiona_sent_msg = fiona.send_text(fiona_chat_id, "Hi").await; + + // The message can't be verified, but the user can re-download it. + let bob_msg = bob.recv_msg(&fiona_sent_msg).await; + assert_eq!(bob_msg.download_state, DownloadState::Available); + assert!( + bob_msg + .text + .contains("Re-download the message or see 'Info' for more details") + ); + + let alice_sent_msg = alice + .send_text(alice_chat_id, "Hi all, it's Alice introducing Fiona") + .await; + bob.recv_msg(&alice_sent_msg).await; + + // Now Bob has Fiona's key and can verify the message. + let bob_msg = bob.recv_msg(&fiona_sent_msg).await; + assert_eq!(bob_msg.download_state, DownloadState::Done); + assert_eq!(bob_msg.text, "Hi"); + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_sanitize_filename_in_received() -> Result<()> { let alice = &TestContext::new_alice().await;