Skip to content

Commit d6af8d2

Browse files
committed
feat: mimefactory: Order message recipients by time of addition (#6872)
Sort recipients by `add_timestamp DESC` so that if the group is large and there are multiple SMTP messages, a newly added member receives the member addition message earlier and has gossiped keys of other members (otherwise the new member may receive messages from other members earlier and fail to verify them).
1 parent 1209e95 commit d6af8d2

File tree

2 files changed

+45
-1
lines changed

2 files changed

+45
-1
lines changed

src/mimefactory.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,10 @@ impl MimeFactory {
253253
let mut missing_key_addresses = BTreeSet::new();
254254
context
255255
.sql
256+
// Sort recipients by `add_timestamp DESC` so that if the group is large and there
257+
// are multiple SMTP messages, a newly added member receives the member addition
258+
// message earlier and has gossiped keys of other members (otherwise the new member
259+
// may receive messages from other members earlier and fail to verify them).
256260
.query_map(
257261
"SELECT
258262
c.authname,
@@ -266,7 +270,8 @@ impl MimeFactory {
266270
LEFT JOIN contacts c ON cc.contact_id=c.id
267271
LEFT JOIN public_keys k ON k.fingerprint=c.fingerprint
268272
WHERE cc.chat_id=?
269-
AND (cc.contact_id>9 OR (cc.contact_id=1 AND ?))",
273+
AND (cc.contact_id>9 OR (cc.contact_id=1 AND ?))
274+
ORDER BY cc.add_timestamp DESC",
270275
(msg.chat_id, chat.typ == Chattype::Group),
271276
|row| {
272277
let authname: String = row.get(0)?;

src/mimefactory/mimefactory_tests.rs

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ use deltachat_contact_tools::ContactAddress;
22
use mail_builder::headers::Header;
33
use mailparse::{MailHeaderMap, addrparse_header};
44
use std::str;
5+
use std::time::Duration;
56

67
use super::*;
78
use crate::chat::{
@@ -16,6 +17,7 @@ use crate::message;
1617
use crate::mimeparser::MimeMessage;
1718
use crate::receive_imf::receive_imf;
1819
use crate::test_utils::{TestContext, TestContextManager, get_chat_msg};
20+
use crate::tools::SystemTime;
1921

2022
fn render_email_address(display_name: &str, addr: &str) -> String {
2123
let mut output = Vec::<u8>::new();
@@ -867,6 +869,43 @@ async fn test_dont_remove_self() -> Result<()> {
867869
Ok(())
868870
}
869871

872+
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
873+
async fn test_new_member_is_first_recipient() -> Result<()> {
874+
let mut tcm = TestContextManager::new();
875+
let alice = &tcm.alice().await;
876+
let bob = &tcm.bob().await;
877+
let charlie = &tcm.charlie().await;
878+
879+
let bob_id = alice.add_or_lookup_contact_id(bob).await;
880+
let charlie_id = alice.add_or_lookup_contact_id(charlie).await;
881+
882+
let group = alice
883+
.create_group_with_members(ProtectionStatus::Unprotected, "Group", &[bob])
884+
.await;
885+
alice.send_text(group, "Hi! I created a group.").await;
886+
887+
SystemTime::shift(Duration::from_secs(60));
888+
add_contact_to_chat(alice, group, charlie_id).await?;
889+
let sent_msg = alice.pop_sent_msg().await;
890+
assert!(
891+
sent_msg
892+
.recipients
893+
.starts_with(&charlie.get_config(Config::Addr).await?.unwrap())
894+
);
895+
896+
remove_contact_from_chat(alice, group, bob_id).await?;
897+
alice.pop_sent_msg().await;
898+
SystemTime::shift(Duration::from_secs(60));
899+
add_contact_to_chat(alice, group, bob_id).await?;
900+
let sent_msg = alice.pop_sent_msg().await;
901+
assert!(
902+
sent_msg
903+
.recipients
904+
.starts_with(&bob.get_config(Config::Addr).await?.unwrap())
905+
);
906+
Ok(())
907+
}
908+
870909
/// Regression test: mimefactory should never create an empty to header,
871910
/// also not if the Selftalk parameter is missing
872911
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]

0 commit comments

Comments
 (0)