diff --git a/src/mimefactory.rs b/src/mimefactory.rs index 154c1b3fd..4469fc7d0 100644 --- a/src/mimefactory.rs +++ b/src/mimefactory.rs @@ -726,18 +726,18 @@ impl MimeFactory { } else if header_name == "autocrypt" { unprotected_headers.push(header.clone()); } else if header_name == "from" { - protected_headers.push(header.clone()); - if is_encrypted && verified || is_securejoin_message { - unprotected_headers.push( - Header::new_with_value( - header.name, - vec![Address::new_mailbox(self.from_addr.clone())], - ) - .unwrap(), - ); - } else { - unprotected_headers.push(header); + // Unencrypted securejoin messages should _not_ include the display name: + if is_encrypted || !is_securejoin_message { + protected_headers.push(header.clone()); } + + unprotected_headers.push( + Header::new_with_value( + header.name, + vec![Address::new_mailbox(self.from_addr.clone())], + ) + .unwrap(), + ); } else if header_name == "to" { protected_headers.push(header.clone()); if is_encrypted { @@ -902,12 +902,11 @@ impl MimeFactory { .fold(message, |message, header| message.header(header.clone())); if skip_autocrypt || !context.get_config_bool(Config::SignUnencrypted).await? { - let protected: HashSet
= HashSet::from_iter(protected_headers.into_iter()); - for h in unprotected_headers.split_off(0) { - if !protected.contains(&h) { - unprotected_headers.push(h); - } - } + // Deduplicate unprotected headers that also are in the protected headers: + let protected: HashSet<&str> = + HashSet::from_iter(protected_headers.iter().map(|h| h.name.as_str())); + unprotected_headers.retain(|h| !protected.contains(&h.name.as_str())); + message } else { let message = message.header(get_content_type_directives_header()); diff --git a/src/securejoin.rs b/src/securejoin.rs index 3f5dbca1e..fdd393e69 100644 --- a/src/securejoin.rs +++ b/src/securejoin.rs @@ -843,6 +843,7 @@ mod tests { ); let sent = bob.pop_sent_msg().await; + assert!(!sent.payload.contains("Bob Examplenet")); assert_eq!(sent.recipient(), EmailAddress::new(alice_addr).unwrap()); let msg = alice.parse_msg(&sent).await; assert!(!msg.was_encrypted()); @@ -860,6 +861,7 @@ mod tests { ); let sent = alice.pop_sent_msg().await; + assert!(!sent.payload.contains("Alice Exampleorg")); let msg = bob.parse_msg(&sent).await; assert!(msg.was_encrypted()); assert_eq!( diff --git a/src/test_utils.rs b/src/test_utils.rs index 94c23d6e7..5e0ceb44a 100644 --- a/src/test_utils.rs +++ b/src/test_utils.rs @@ -2,7 +2,7 @@ //! //! This private module is only compiled for test runs. #![allow(clippy::indexing_slicing)] -use std::collections::BTreeMap; +use std::collections::{BTreeMap, HashSet}; use std::fmt::Write; use std::ops::{Deref, DerefMut}; use std::panic; @@ -477,6 +477,36 @@ impl TestContext { update_msg_state(&self.ctx, msg_id, MessageState::OutDelivered) .await .expect("failed to update message state"); + + let payload_headers = payload.split("\r\n\r\n").next().unwrap().lines(); + let payload_header_names: Vec<_> = payload_headers + .map(|h| h.split(':').next().unwrap()) + .collect(); + + // Check that we are sending exactly one From, Subject, Date, To, Message-ID, and MIME-Version header: + for header in &[ + "From", + "Subject", + "Date", + "To", + "Message-ID", + "MIME-Version", + ] { + assert_eq!( + payload_header_names.iter().filter(|h| *h == header).count(), + 1, + "This sent email should contain the header {header} exactly 1 time:\n{payload}" + ); + } + // Check that we aren't sending any header twice: + let mut hash_set = HashSet::new(); + for header_name in payload_header_names { + assert!( + hash_set.insert(header_name), + "This sent email shouldn't contain the header {header_name} multiple times:\n{payload}" + ); + } + Some(SentMessage { payload, sender_msg_id: msg_id, diff --git a/src/tests/verified_chats.rs b/src/tests/verified_chats.rs index 732047eb0..757158e34 100644 --- a/src/tests/verified_chats.rs +++ b/src/tests/verified_chats.rs @@ -881,7 +881,7 @@ async fn test_verified_member_added_reordering() -> Result<()> { } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn test_no_unencrypted_name_if_verified() -> Result<()> { +async fn test_no_unencrypted_name_if_encrypted() -> Result<()> { let mut tcm = TestContextManager::new(); for verified in [false, true] { let alice = tcm.alice().await; @@ -898,7 +898,7 @@ async fn test_no_unencrypted_name_if_verified() -> Result<()> { let chat_id = bob.create_chat(&alice).await.id; let msg = &bob.send_text(chat_id, "hi").await; - assert_eq!(msg.payload.contains("Bob Smith"), !verified); + assert_eq!(msg.payload.contains("Bob Smith"), false); assert!(msg.payload.contains("BEGIN PGP MESSAGE")); let msg = alice.recv_msg(msg).await;