test: Check headers absense straightforwardly

In the `test` cfg, introduce `MimeMessage::headers_removed` hash set and `header_exists()` function
returning whether the header exists in any part of the parsed message. `get_header()` shouldn't be
used in tests for checking absense of headers because it returns `None` for removed ("ignored")
headers.
This commit is contained in:
iequidoo
2025-04-12 03:58:26 -03:00
committed by iequidoo
parent 0e3277bc5a
commit 49c300d2ac
4 changed files with 61 additions and 25 deletions

View File

@@ -2553,10 +2553,8 @@ async fn test_broadcast() -> Result<()> {
let sent_msg = alice.pop_sent_msg().await; let sent_msg = alice.pop_sent_msg().await;
let msg = bob.parse_msg(&sent_msg).await; let msg = bob.parse_msg(&sent_msg).await;
assert!(msg.was_encrypted()); assert!(msg.was_encrypted());
assert!(msg assert!(!msg.header_exists(HeaderDef::ChatGroupMemberTimestamps));
.get_header(HeaderDef::ChatGroupMemberTimestamps) assert!(!msg.header_exists(HeaderDef::AutocryptGossip));
.is_none());
assert!(msg.get_header(HeaderDef::AutocryptGossip).is_none());
let msg = bob.recv_msg(&sent_msg).await; let msg = bob.recv_msg(&sent_msg).await;
assert_eq!(msg.get_text(), "ola!"); assert_eq!(msg.get_text(), "ola!");
assert_eq!(msg.subject, "Broadcast list"); assert_eq!(msg.subject, "Broadcast list");

View File

@@ -893,10 +893,7 @@ async fn test_dont_remove_self() -> Result<()> {
let mime_message = MimeMessage::from_bytes(alice, sent.payload.as_bytes(), None) let mime_message = MimeMessage::from_bytes(alice, sent.payload.as_bytes(), None)
.await .await
.unwrap(); .unwrap();
assert_eq!( assert!(!mime_message.header_exists(HeaderDef::ChatGroupPastMembers));
mime_message.get_header(HeaderDef::ChatGroupPastMembers),
None
);
assert_eq!( assert_eq!(
mime_message.chat_group_member_timestamps().unwrap().len(), mime_message.chat_group_member_timestamps().unwrap().len(),
1 // There is a timestamp for Bob, not for Alice 1 // There is a timestamp for Bob, not for Alice

View File

@@ -57,6 +57,10 @@ pub(crate) struct MimeMessage {
/// Message headers. /// Message headers.
headers: HashMap<String, String>, headers: HashMap<String, String>,
#[cfg(test)]
/// Names of removed (ignored) headers. Used by `header_exists()` needed for tests.
headers_removed: HashSet<String>,
/// List of addresses from the `To` and `Cc` headers. /// List of addresses from the `To` and `Cc` headers.
/// ///
/// Addresses are normalized and lowercase. /// Addresses are normalized and lowercase.
@@ -236,6 +240,7 @@ impl MimeMessage {
let mut hop_info = parse_receive_headers(&mail.get_headers()); let mut hop_info = parse_receive_headers(&mail.get_headers());
let mut headers = Default::default(); let mut headers = Default::default();
let mut headers_removed = HashSet::<String>::new();
let mut recipients = Default::default(); let mut recipients = Default::default();
let mut past_members = Default::default(); let mut past_members = Default::default();
let mut from = Default::default(); let mut from = Default::default();
@@ -253,7 +258,12 @@ impl MimeMessage {
&mut chat_disposition_notification_to, &mut chat_disposition_notification_to,
&mail.headers, &mail.headers,
); );
headers.retain(|k, _| !is_hidden(k)); headers.retain(|k, _| {
!is_hidden(k) || {
headers_removed.insert(k.clone());
false
}
});
// Parse hidden headers. // Parse hidden headers.
let mimetype = mail.ctype.mimetype.parse::<Mime>()?; let mimetype = mail.ctype.mimetype.parse::<Mime>()?;
@@ -298,9 +308,11 @@ impl MimeMessage {
// Overwrite Message-ID with X-Microsoft-Original-Message-ID. // Overwrite Message-ID with X-Microsoft-Original-Message-ID.
// However if we later find Message-ID in the protected part, // However if we later find Message-ID in the protected part,
// it will overwrite both. // it will overwrite both.
if let Some(microsoft_message_id) = if let Some(microsoft_message_id) = remove_header(
headers.remove(HeaderDef::XMicrosoftOriginalMessageId.get_headername()) &mut headers,
{ HeaderDef::XMicrosoftOriginalMessageId.get_headername(),
&mut headers_removed,
) {
headers.insert( headers.insert(
HeaderDef::MessageId.get_headername().to_string(), HeaderDef::MessageId.get_headername().to_string(),
microsoft_message_id, 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 // 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. // 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 mut from = from.context("No from in message")?;
let private_keyring = load_self_secret_keyring(context).await?; let private_keyring = load_self_secret_keyring(context).await?;
@@ -442,7 +454,7 @@ impl MimeMessage {
HeaderDef::ChatEdit, HeaderDef::ChatEdit,
HeaderDef::ChatUserAvatar, 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() { 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 it is not a read receipt, degrade encryption.
if let (Some(peerstate), Ok(mail)) = (&mut peerstate, mail) { if let (Some(peerstate), Ok(mail)) = (&mut peerstate, mail) {
@@ -530,6 +542,9 @@ impl MimeMessage {
let mut parser = MimeMessage { let mut parser = MimeMessage {
parts: Vec::new(), parts: Vec::new(),
headers, headers,
#[cfg(test)]
headers_removed,
recipients, recipients,
past_members, past_members,
list_post, list_post,
@@ -931,6 +946,16 @@ impl MimeMessage {
.map(|s| s.as_str()) .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. /// Returns `Chat-Group-ID` header value if it is a valid group ID.
pub fn get_chat_group_id(&self) -> Option<&str> { pub fn get_chat_group_id(&self) -> Option<&str> {
self.get_header(HeaderDef::ChatGroupId) self.get_header(HeaderDef::ChatGroupId)
@@ -1526,14 +1551,17 @@ impl MimeMessage {
.and_then(|msgid| parse_message_id(msgid).ok()) .and_then(|msgid| parse_message_id(msgid).ok())
} }
fn remove_secured_headers(headers: &mut HashMap<String, String>) { fn remove_secured_headers(
headers.remove("secure-join-fingerprint"); headers: &mut HashMap<String, String>,
headers.remove("secure-join-auth"); removed: &mut HashSet<String>,
headers.remove("chat-verified"); ) {
headers.remove("autocrypt-gossip"); 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". // 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" { if secure_join == "vc-request" || secure_join == "vg-request" {
headers.insert("secure-join".to_string(), secure_join); headers.insert("secure-join".to_string(), secure_join);
} }
@@ -1861,6 +1889,19 @@ impl MimeMessage {
} }
} }
fn remove_header(
headers: &mut HashMap<String, String>,
key: &str,
removed: &mut HashSet<String>,
) -> Option<String> {
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. /// Parses `Autocrypt-Gossip` headers from the email and applies them to peerstates.
/// Params: /// Params:
/// from: The address which sent the message currently being parsed /// from: The address which sent the message currently being parsed

View File

@@ -120,7 +120,7 @@ async fn test_setup_contact_ex(case: SetupContactCase) {
assert!(!msg.was_encrypted()); assert!(!msg.was_encrypted());
assert_eq!(msg.get_header(HeaderDef::SecureJoin).unwrap(), "vc-request"); assert_eq!(msg.get_header(HeaderDef::SecureJoin).unwrap(), "vc-request");
assert!(msg.get_header(HeaderDef::SecureJoinInvitenumber).is_some()); 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 // Step 3: Alice receives vc-request, sends vc-auth-required
alice.recv_msg_trash(&sent).await; alice.recv_msg_trash(&sent).await;
@@ -523,7 +523,7 @@ async fn test_secure_join() -> Result<()> {
assert!(!msg.was_encrypted()); assert!(!msg.was_encrypted());
assert_eq!(msg.get_header(HeaderDef::SecureJoin).unwrap(), "vg-request"); assert_eq!(msg.get_header(HeaderDef::SecureJoin).unwrap(), "vg-request");
assert!(msg.get_header(HeaderDef::SecureJoinInvitenumber).is_some()); 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`, // Old Delta Chat core sent `Secure-Join-Group` header in `vg-request`,
// but it was only used by Alice in `vg-request-with-auth`. // 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. // and it is deprecated.
// Now `Secure-Join-Group` header // Now `Secure-Join-Group` header
// is only sent in `vg-request-with-auth` for compatibility. // 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 // Step 3: Alice receives vg-request, sends vg-auth-required
alice.recv_msg_trash(&sent).await; 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 // 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 // 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. // 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. // This is a two-member group, but Alice must Autocrypt-gossip to her other devices.
assert!(msg.get_header(HeaderDef::AutocryptGossip).is_some()); assert!(msg.get_header(HeaderDef::AutocryptGossip).is_some());