diff --git a/src/decrypt.rs b/src/decrypt.rs index 7be616c309..e3a8380e52 100644 --- a/src/decrypt.rs +++ b/src/decrypt.rs @@ -259,7 +259,8 @@ mod tests { let bob = TestContext::new_bob().await; receive_imf(&bob, attachment_mime, false).await?; let msg = bob.get_last_msg().await; - assert_eq!(msg.text, "Hello from Thunderbird!"); + // Subject should be prepended because the attachment doesn't have "Chat-Version". + assert_eq!(msg.text, "Hello, Bob! – Hello from Thunderbird!"); Ok(()) } diff --git a/src/mimeparser.rs b/src/mimeparser.rs index fe7afe15a2..80c7f0d057 100644 --- a/src/mimeparser.rs +++ b/src/mimeparser.rs @@ -419,26 +419,9 @@ impl MimeMessage { timestamp_sent = Self::get_timestamp_sent(&mail.headers, timestamp_sent, timestamp_rcvd); if !signatures.is_empty() { - // Remove unsigned opportunistically protected headers from messages considered - // Autocrypt-encrypted / displayed with padlock. // For "Subject" see . - for h in [ - HeaderDef::Subject, - HeaderDef::ChatGroupId, - HeaderDef::ChatGroupName, - HeaderDef::ChatGroupNameChanged, - HeaderDef::ChatGroupNameTimestamp, - HeaderDef::ChatGroupAvatar, - HeaderDef::ChatGroupMemberRemoved, - HeaderDef::ChatGroupMemberAdded, - HeaderDef::ChatGroupMemberTimestamps, - HeaderDef::ChatGroupPastMembers, - HeaderDef::ChatDelete, - HeaderDef::ChatEdit, - HeaderDef::ChatUserAvatar, - ] { - headers.remove(h.get_headername()); - } + // Other headers are removed by `MimeMessage::merge_headers()`. + headers.remove("subject"); } // let known protected headers from the decrypted @@ -1546,12 +1529,15 @@ impl MimeMessage { chat_disposition_notification_to: &mut Option, fields: &[mailparse::MailHeader<'_>], ) { + // Keep Subject so that it's displayed for signed-only messages. They are shown w/o a + // padlock anyway. + headers.retain(|k, _| !is_protected(k) || k == "subject"); for field in fields { // lowercasing all headers is technically not correct, but makes things work better let key = field.get_key().to_lowercase(); - if !headers.contains_key(&key) || // key already exists, only overwrite known types (protected headers) - is_known(&key) || key.starts_with("chat-") - { + // Don't overwrite unprotected headers, but overwrite protected ones because DKIM + // signature applies to the last headers. + if !headers.contains_key(&key) || is_protected(&key) { if key == HeaderDef::ChatDispositionNotificationTo.get_headername() { match addrparse_header(field) { Ok(addrlist) => { @@ -1966,23 +1952,26 @@ pub(crate) fn parse_message_id(ids: &str) -> Result { /// Returns true if the header overwrites outer header /// when it comes from protected headers. -fn is_known(key: &str) -> bool { - matches!( - key, - "return-path" - | "date" - | "from" - | "sender" - | "reply-to" - | "to" - | "cc" - | "bcc" - | "message-id" - | "in-reply-to" - | "references" - | "subject" - | "secure-join" - ) +fn is_protected(key: &str) -> bool { + key.starts_with("chat-") + || matches!( + key, + "return-path" + | "auto-submitted" + | "autocrypt-setup-message" + | "date" + | "from" + | "sender" + | "reply-to" + | "to" + | "cc" + | "bcc" + | "message-id" + | "in-reply-to" + | "references" + | "subject" + | "secure-join" + ) } /// Returns if the header is hidden and must be ignored in the IMF section. diff --git a/src/mimeparser/mimeparser_tests.rs b/src/mimeparser/mimeparser_tests.rs index 19a3095f60..27cf3d7359 100644 --- a/src/mimeparser/mimeparser_tests.rs +++ b/src/mimeparser/mimeparser_tests.rs @@ -1418,6 +1418,26 @@ async fn test_x_microsoft_original_message_id_precedence() -> Result<()> { Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_extra_imf_chat_header() -> Result<()> { + let mut tcm = TestContextManager::new(); + let t = &tcm.alice().await; + let chat_id = t.get_self_chat().await.id; + + chat::send_text_msg(t, chat_id, "hi!".to_string()).await?; + let sent_msg = t.pop_sent_msg().await; + // Check removal of some nonexistent "Chat-*" header to protect the code from future breakages. + let payload = sent_msg + .payload + .replace("Message-ID:", "Chat-Forty-Two: 42\r\nMessage-ID:"); + let msg = MimeMessage::from_bytes(t, payload.as_bytes(), None) + .await + .unwrap(); + assert!(msg.headers.contains_key("chat-version")); + assert!(!msg.headers.contains_key("chat-forty-two")); + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_long_in_reply_to() -> Result<()> { let t = TestContext::new_alice().await; diff --git a/src/receive_imf/receive_imf_tests.rs b/src/receive_imf/receive_imf_tests.rs index 4df492c679..a8d30112ea 100644 --- a/src/receive_imf/receive_imf_tests.rs +++ b/src/receive_imf/receive_imf_tests.rs @@ -4052,6 +4052,24 @@ async fn test_unsigned_chat_group_hdr() -> Result<()> { Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_ignore_protected_headers_in_outer_msg() -> Result<()> { + let mut tcm = TestContextManager::new(); + let alice = &tcm.alice().await; + let bob = &tcm.bob().await; + let bob_chat_id = tcm.send_recv_accept(alice, bob, "hi").await.chat_id; + send_text_msg(bob, bob_chat_id, "hi all!".to_string()).await?; + let mut sent_msg = bob.pop_sent_msg().await; + sent_msg.payload = sent_msg.payload.replace( + "Chat-Version:", + "Auto-Submitted: auto-generated\r\nChat-Version:", + ); + alice.recv_msg(&sent_msg).await; + let ab_contact = alice.add_or_lookup_contact(bob).await; + assert!(!ab_contact.is_bot()); + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_sync_member_list_on_rejoin() -> Result<()> { let mut tcm = TestContextManager::new();