Skip to content

Commit 4157d19

Browse files
committed
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.
1 parent d13eb2f commit 4157d19

File tree

2 files changed

+51
-14
lines changed

2 files changed

+51
-14
lines changed

src/receive_imf.rs

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1863,6 +1863,7 @@ async fn add_parts(
18631863
None
18641864
};
18651865

1866+
let mut verification_failed = false;
18661867
if !chat_id.is_special() && is_partial_download.is_none() {
18671868
// For outgoing emails in the 1:1 chat we have an exception that
18681869
// they are allowed to be unencrypted:
@@ -1874,8 +1875,9 @@ async fn add_parts(
18741875
// likely reinstalled DC" or similar) would be wrong.
18751876
if chat.is_protected() && (mime_parser.incoming || chat.typ != Chattype::Single) {
18761877
if let VerifiedEncryption::NotVerified(err) = verified_encryption {
1878+
verification_failed = true;
18771879
warn!(context, "Verification problem: {err:#}.");
1878-
let s = format!("{err}. See 'Info' for more details");
1880+
let s = format!("{err}. Re-download the message or see 'Info' for more details");
18791881
mime_parser.replace_msg_by_error(&s);
18801882
}
18811883
}
@@ -2137,6 +2139,10 @@ RETURNING id
21372139
DownloadState::Available
21382140
} else if mime_parser.decrypting_failed {
21392141
DownloadState::Undecipherable
2142+
} else if verification_failed {
2143+
// Verification can fail because of message reordering. Re-downloading the
2144+
// message should help if so.
2145+
DownloadState::Available
21402146
} else {
21412147
DownloadState::Done
21422148
},
@@ -2871,20 +2877,13 @@ async fn apply_group_changes(
28712877
let is_from_in_chat =
28722878
!chat_contacts.contains(&ContactId::SELF) || chat_contacts.contains(&from_id);
28732879

2874-
if mime_parser.get_header(HeaderDef::ChatVerified).is_some() {
2880+
if mime_parser.get_header(HeaderDef::ChatVerified).is_some() && !chat.is_protected() {
28752881
if let VerifiedEncryption::NotVerified(err) = verified_encryption {
2876-
if chat.is_protected() {
2877-
warn!(context, "Verification problem: {err:#}.");
2878-
let s = format!("{err}. See 'Info' for more details");
2879-
mime_parser.replace_msg_by_error(&s);
2880-
} else {
2881-
warn!(
2882-
context,
2883-
"Not marking chat {} as protected due to verification problem: {err:#}.",
2884-
chat.id
2885-
);
2886-
}
2887-
} else if !chat.is_protected() {
2882+
warn!(
2883+
context,
2884+
"Not marking chat {} as protected due to verification problem: {err:#}.", chat.id,
2885+
);
2886+
} else {
28882887
chat.id
28892888
.set_protection(
28902889
context,

src/receive_imf/receive_imf_tests.rs

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5091,6 +5091,44 @@ async fn test_two_group_securejoins() -> Result<()> {
50915091
Ok(())
50925092
}
50935093

5094+
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
5095+
async fn test_unverified_member_msg() -> Result<()> {
5096+
let mut tcm = TestContextManager::new();
5097+
let alice = &tcm.alice().await;
5098+
let bob = &tcm.bob().await;
5099+
let fiona = &tcm.fiona().await;
5100+
5101+
let alice_chat_id =
5102+
chat::create_group_chat(alice, ProtectionStatus::Protected, "Group").await?;
5103+
let qr = get_securejoin_qr(alice, Some(alice_chat_id)).await?;
5104+
5105+
tcm.exec_securejoin_qr(bob, alice, &qr).await;
5106+
tcm.exec_securejoin_qr(fiona, alice, &qr).await;
5107+
5108+
let fiona_chat_id = fiona.get_last_msg().await.chat_id;
5109+
let fiona_sent_msg = fiona.send_text(fiona_chat_id, "Hi").await;
5110+
5111+
// The message can't be verified, but the user can re-download it.
5112+
let bob_msg = bob.recv_msg(&fiona_sent_msg).await;
5113+
assert_eq!(bob_msg.download_state, DownloadState::Available);
5114+
assert!(
5115+
bob_msg
5116+
.text
5117+
.contains("Re-download the message or see 'Info' for more details")
5118+
);
5119+
5120+
let alice_sent_msg = alice
5121+
.send_text(alice_chat_id, "Hi all, it's Alice introducing Fiona")
5122+
.await;
5123+
bob.recv_msg(&alice_sent_msg).await;
5124+
5125+
// Now Bob has Fiona's key and can verify the message.
5126+
let bob_msg = bob.recv_msg(&fiona_sent_msg).await;
5127+
assert_eq!(bob_msg.download_state, DownloadState::Done);
5128+
assert_eq!(bob_msg.text, "Hi");
5129+
Ok(())
5130+
}
5131+
50945132
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
50955133
async fn test_sanitize_filename_in_received() -> Result<()> {
50965134
let alice = &TestContext::new_alice().await;

0 commit comments

Comments
 (0)