Skip to content

Commit 56c605f

Browse files
iequidooiequidoo
committed
feat: imap: Don't prefetch Chat-Version; try to find out message encryption state instead
Instead, prefetch Secure-Join, Content-Type and Subject headers, try to find out if the message is encrypted, i.e.: - if its Content-Type is "multipart/encrypted" - or Subject is "..." or "[...]" as some MUAs use "multipart/mixed"; we can't only look at Subject as it's not mandatory; and depending on this decide on the target folder and whether the message should be downloaded. There's no much sense in downloading unencrypted "Chat-Version"-containing messages if `ShowEmails` is `Off` or `AcceptedContacts`, unencrypted Delta Chat messages should be considered as usual emails, there's even the "New E-Mail" feature in UIs nowadays which sends such messages. Don't prefetch Auto-Submitted as well, this becomes unnecessary. Changed behavior: before, "Chat-Version"-containing messages were moved from INBOX to DeltaChat, now such encrypted messages may remain in INBOX -- if there's no parent message or it's not `MessengerMessage`. Don't unconditionally move encrypted messages yet because the account may be shared with other software which doesn't and shouldn't look into the DeltaChat folder.
1 parent 2e9fd1c commit 56c605f

File tree

4 files changed

+65
-70
lines changed

4 files changed

+65
-70
lines changed

‎src/imap.rs‎

Lines changed: 32 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1964,21 +1964,24 @@ impl Session {
19641964
}
19651965
}
19661966

1967+
fn is_encrypted(headers: &[mailparse::MailHeader<'_>]) -> bool {
1968+
let content_type = headers.get_header_value(HeaderDef::ContentType);
1969+
let content_type = content_type.as_ref();
1970+
let res = content_type.is_some_and(|v| v.contains("multipart/encrypted"));
1971+
// Some MUAs use "multipart/mixed", look also at Subject in this case. We can't only look at
1972+
// Subject as it's not mandatory (<https://datatracker.ietf.org/doc/html/rfc5322#section-3.6>)
1973+
// and may be user-formed.
1974+
res || content_type.is_some_and(|v| v.contains("multipart/mixed"))
1975+
&& headers
1976+
.get_header_value(HeaderDef::Subject)
1977+
.is_some_and(|v| v == "..." || v == "[...]")
1978+
}
1979+
19671980
async fn should_move_out_of_spam(
19681981
context: &Context,
19691982
headers: &[mailparse::MailHeader<'_>],
19701983
) -> Result<bool> {
1971-
if headers.get_header_value(HeaderDef::ChatVersion).is_some() {
1972-
// If this is a chat message (i.e. has a ChatVersion header), then this might be
1973-
// a securejoin message. We can't find out at this point as we didn't prefetch
1974-
// the SecureJoin header. So, we always move chat messages out of Spam.
1975-
// Two possibilities to change this would be:
1976-
// 1. Remove the `&& !context.is_spam_folder(folder).await?` check from
1977-
// `fetch_new_messages()`, and then let `receive_imf()` check
1978-
// if it's a spam message and should be hidden.
1979-
// 2. Or add a flag to the ChatVersion header that this is a securejoin
1980-
// request, and return `true` here only if the message has this flag.
1981-
// `receive_imf()` can then check if the securejoin request is valid.
1984+
if headers.get_header_value(HeaderDef::SecureJoin).is_some() || is_encrypted(headers) {
19821985
return Ok(true);
19831986
}
19841987

@@ -2037,7 +2040,8 @@ async fn spam_target_folder_cfg(
20372040
return Ok(None);
20382041
}
20392042

2040-
if needs_move_to_mvbox(context, headers).await?
2043+
if is_encrypted(headers) && context.get_config_bool(Config::MvboxMove).await?
2044+
|| needs_move_to_mvbox(context, headers).await?
20412045
// If OnlyFetchMvbox is set, we don't want to move the message to
20422046
// the inbox where we wouldn't fetch it again:
20432047
|| context.get_config_bool(Config::OnlyFetchMvbox).await?
@@ -2090,20 +2094,6 @@ async fn needs_move_to_mvbox(
20902094
context: &Context,
20912095
headers: &[mailparse::MailHeader<'_>],
20922096
) -> Result<bool> {
2093-
let has_chat_version = headers.get_header_value(HeaderDef::ChatVersion).is_some();
2094-
if !context.get_config_bool(Config::IsChatmail).await?
2095-
&& has_chat_version
2096-
&& headers
2097-
.get_header_value(HeaderDef::AutoSubmitted)
2098-
.filter(|val| val.eq_ignore_ascii_case("auto-generated"))
2099-
.is_some()
2100-
{
2101-
if let Some(from) = mimeparser::get_from(headers) {
2102-
if context.is_self_addr(&from.addr).await? {
2103-
return Ok(true);
2104-
}
2105-
}
2106-
}
21072097
if !context.get_config_bool(Config::MvboxMove).await? {
21082098
return Ok(false);
21092099
}
@@ -2117,7 +2107,7 @@ async fn needs_move_to_mvbox(
21172107
return Ok(false);
21182108
}
21192109

2120-
if has_chat_version {
2110+
if headers.get_header_value(HeaderDef::SecureJoin).is_some() {
21212111
Ok(true)
21222112
} else if let Some(parent) = get_prefetch_parent_message(context, headers).await? {
21232113
match parent.is_dc_message {
@@ -2309,27 +2299,24 @@ pub(crate) async fn prefetch_should_download(
23092299
return Ok(false);
23102300
}
23112301

2312-
let is_chat_message = headers.get_header_value(HeaderDef::ChatVersion).is_some();
23132302
let accepted_contact = origin.is_known();
2314-
let is_reply_to_chat_message = get_prefetch_parent_message(context, headers)
2315-
.await?
2316-
.map(|parent| match parent.is_dc_message {
2317-
MessengerMessage::No => false,
2318-
MessengerMessage::Yes | MessengerMessage::Reply => true,
2319-
})
2320-
.unwrap_or_default();
2321-
2322-
let show_emails =
2323-
ShowEmails::from_i32(context.get_config_int(Config::ShowEmails).await?).unwrap_or_default();
2324-
23252303
let show = is_autocrypt_setup_message
2326-
|| match show_emails {
2327-
ShowEmails::Off => is_chat_message || is_reply_to_chat_message,
2328-
ShowEmails::AcceptedContacts => {
2329-
is_chat_message || is_reply_to_chat_message || accepted_contact
2330-
}
2304+
|| headers.get_header_value(HeaderDef::SecureJoin).is_some()
2305+
|| is_encrypted(headers)
2306+
|| match ShowEmails::from_i32(context.get_config_int(Config::ShowEmails).await?)
2307+
.unwrap_or_default()
2308+
{
2309+
ShowEmails::Off => false,
2310+
ShowEmails::AcceptedContacts => accepted_contact,
23312311
ShowEmails::All => true,
2332-
};
2312+
}
2313+
|| get_prefetch_parent_message(context, headers)
2314+
.await?
2315+
.map(|parent| match parent.is_dc_message {
2316+
MessengerMessage::No => false,
2317+
MessengerMessage::Yes | MessengerMessage::Reply => true,
2318+
})
2319+
.unwrap_or_default();
23332320

23342321
let should_download = (show && !blocked_contact) || maybe_ndn;
23352322
Ok(should_download)

‎src/imap/imap_tests.rs‎

Lines changed: 25 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -94,14 +94,14 @@ fn test_build_sequence_sets() {
9494
async fn check_target_folder_combination(
9595
folder: &str,
9696
mvbox_move: bool,
97-
chat_msg: bool,
97+
is_encrypted: bool,
9898
expected_destination: &str,
9999
accepted_chat: bool,
100100
outgoing: bool,
101101
setupmessage: bool,
102102
) -> Result<()> {
103103
println!(
104-
"Testing: For folder {folder}, mvbox_move {mvbox_move}, chat_msg {chat_msg}, accepted {accepted_chat}, outgoing {outgoing}, setupmessage {setupmessage}"
104+
"Testing: For folder {folder}, mvbox_move {mvbox_move}, is_encrypted {is_encrypted}, accepted {accepted_chat}, outgoing {outgoing}, setupmessage {setupmessage}"
105105
);
106106

107107
let t = TestContext::new_alice().await;
@@ -124,7 +124,6 @@ async fn check_target_folder_combination(
124124
temp = format!(
125125
"Received: (Postfix, from userid 1000); Mon, 4 Dec 2006 14:51:39 +0100 (CET)\n\
126126
{}\
127-
Subject: foo\n\
128127
Message-ID: <abc@example.com>\n\
129128
{}\
130129
Date: Sun, 22 Mar 2020 22:37:57 +0000\n\
@@ -135,7 +134,12 @@ async fn check_target_folder_combination(
135134
} else {
136135
"From: bob@example.net\nTo: alice@example.org\n"
137136
},
138-
if chat_msg { "Chat-Version: 1.0\n" } else { "" },
137+
if is_encrypted {
138+
"Subject: [...]\n\
139+
Content-Type: multipart/mixed; boundary=\"someboundary\"\n"
140+
} else {
141+
"Subject: foo\n"
142+
},
139143
);
140144
temp.as_bytes()
141145
};
@@ -157,30 +161,31 @@ async fn check_target_folder_combination(
157161
assert_eq!(
158162
expected,
159163
actual.as_deref(),
160-
"For folder {folder}, mvbox_move {mvbox_move}, chat_msg {chat_msg}, accepted {accepted_chat}, outgoing {outgoing}, setupmessage {setupmessage}: expected {expected:?}, got {actual:?}"
164+
"For folder {folder}, mvbox_move {mvbox_move}, is_encrypted {is_encrypted}, accepted {accepted_chat}, outgoing {outgoing}, setupmessage {setupmessage}: expected {expected:?}, got {actual:?}"
161165
);
162166
Ok(())
163167
}
164168

165-
// chat_msg means that the message was sent by Delta Chat
166-
// The tuples are (folder, mvbox_move, chat_msg, expected_destination)
169+
// The tuples are (folder, mvbox_move, is_encrypted, expected_destination)
167170
const COMBINATIONS_ACCEPTED_CHAT: &[(&str, bool, bool, &str)] = &[
168171
("INBOX", false, false, "INBOX"),
169172
("INBOX", false, true, "INBOX"),
170173
("INBOX", true, false, "INBOX"),
171-
("INBOX", true, true, "DeltaChat"),
172-
("Spam", false, false, "INBOX"), // Move classical emails in accepted chats from Spam to Inbox, not 100% sure on this, we could also just never move non-chat-msgs
174+
("INBOX", true, true, "INBOX"),
175+
("Spam", false, false, "INBOX"),
173176
("Spam", false, true, "INBOX"),
174-
("Spam", true, false, "INBOX"), // Move classical emails in accepted chats from Spam to Inbox, not 100% sure on this, we could also just never move non-chat-msgs
177+
// Move unencrypted emails in accepted chats from Spam to INBOX, not 100% sure on this, we could
178+
// also not move unencrypted emails or, if mvbox_move=1, move them to DeltaChat.
179+
("Spam", true, false, "INBOX"),
175180
("Spam", true, true, "DeltaChat"),
176181
];
177182

178-
// These are the same as above, but non-chat messages in Spam stay in Spam
183+
// These are the same as above, but unencrypted messages in Spam stay in Spam.
179184
const COMBINATIONS_REQUEST: &[(&str, bool, bool, &str)] = &[
180185
("INBOX", false, false, "INBOX"),
181186
("INBOX", false, true, "INBOX"),
182187
("INBOX", true, false, "INBOX"),
183-
("INBOX", true, true, "DeltaChat"),
188+
("INBOX", true, true, "INBOX"),
184189
("Spam", false, false, "Spam"),
185190
("Spam", false, true, "INBOX"),
186191
("Spam", true, false, "Spam"),
@@ -189,11 +194,11 @@ const COMBINATIONS_REQUEST: &[(&str, bool, bool, &str)] = &[
189194

190195
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
191196
async fn test_target_folder_incoming_accepted() -> Result<()> {
192-
for (folder, mvbox_move, chat_msg, expected_destination) in COMBINATIONS_ACCEPTED_CHAT {
197+
for (folder, mvbox_move, is_encrypted, expected_destination) in COMBINATIONS_ACCEPTED_CHAT {
193198
check_target_folder_combination(
194199
folder,
195200
*mvbox_move,
196-
*chat_msg,
201+
*is_encrypted,
197202
expected_destination,
198203
true,
199204
false,
@@ -206,11 +211,11 @@ async fn test_target_folder_incoming_accepted() -> Result<()> {
206211

207212
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
208213
async fn test_target_folder_incoming_request() -> Result<()> {
209-
for (folder, mvbox_move, chat_msg, expected_destination) in COMBINATIONS_REQUEST {
214+
for (folder, mvbox_move, is_encrypted, expected_destination) in COMBINATIONS_REQUEST {
210215
check_target_folder_combination(
211216
folder,
212217
*mvbox_move,
213-
*chat_msg,
218+
*is_encrypted,
214219
expected_destination,
215220
false,
216221
false,
@@ -224,11 +229,11 @@ async fn test_target_folder_incoming_request() -> Result<()> {
224229
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
225230
async fn test_target_folder_outgoing() -> Result<()> {
226231
// Test outgoing emails
227-
for (folder, mvbox_move, chat_msg, expected_destination) in COMBINATIONS_ACCEPTED_CHAT {
232+
for (folder, mvbox_move, is_encrypted, expected_destination) in COMBINATIONS_ACCEPTED_CHAT {
228233
check_target_folder_combination(
229234
folder,
230235
*mvbox_move,
231-
*chat_msg,
236+
*is_encrypted,
232237
expected_destination,
233238
true,
234239
true,
@@ -242,11 +247,11 @@ async fn test_target_folder_outgoing() -> Result<()> {
242247
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
243248
async fn test_target_folder_setupmsg() -> Result<()> {
244249
// Test setupmessages
245-
for (folder, mvbox_move, chat_msg, _expected_destination) in COMBINATIONS_ACCEPTED_CHAT {
250+
for (folder, mvbox_move, is_encrypted, _expected_destination) in COMBINATIONS_ACCEPTED_CHAT {
246251
check_target_folder_combination(
247252
folder,
248253
*mvbox_move,
249-
*chat_msg,
254+
*is_encrypted,
250255
if folder == &"Spam" { "INBOX" } else { folder }, // Never move setup messages, except if they are in "Spam"
251256
false,
252257
true,

‎src/imap/session.rs‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,17 +14,20 @@ use crate::tools;
1414
/// Prefetch:
1515
/// - Message-ID to check if we already have the message.
1616
/// - In-Reply-To and References to check if message is a reply to chat message.
17-
/// - Chat-Version to check if a message is a chat message
1817
/// - Autocrypt-Setup-Message to check if a message is an autocrypt setup message,
1918
/// not necessarily sent by Delta Chat.
19+
///
20+
/// NB: We don't look at Chat-Version as we don't want any "better" handling for unencrypted
21+
/// messages.
2022
const PREFETCH_FLAGS: &str = "(UID INTERNALDATE RFC822.SIZE BODY.PEEK[HEADER.FIELDS (\
2123
MESSAGE-ID \
2224
DATE \
2325
X-MICROSOFT-ORIGINAL-MESSAGE-ID \
2426
FROM \
2527
IN-REPLY-TO REFERENCES \
26-
CHAT-VERSION \
27-
AUTO-SUBMITTED \
28+
CONTENT-TYPE \
29+
SECURE-JOIN \
30+
SUBJECT \
2831
AUTOCRYPT-SETUP-MESSAGE\
2932
)])";
3033

‎src/receive_imf.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -997,7 +997,7 @@ pub(crate) async fn receive_imf_inner(
997997
if let Some(is_bot) = mime_parser.is_bot {
998998
// If the message is auto-generated and was generated by Delta Chat,
999999
// mark the contact as a bot.
1000-
if mime_parser.get_header(HeaderDef::ChatVersion).is_some() {
1000+
if mime_parser.has_chat_version() {
10011001
from_id.mark_bot(context, is_bot).await?;
10021002
}
10031003
}
@@ -2949,7 +2949,7 @@ async fn apply_group_changes(
29492949
}
29502950

29512951
// Allow non-Delta Chat MUAs to add members.
2952-
if mime_parser.get_header(HeaderDef::ChatVersion).is_none() {
2952+
if !mime_parser.has_chat_version() {
29532953
// Don't delete any members locally, but instead add absent ones to provide group
29542954
// membership consistency for all members:
29552955
new_members.extend(to_ids_flat.iter());

0 commit comments

Comments
 (0)