diff --git a/src/receive_imf.rs b/src/receive_imf.rs index 010ba31df..4222e69ab 100644 --- a/src/receive_imf.rs +++ b/src/receive_imf.rs @@ -754,8 +754,12 @@ async fn add_parts( new_protection = ProtectionStatus::ProtectionBroken; } + // The message itself will be sorted under the device message since the device + // message is `MessageState::InNoticed`, which means that all following + // messages are sorted under it. let sort_timestamp = - calc_sort_timestamp(context, sent_timestamp, chat_id, true).await?; + calc_sort_timestamp(context, sent_timestamp, chat_id, true, incoming) + .await?; chat_id .set_protection(context, new_protection, sort_timestamp, Some(from_id)) .await?; @@ -948,7 +952,8 @@ async fn add_parts( }; let in_fresh = state == MessageState::InFresh; - let sort_timestamp = calc_sort_timestamp(context, sent_timestamp, chat_id, in_fresh).await?; + let sort_timestamp = + calc_sort_timestamp(context, sent_timestamp, chat_id, false, incoming).await?; // Apply ephemeral timer changes to the chat. // @@ -1376,25 +1381,41 @@ async fn calc_sort_timestamp( context: &Context, message_timestamp: i64, chat_id: ChatId, - is_fresh_msg: bool, + always_sort_to_bottom: bool, + incoming: bool, ) -> Result { let mut sort_timestamp = message_timestamp; - // get newest non fresh message for this chat - // update sort_timestamp if less than that - if is_fresh_msg { - let last_msg_time: Option = context + let last_msg_time: Option = if always_sort_to_bottom { + // get newest message for this chat + context .sql .query_get_value( - "SELECT MAX(timestamp) FROM msgs WHERE chat_id=? AND state>?", - (chat_id, MessageState::InFresh), + "SELECT MAX(timestamp) FROM msgs WHERE chat_id=?", + (chat_id,), ) - .await?; + .await? + } else if incoming { + // get newest incoming non fresh message for this chat. - if let Some(last_msg_time) = last_msg_time { - if last_msg_time > sort_timestamp { - sort_timestamp = last_msg_time; - } + // If a user hasn't been online for some time, the Inbox is + // fetched first and then the Sentbox. In order for Inbox + // and Sent messages to be allowed to mingle, + // outgoing messages are purely sorted by their sent timestamp. + context + .sql + .query_get_value( + "SELECT MAX(timestamp) FROM msgs WHERE chat_id=? AND state>? AND from_id!=?", + (chat_id, MessageState::InFresh, ContactId::SELF), + ) + .await? + } else { + None + }; + + if let Some(last_msg_time) = last_msg_time { + if last_msg_time > sort_timestamp { + sort_timestamp = last_msg_time; } } diff --git a/src/tests/verified_chats.rs b/src/tests/verified_chats.rs index 5fb892c35..e8a8f3e7f 100644 --- a/src/tests/verified_chats.rs +++ b/src/tests/verified_chats.rs @@ -313,6 +313,173 @@ async fn test_verified_oneonone_chat_enable_disable() -> Result<()> { Ok(()) } +/// Messages with old timestamps are difficult for verified chats: +/// - They must not be sorted over a protection-changed info message. +/// That's what `test_old_message_2` tests +/// - If they change the protection, then they must not be sorted over existing other messages, +/// because then the protection-changed info message would also be above these existing messages. +/// That's what `test_old_message_3` tests. +/// +/// `test_old_message_1` tests the case where both the old and the new message +/// change verification +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_old_message_1() -> Result<()> { + let mut tcm = TestContextManager::new(); + let alice = tcm.alice().await; + let bob = tcm.bob().await; + enable_verified_oneonone_chats(&[&alice, &bob]).await; + + mark_as_verified(&alice, &bob).await; + + let chat = alice.create_chat(&bob).await; // This creates a protection-changed info message + assert!(chat.is_protected()); + + // This creates protection-changed info message #2; + // even though the date is old, info message and email must be sorted below the original info message. + receive_imf( + &alice, + b"From: Bob \n\ + To: alice@example.org\n\ + Message-ID: <1234-2-3@example.org>\n\ + Date: Sat, 07 Dec 2019 19:00:27 +0000\n\ + \n\ + Message from Thunderbird\n", + true, + ) + .await?; + + alice.golden_test_chat(chat.id, "test_old_message_1").await; + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_old_message_2() -> Result<()> { + let mut tcm = TestContextManager::new(); + let alice = tcm.alice().await; + let bob = tcm.bob().await; + enable_verified_oneonone_chats(&[&alice, &bob]).await; + + mark_as_verified(&alice, &bob).await; + + // This creates protection-changed info message #1: + let chat = alice.create_chat(&bob).await; + assert!(chat.is_protected()); + let protection_msg = alice.get_last_msg().await; + assert_eq!( + protection_msg.param.get_cmd(), + SystemMessage::ChatProtectionEnabled + ); + + // This creates protection-changed info message #2. + let first_email = receive_imf( + &alice, + b"From: Bob \n\ + To: alice@example.org\n\ + Message-ID: <1234-2-3@example.org>\n\ + Date: Sun, 08 Dec 2019 19:00:27 +0000\n\ + \n\ + Somewhat old message\n", + false, + ) + .await? + .unwrap(); + + // Both messages will get the same timestamp as the protection-changed + // message, so this one will be sorted under the previous one + // even though it has an older timestamp. + let second_email = receive_imf( + &alice, + b"From: Bob \n\ + To: alice@example.org\n\ + Message-ID: <2319-2-3@example.org>\n\ + Date: Sat, 07 Dec 2019 19:00:27 +0000\n\ + \n\ + Even older message, that must NOT be shown before the info message\n", + true, + ) + .await? + .unwrap(); + + assert_eq!(first_email.sort_timestamp, second_email.sort_timestamp); + assert_eq!(first_email.sort_timestamp, protection_msg.timestamp_sort); + + alice.golden_test_chat(chat.id, "test_old_message_2").await; + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_old_message_3() -> Result<()> { + let mut tcm = TestContextManager::new(); + let alice = tcm.alice().await; + let bob = tcm.bob().await; + enable_verified_oneonone_chats(&[&alice, &bob]).await; + + mark_as_verified(&alice, &bob).await; + mark_as_verified(&bob, &alice).await; + + tcm.send_recv_accept(&bob, &alice, "Heyho from my verified device!") + .await; + + // This unverified message must not be sorted over the message sent in the previous line: + receive_imf( + &alice, + b"From: Bob \n\ + To: alice@example.org\n\ + Message-ID: <1234-2-3@example.org>\n\ + Date: Sat, 07 Dec 2019 19:00:27 +0000\n\ + \n\ + Old, unverified message\n", + true, + ) + .await?; + + alice + .golden_test_chat(alice.get_chat(&bob).await.unwrap().id, "test_old_message_3") + .await; + + Ok(()) +} + +/// Alice is offline for some time. +/// When she comes online, first her inbox is synced and then her sentbox. +/// This test tests that the messages are still in the right order. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_old_message_4() -> Result<()> { + let alice = TestContext::new_alice().await; + let msg_incoming = receive_imf( + &alice, + b"From: Bob \n\ + To: alice@example.org\n\ + Message-ID: <1234-2-3@example.org>\n\ + Date: Sun, 08 Dec 2019 19:00:27 +0000\n\ + \n\ + Thanks, Alice!\n", + true, + ) + .await? + .unwrap(); + + let msg_sent = receive_imf( + &alice, + b"From: alice@example.org\n\ + To: Bob \n\ + Message-ID: <1234-2-4@example.org>\n\ + Date: Sat, 07 Dec 2019 19:00:27 +0000\n\ + \n\ + Happy birthday, Bob!\n", + true, + ) + .await? + .unwrap(); + + // The "Happy birthday" message should be shown first, and then the "Thanks" message + assert!(msg_sent.sort_timestamp < msg_incoming.sort_timestamp); + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_mdn_doesnt_disable_verification() -> Result<()> { let mut tcm = TestContextManager::new(); diff --git a/test-data/golden/test_old_message_1 b/test-data/golden/test_old_message_1 new file mode 100644 index 000000000..61e54d298 --- /dev/null +++ b/test-data/golden/test_old_message_1 @@ -0,0 +1,6 @@ +Single#Chat#10: Bob [bob@example.net] +-------------------------------------------------------------------------------- +Msg#10: info (Contact#Contact#Info): Messages are guaranteed to be end-to-end encrypted from now on. [NOTICED][INFO 🛡️] +Msg#11: info (Contact#Contact#Info): Bob sent a message from another device. [NOTICED][INFO 🛡️❌] +Msg#12: (Contact#Contact#10): Message from Thunderbird [SEEN] +-------------------------------------------------------------------------------- diff --git a/test-data/golden/test_old_message_2 b/test-data/golden/test_old_message_2 new file mode 100644 index 000000000..d7e7712ab --- /dev/null +++ b/test-data/golden/test_old_message_2 @@ -0,0 +1,7 @@ +Single#Chat#10: Bob [bob@example.net] +-------------------------------------------------------------------------------- +Msg#10: info (Contact#Contact#Info): Messages are guaranteed to be end-to-end encrypted from now on. [NOTICED][INFO 🛡️] +Msg#11: info (Contact#Contact#Info): Bob sent a message from another device. [NOTICED][INFO 🛡️❌] +Msg#12: (Contact#Contact#10): Somewhat old message [FRESH] +Msg#13: (Contact#Contact#10): Even older message, that must NOT be shown before the info message [SEEN] +-------------------------------------------------------------------------------- diff --git a/test-data/golden/test_old_message_3 b/test-data/golden/test_old_message_3 new file mode 100644 index 000000000..30d5ed041 --- /dev/null +++ b/test-data/golden/test_old_message_3 @@ -0,0 +1,7 @@ +Single#Chat#10: Bob [bob@example.net] +-------------------------------------------------------------------------------- +Msg#10: info (Contact#Contact#Info): Messages are guaranteed to be end-to-end encrypted from now on. [NOTICED][INFO 🛡️] +Msg#11🔒: (Contact#Contact#10): Heyho from my verified device! [FRESH] +Msg#12: info (Contact#Contact#Info): Bob sent a message from another device. [NOTICED][INFO 🛡️❌] +Msg#13: (Contact#Contact#10): Old, unverified message [SEEN] +--------------------------------------------------------------------------------