fix: Ignore protected headers in outer message part (#6357) #6370
Conversation
| 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 |
There was a problem hiding this comment.
I don't understand this comment, why is it important here that DKIM signature applies to last headers?
There was a problem hiding this comment.
At least test_take_last_header() breaks if i remove || is_protected(&key) here. For unprotected headers we just don't mind. Feel free to improve the comment
| @@ -1528,12 +1528,15 @@ impl MimeMessage { | |||
| chat_disposition_notification_to: &mut Option<SingleInfo>, | |||
| 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"); | |||
There was a problem hiding this comment.
Could you also extend a test test_is_bot with a non-chat message? It's important that non-DC unencrypted messages from auto-repliers have dc_msg_is_bot returning true. Seems to work because for such messages merge_headers is never called, but better test it explicitly.
There was a problem hiding this comment.
I tried, but this doesn't work, if it's a non-chat message, we don't mark a contact as bot and this looks intentional:
let is_bot = parser.headers.get("auto-submitted")
== Some(&"auto-generated".to_string())
&& parser.headers.contains_key("chat-version");Anyway i improved the test to record the current behaviour.
There was a problem hiding this comment.
I think it is a bug, message should be marked as a bot message, but the contact should not become a bot in this case. I created an issue #6373
There was a problem hiding this comment.
Removed the second commit because you're fixing this in #6374
iequidoo
force-pushed
the
iequidoo/outer-protected-hdrs
branch
from
3ff41b4 to
5445310
Compare
Delta Chat always adds protected headers to the inner encrypted or signed message, so if a protected header is only present in the outer part, it should be ignored because it's probably added by the server or somebody else. The exception is Subject because there are known cases when it's only present in the outer message part, e.g. an encrypted unsigned Thunderbird message. Also handle "Auto-Submitted" and "Autocrypt-Setup-Message" as protected headers on the receiver side, this was apparently forgotten. This may fix #6357 where Saved Messages (i.e. `ContactId::SELF`) and some contacts are unexpectedly marked as bots which can happen if e.g. the server adds "Auto-Submitted: auto-generated" to messages for some reason. Maybe sounds unlikely, but let's try.
iequidoo
force-pushed
the
iequidoo/outer-protected-hdrs
branch
from
5445310 to
92a4af4
Compare
Fix#6357. The true reason of the bug: #6357 (comment). Still this PR is useful