diff --git a/src/chat/chat_tests.rs b/src/chat/chat_tests.rs index e60c93fcd..be3d206d9 100644 --- a/src/chat/chat_tests.rs +++ b/src/chat/chat_tests.rs @@ -2553,10 +2553,8 @@ async fn test_broadcast() -> Result<()> { let sent_msg = alice.pop_sent_msg().await; let msg = bob.parse_msg(&sent_msg).await; assert!(msg.was_encrypted()); - assert!(msg - .get_header(HeaderDef::ChatGroupMemberTimestamps) - .is_none()); - assert!(msg.get_header(HeaderDef::AutocryptGossip).is_none()); + assert!(!msg.header_exists(HeaderDef::ChatGroupMemberTimestamps)); + assert!(!msg.header_exists(HeaderDef::AutocryptGossip)); let msg = bob.recv_msg(&sent_msg).await; assert_eq!(msg.get_text(), "ola!"); assert_eq!(msg.subject, "Broadcast list"); diff --git a/src/mimefactory/mimefactory_tests.rs b/src/mimefactory/mimefactory_tests.rs index fcd99bbd7..fb909840e 100644 --- a/src/mimefactory/mimefactory_tests.rs +++ b/src/mimefactory/mimefactory_tests.rs @@ -893,10 +893,7 @@ async fn test_dont_remove_self() -> Result<()> { let mime_message = MimeMessage::from_bytes(alice, sent.payload.as_bytes(), None) .await .unwrap(); - assert_eq!( - mime_message.get_header(HeaderDef::ChatGroupPastMembers), - None - ); + assert!(!mime_message.header_exists(HeaderDef::ChatGroupPastMembers)); assert_eq!( mime_message.chat_group_member_timestamps().unwrap().len(), 1 // There is a timestamp for Bob, not for Alice diff --git a/src/mimeparser.rs b/src/mimeparser.rs index b44166b48..980ba2414 100644 --- a/src/mimeparser.rs +++ b/src/mimeparser.rs @@ -57,6 +57,10 @@ pub(crate) struct MimeMessage { /// Message headers. headers: HashMap, + #[cfg(test)] + /// Names of removed (ignored) headers. Used by `header_exists()` needed for tests. + headers_removed: HashSet, + /// List of addresses from the `To` and `Cc` headers. /// /// Addresses are normalized and lowercase. @@ -236,6 +240,7 @@ impl MimeMessage { let mut hop_info = parse_receive_headers(&mail.get_headers()); let mut headers = Default::default(); + let mut headers_removed = HashSet::::new(); let mut recipients = Default::default(); let mut past_members = Default::default(); let mut from = Default::default(); @@ -253,7 +258,12 @@ impl MimeMessage { &mut chat_disposition_notification_to, &mail.headers, ); - headers.retain(|k, _| !is_hidden(k)); + headers.retain(|k, _| { + !is_hidden(k) || { + headers_removed.insert(k.clone()); + false + } + }); // Parse hidden headers. let mimetype = mail.ctype.mimetype.parse::()?; @@ -298,9 +308,11 @@ impl MimeMessage { // Overwrite Message-ID with X-Microsoft-Original-Message-ID. // However if we later find Message-ID in the protected part, // it will overwrite both. - if let Some(microsoft_message_id) = - headers.remove(HeaderDef::XMicrosoftOriginalMessageId.get_headername()) - { + if let Some(microsoft_message_id) = remove_header( + &mut headers, + HeaderDef::XMicrosoftOriginalMessageId.get_headername(), + &mut headers_removed, + ) { headers.insert( HeaderDef::MessageId.get_headername().to_string(), microsoft_message_id, @@ -309,7 +321,7 @@ impl MimeMessage { // Remove headers that are allowed _only_ in the encrypted+signed part. It's ok to leave // them in signed-only emails, but has no value currently. - Self::remove_secured_headers(&mut headers); + Self::remove_secured_headers(&mut headers, &mut headers_removed); let mut from = from.context("No from in message")?; let private_keyring = load_self_secret_keyring(context).await?; @@ -442,7 +454,7 @@ impl MimeMessage { HeaderDef::ChatEdit, HeaderDef::ChatUserAvatar, ] { - headers.remove(h.get_headername()); + remove_header(&mut headers, h.get_headername(), &mut headers_removed); } } @@ -506,7 +518,7 @@ impl MimeMessage { } } if signatures.is_empty() { - Self::remove_secured_headers(&mut headers); + Self::remove_secured_headers(&mut headers, &mut headers_removed); // If it is not a read receipt, degrade encryption. if let (Some(peerstate), Ok(mail)) = (&mut peerstate, mail) { @@ -530,6 +542,9 @@ impl MimeMessage { let mut parser = MimeMessage { parts: Vec::new(), headers, + #[cfg(test)] + headers_removed, + recipients, past_members, list_post, @@ -931,6 +946,16 @@ impl MimeMessage { .map(|s| s.as_str()) } + #[cfg(test)] + /// Returns whether the header exists in any part of the parsed message. + /// + /// Use this to check for header absense. Header presense should be checked using + /// `get_header(...).is_some()` as it also checks that the header isn't ignored. + pub(crate) fn header_exists(&self, headerdef: HeaderDef) -> bool { + let hname = headerdef.get_headername(); + self.headers.contains_key(hname) || self.headers_removed.contains(hname) + } + /// Returns `Chat-Group-ID` header value if it is a valid group ID. pub fn get_chat_group_id(&self) -> Option<&str> { self.get_header(HeaderDef::ChatGroupId) @@ -1526,14 +1551,17 @@ impl MimeMessage { .and_then(|msgid| parse_message_id(msgid).ok()) } - fn remove_secured_headers(headers: &mut HashMap) { - headers.remove("secure-join-fingerprint"); - headers.remove("secure-join-auth"); - headers.remove("chat-verified"); - headers.remove("autocrypt-gossip"); + fn remove_secured_headers( + headers: &mut HashMap, + removed: &mut HashSet, + ) { + remove_header(headers, "secure-join-fingerprint", removed); + remove_header(headers, "secure-join-auth", removed); + remove_header(headers, "chat-verified", removed); + remove_header(headers, "autocrypt-gossip", removed); // Secure-Join is secured unless it is an initial "vc-request"/"vg-request". - if let Some(secure_join) = headers.remove("secure-join") { + if let Some(secure_join) = remove_header(headers, "secure-join", removed) { if secure_join == "vc-request" || secure_join == "vg-request" { headers.insert("secure-join".to_string(), secure_join); } @@ -1861,6 +1889,19 @@ impl MimeMessage { } } +fn remove_header( + headers: &mut HashMap, + key: &str, + removed: &mut HashSet, +) -> Option { + if let Some((k, v)) = headers.remove_entry(key) { + removed.insert(k); + Some(v) + } else { + None + } +} + /// Parses `Autocrypt-Gossip` headers from the email and applies them to peerstates. /// Params: /// from: The address which sent the message currently being parsed diff --git a/src/securejoin/securejoin_tests.rs b/src/securejoin/securejoin_tests.rs index 3bfee4f34..e67174049 100644 --- a/src/securejoin/securejoin_tests.rs +++ b/src/securejoin/securejoin_tests.rs @@ -120,7 +120,7 @@ async fn test_setup_contact_ex(case: SetupContactCase) { assert!(!msg.was_encrypted()); assert_eq!(msg.get_header(HeaderDef::SecureJoin).unwrap(), "vc-request"); assert!(msg.get_header(HeaderDef::SecureJoinInvitenumber).is_some()); - assert!(msg.get_header(HeaderDef::AutoSubmitted).is_none()); + assert!(!msg.header_exists(HeaderDef::AutoSubmitted)); // Step 3: Alice receives vc-request, sends vc-auth-required alice.recv_msg_trash(&sent).await; @@ -523,7 +523,7 @@ async fn test_secure_join() -> Result<()> { assert!(!msg.was_encrypted()); assert_eq!(msg.get_header(HeaderDef::SecureJoin).unwrap(), "vg-request"); assert!(msg.get_header(HeaderDef::SecureJoinInvitenumber).is_some()); - assert!(msg.get_header(HeaderDef::AutoSubmitted).is_none()); + assert!(!msg.header_exists(HeaderDef::AutoSubmitted)); // Old Delta Chat core sent `Secure-Join-Group` header in `vg-request`, // but it was only used by Alice in `vg-request-with-auth`. @@ -531,7 +531,7 @@ async fn test_secure_join() -> Result<()> { // and it is deprecated. // Now `Secure-Join-Group` header // is only sent in `vg-request-with-auth` for compatibility. - assert!(msg.get_header(HeaderDef::SecureJoinGroup).is_none()); + assert!(!msg.header_exists(HeaderDef::SecureJoinGroup)); // Step 3: Alice receives vg-request, sends vg-auth-required alice.recv_msg_trash(&sent).await; @@ -606,7 +606,7 @@ async fn test_secure_join() -> Result<()> { // Formally this message is auto-submitted, but as the member addition is a result of an // explicit user action, the Auto-Submitted header shouldn't be present. Otherwise it would // be strange to have it in "member-added" messages of verified groups only. - assert!(msg.get_header(HeaderDef::AutoSubmitted).is_none()); + assert!(!msg.header_exists(HeaderDef::AutoSubmitted)); // This is a two-member group, but Alice must Autocrypt-gossip to her other devices. assert!(msg.get_header(HeaderDef::AutocryptGossip).is_some());