From 396ec131fc2c1d70fb71ad5e694cecd87c7ee8e0 Mon Sep 17 00:00:00 2001 From: Hocuri Date: Sun, 7 Feb 2021 23:37:04 +0100 Subject: [PATCH] Implement receiving mailing lists (#1964) * Copypaste-merge my old work * Start implementing mailinglists the new way * Create pseudo contact * Fine-tune docs * Remove some unnecessary changes * style * Make a stock str * Fix a crash. Yes, this line caused a panic when reconfiguring on Android (without a reasonable error log). Also, I could not receive any messages anymore. * rfmt * Add tests and make them pass * Even more tests * rfmt * Enhance test and fix bug * Don't update the sender name when prefetching because maybe it's a mailing list * Use an enum instead of numbers for the decision * Don't remove anyone from mailing lists * Fix bug in the regex * Adjust error msg * Compile error after rebase * Correctly emit event * Add dc_msg_is_mailing_list so that you can find out whether messages in the deaddrop belong to mailing lists. * Add received headers to unit tests * Comments, small tweaks * Use dc_msg_get_override_sender_name instead of dc_msg_get_sender_name * Add dc_msg_get_sender_first_name() because sometimes the first name was not correctly shown in mailing lists * small fixes, don't let the user modify mailing list groups * Hide contacts for addresses like noreply@github.com and reply+AEJ...@reply.github.com When testing mailing lists, I noticed that sometimes a mailing list contact got a name (like, hocuri ). It turned out that ages ago, I had accidentally written an email to - in this example - hocuri and it had been added to the contacts list. This hides email addresses from the contacts list that are obviously not meant to be written at and prevents updating the names. * Comment, clippy * Replace u32 with ChatId * Resolve lost of small issues from the reviews * remove dc_msg_get_sender_first_name * add dc_msg_get_real_chat_id() this allows to check if a contact request belongs to a mailing list and to show name/avatar/whatever of the mailinglist. another approach was to duplicate some chat-apis for messages (eg. dc_msg_is_mailing_list()) however that would require far more new apis. the idea to change the behavior of dc_msg_get_chat_id() would be more clean, however, that easily breaks existing implementations and can led to hard to find bugs. * remove now unused Message.is_mailing_list() * if a name for a mailing list is missing, use List-ID * fix comment * fix error message * document how dc_get_chat_contacts() works for mailing lists * refine decide api (#2185) * add DC_DECIDE* constants to deltachat.h, tweak documentation * use StartChat/Block/NotNow instead of Yes/Never/NotNow * decide_on_contact_request works on ctx/msg-id functions working on message-objects usually do not read or write directly on the database. therefore, decide_on_contact_request() should not work with message-objects as well, it is even a bit misleading, as eg. chat-id of the object is not modified. instead, the function works on context, similar to dc_send_msg(), dc_create_chat() and so on. for now, i moved it to context, could maybe be part od MsgId. * Update src/chatlist.rs Co-authored-by: Hocuri Co-authored-by: Hocuri * refine documentation * re-add accidentally deleted Param::MailingList * remove pseudo-contact in domain @mailing.list 1. the pseudo-contact was added to the mailing list contacts, which is not needed. might be that we want to add the recent contacts there in a subsequent pr (we do not know all contacts of a mailing list) 2. the pseudo-contact was used to block mailing lists; this is done by setting the chat to Blocked::Manually now 3- the pseudo-contact was used for unblocking; as it is very neat not to require additional ui for mailing list unblocking, might be that we introduce a similar pseudo-contact for this limited purpose in a subsequent pr, however, the pseudo-contact needs to exist only during unblocking then, maybe also the special domain is not needed, we'll see :) * Move dc_decide_on_contact_request() up to the dc_context section as it's a member of dc_context now More specifically, to the "handle messages" section * re-introduce Chattype::Mailinglist (#2195) * re-introduce Chattype::Mailinglist * exhaustive chattype-check in fetch_existing_msgs() and get_summary2() * exhaustive chattype-check in ndn_maybe_add_info_msg() * exhaustive chattype-check in message::fill() * remove dc_chat_is_mailing_list() from ffi Co-authored-by: B. Petersen --- deltachat-ffi/deltachat.h | 117 +++++++++- deltachat-ffi/src/lib.rs | 47 ++++ src/chat.rs | 19 +- src/chatlist.rs | 33 +-- src/constants.rs | 1 + src/contact.rs | 40 +++- src/dc_receive_imf.rs | 459 +++++++++++++++++++++++++++++++++----- src/imap/mod.rs | 13 +- src/job.rs | 23 +- src/message.rs | 222 +++++++++++++++--- src/mimeparser.rs | 24 +- src/param.rs | 4 + src/test_utils.rs | 23 +- 13 files changed, 879 insertions(+), 146 deletions(-) diff --git a/deltachat-ffi/deltachat.h b/deltachat-ffi/deltachat.h index ae2272dbb..56a3fe920 100644 --- a/deltachat-ffi/deltachat.h +++ b/deltachat-ffi/deltachat.h @@ -629,9 +629,8 @@ int dc_preconfigure_keypair (dc_context_t* context, const cha * messages from addresses that have no relationship to the configured account. * The last of these messages is represented by DC_CHAT_ID_DEADDROP and you can retrieve details * about it with dc_chatlist_get_msg_id(). Typically, the UI asks the user "Do you want to chat with NAME?" - * and offers the options "Yes" (call dc_create_chat_by_msg_id()), "Never" (call dc_block_contact()) - * or "Not now". - * The UI can also offer a "Close" button that calls dc_marknoticed_contact() then. + * and offers the options "Start chat", "Block" or "Not now". + * Call dc_decide_on_contact_request() when the user selected one of these options. * - DC_CHAT_ID_ARCHIVED_LINK (6) - this special chat is present if the user has * archived _any_ chat using dc_set_chat_visibility(). The UI should show a link as * "Show archived chats", if the user clicks this item, the UI should show a @@ -673,6 +672,8 @@ dc_chatlist_t* dc_get_chatlist (dc_context_t* context, int flags, // handle chats /** + * DEPRECATED Use dc_decide_on_contact_request(). + * * Create a normal chat or a group chat by a messages ID that comes typically * from the deaddrop, DC_CHAT_ID_DEADDROP (1). * @@ -692,6 +693,7 @@ dc_chatlist_t* dc_get_chatlist (dc_context_t* context, int flags, * same group may be shown or not - so, all in all, it is fine to show the * contact name only. * + * @deprecated Use dc_decide_on_contact_request() instead * @memberof dc_context_t * @param context The context object as returned from dc_context_new(). * @param msg_id The message ID to create the chat for. @@ -1087,7 +1089,7 @@ dc_array_t* dc_get_fresh_msgs (dc_context_t* context); * (IMAP/MDNs is not done for noticed messages). * * Calling this function usually results in the event #DC_EVENT_MSGS_NOTICED. - * See also dc_marknoticed_contact() and dc_markseen_msgs(). + * See also dc_markseen_msgs(). * * @memberof dc_context_t * @param context The context object as returned from dc_context_new(). @@ -1213,6 +1215,11 @@ void dc_delete_chat (dc_context_t* context, uint32_t ch * * - for the deaddrop, the list is empty * + * - for mailing lists, the behavior is not documented currently, we will decide on that later. + * for now, the UI should not show the list for mailing lists. + * (we do not know all members and there is not always a global mailing list address, + * so we could return only SELF or the known members; this is not decided yet) + * * @memberof dc_context_t * @param context The context object as returned from dc_context_new(). * @param chat_id Chat ID to get the belonging contact IDs for. @@ -1540,6 +1547,8 @@ void dc_forward_msgs (dc_context_t* context, const uint3 /** + * DEPRECATED + * * Mark all messages sent by the given contact as _noticed_. * This function is typically used to ignore a user in the deaddrop temporarily ("Not now" button). * @@ -1548,6 +1557,9 @@ void dc_forward_msgs (dc_context_t* context, const uint3 * * See also dc_marknoticed_chat() and dc_markseen_msgs() * + * @deprecated Use dc_decide_on_contact_request() if the user just hit "Not now" on a button in the deaddrop, + * dc_marknoticed_chat() if the user has entered a chat + * and dc_markseen_msgs() if the user actually _saw_ a message. * @memberof dc_context_t * @param context The context object. * @param contact_id The contact ID of which all messages should be marked as noticed. @@ -1559,7 +1571,7 @@ void dc_marknoticed_contact (dc_context_t* context, uint32_t co * Mark a message as _seen_, updates the IMAP state and * sends MDNs. If the message is not in a real chat (e.g. a contact request), the * message is only marked as NOTICED and no IMAP/MDNs is done. See also - * dc_marknoticed_chat() and dc_marknoticed_contact() + * dc_marknoticed_chat(). * * Moreover, if messages belong to a chat with ephemeral messages enabled, * the ephemeral timer is started for these messages. @@ -1589,6 +1601,53 @@ void dc_markseen_msgs (dc_context_t* context, const uint3 dc_msg_t* dc_get_msg (dc_context_t* context, uint32_t msg_id); +#define DC_DECISION_START_CHAT 0 +#define DC_DECISION_BLOCK 1 +#define DC_DECISION_NOT_NOW 2 + + +/** + * Call this when the user decided about a deaddrop message ("Do you want to chat with NAME?"). + * + * Possible decisions are: + * - DC_DECISION_START_CHAT (0) + * - This will create a new chat and return the chat id. + * - DC_DECISION_BLOCK (1) + * - This will block the sender. + * - When a new message from the sender arrives, + * that will not result in a new contact request. + * - The blocked sender will be returned by dc_get_blocked_contacts() + * typically, the UI offers an option to unblock senders from there. + * - DC_DECISION_NOT_NOW (2) + * - This will mark all messages from this sender as noticed. + * - That the contact request is removed from the chat list. + * - When a new message from the sender arrives, + * a new contact request with the new message will pop up in the chatlist. + * - The contact request stays available in the explicit deaddrop. + * - If the contact request is already noticed, nothing happens. + * + * If the message belongs to a mailing list, + * the function makes sure that all messages + * from the mailing list are blocked or marked as noticed. + * + * The user should be asked whether they want to chat with the _contact_ belonging to the message; + * the group names may be really weird when taken from the subject of implicit (= ad-hoc) + * groups and this may look confusing. Moreover, this function also scales up the origin of the contact. + * + * If the chat belongs to a mailing list, you can also ask + * "Would you like to read MAILING LIST NAME?" + * (use dc_msg_get_real_chat_id() to get the chat-id for the contact request + * and then dc_chat_is_mailing_list(), dc_chat_get_name() and so on) + * + * @memberof dc_context_t + * @param context The context object. + * @param msg_id ID of Message to decide on. + * @param decision One of the DC_DECISION_* values. + * @return The chat id of the created chat, if any. + */ +uint32_t dc_decide_on_contact_request (dc_context_t* context, uint32_t msg_id, int decision); + + // handle contacts /** @@ -2818,6 +2877,7 @@ char* dc_chat_get_info_json (dc_context_t* context, size_t chat #define DC_CHAT_TYPE_UNDEFINED 0 #define DC_CHAT_TYPE_SINGLE 100 #define DC_CHAT_TYPE_GROUP 120 +#define DC_CHAT_TYPE_MAILINGLIST 140 /** @@ -2858,6 +2918,11 @@ uint32_t dc_chat_get_id (const dc_chat_t* chat); * - DC_CHAT_TYPE_GROUP (120) - a group chat, chats_contacts contain all group * members, incl. DC_CONTACT_ID_SELF * + * - DC_CHAT_TYPE_MAILINGLIST (140) - a mailing list, this is similar to groups, + * however, the member list cannot be retrieved completely + * and cannot be changed using this api. + * moreover, for now, mailist lists are read-only. + * * @memberof dc_chat_t * @param chat The chat object. * @return Chat type. @@ -3120,6 +3185,7 @@ uint32_t dc_msg_get_from_id (const dc_msg_t* msg); * To get details about the chat, pass the returned ID to dc_get_chat(). * If a message is still in the deaddrop, the ID DC_CHAT_ID_DEADDROP is returned * although internally another ID is used. + * (to get that internal id, use dc_msg_get_real_chat_id()) * * @memberof dc_msg_t * @param msg The message object. @@ -3128,6 +3194,19 @@ uint32_t dc_msg_get_from_id (const dc_msg_t* msg); uint32_t dc_msg_get_chat_id (const dc_msg_t* msg); +/** + * Get the ID of chat the message belongs to. + * To get details about the chat, pass the returned ID to dc_get_chat(). + * In contrast to dc_msg_get_chat_id(), this function returns the chat-id also + * for messages in the deaddrop. + * + * @memberof dc_msg_t + * @param msg The message object. + * @return The ID of the chat the message belongs to, 0 on errors. + */ +uint32_t dc_msg_get_real_chat_id (const dc_msg_t* msg); + + /** * Get the type of the message. * @@ -3144,7 +3223,7 @@ int dc_msg_get_viewtype (const dc_msg_t* msg); * * Incoming message states: * - DC_STATE_IN_FRESH (10) - Incoming _fresh_ message. Fresh messages are neither noticed nor seen and are typically shown in notifications. Use dc_get_fresh_msgs() to get all fresh messages. - * - DC_STATE_IN_NOTICED (13) - Incoming _noticed_ message. E.g. chat opened but message not yet read - noticed messages are not counted as unread but did not marked as read nor resulted in MDNs. Use dc_marknoticed_chat() or dc_marknoticed_contact() to mark messages as being noticed. + * - DC_STATE_IN_NOTICED (13) - Incoming _noticed_ message. E.g. chat opened but message not yet read - noticed messages are not counted as unread but were not marked as read nor resulted in MDNs. Use dc_marknoticed_chat() to mark messages as being noticed. * - DC_STATE_IN_SEEN (16) - Incoming message, really _seen_ by the user. Marked as read on IMAP and MDN may be sent. Use dc_markseen_msgs() to mark messages as being seen. * * Outgoing message states: @@ -3426,6 +3505,24 @@ dc_lot_t* dc_msg_get_summary (const dc_msg_t* msg, const dc_cha char* dc_msg_get_summarytext (const dc_msg_t* msg, int approx_characters); +/** + * Get the name that should be shown over the message (in a group chat) instead of the contact + * display name. + * + * In mailing lists, sender display name and sender address do not always belong together. + * In this case, this function gives you the name that should actually be shown over the message. + * + * @memberof dc_msg_t + * @param msg The message object. + * @return the name to show over this message or NULL. + * If this returns NULL, call `dc_contact_get_display_name()`. + * The returned string must be released using dc_str_unref(). + * Returns an empty string on errors, never returns NULL. + */ +char* dc_msg_get_override_sender_name(const dc_msg_t* msg); + + + /** * Check if a message has a deviating timestamp. * A message has a deviating timestamp @@ -3595,7 +3692,8 @@ char* dc_msg_get_setupcodebegin (const dc_msg_t* msg); * If the message is no videochat invitation, NULL is returned. * Must be released using dc_str_unref() when done. */ -char* dc_msg_get_videochat_url (const dc_msg_t* msg); +char* dc_msg_get_videochat_url (const dc_msg_t* msg); + /** * Gets the error status of the message. @@ -3938,6 +4036,9 @@ char* dc_contact_get_name (const dc_contact_t* contact); * This name is typically used in lists. * To get the name editable in a formular, use dc_contact_get_name(). * + * In a group, you should show the sender's name over a message. To get it, call dc_msg_get_override_sender_name() + * first and if it returns NULL, call dc_contact_get_display_name(). + * * @memberof dc_contact_t * @param contact The contact object. * @return String with the name to display, must be released using dc_str_unref(). @@ -4741,7 +4842,7 @@ void dc_event_unref(dc_event_t* event); * Messages were marked noticed or seen. * The UI may update badge counters or stop showing a chatlist-item with a bold font. * - * This event is emitted e.g. when calling dc_markseen_msgs(), dc_marknoticed_chat() or dc_marknoticed_contact() + * This event is emitted e.g. when calling dc_markseen_msgs() or dc_marknoticed_chat() * or when a chat is answered on another device. * Do not try to derive the state of an item from just the fact you received the event; * use e.g. dc_msg_get_state() or dc_get_fresh_msg_cnt() for this purpose. diff --git a/deltachat-ffi/src/lib.rs b/deltachat-ffi/src/lib.rs index 62ca694cf..3930e013c 100644 --- a/deltachat-ffi/src/lib.rs +++ b/deltachat-ffi/src/lib.rs @@ -2591,6 +2591,16 @@ pub unsafe extern "C" fn dc_msg_get_chat_id(msg: *mut dc_msg_t) -> u32 { ffi_msg.message.get_chat_id().to_u32() } +#[no_mangle] +pub unsafe extern "C" fn dc_msg_get_real_chat_id(msg: *mut dc_msg_t) -> u32 { + if msg.is_null() { + eprintln!("ignoring careless call to dc_msg_get_real_chat_id()"); + return 0; + } + let ffi_msg = &*msg; + ffi_msg.message.get_real_chat_id().to_u32() +} + #[no_mangle] pub unsafe extern "C" fn dc_msg_get_viewtype(msg: *mut dc_msg_t) -> libc::c_int { if msg.is_null() { @@ -2810,6 +2820,17 @@ pub unsafe extern "C" fn dc_msg_get_summarytext( .strdup() } +#[no_mangle] +pub unsafe extern "C" fn dc_msg_get_override_sender_name(msg: *mut dc_msg_t) -> *mut libc::c_char { + if msg.is_null() { + eprintln!("ignoring careless call to dc_msg_get_override_sender_name()"); + return "".strdup(); + } + let ffi_msg = &mut *msg; + + ffi_msg.message.get_override_sender_name().strdup() +} + #[no_mangle] pub unsafe extern "C" fn dc_msg_has_deviating_timestamp(msg: *mut dc_msg_t) -> libc::c_int { if msg.is_null() { @@ -2915,6 +2936,32 @@ pub unsafe extern "C" fn dc_msg_get_videochat_url(msg: *mut dc_msg_t) -> *mut li .strdup() } +#[no_mangle] +pub unsafe extern "C" fn dc_decide_on_contact_request( + context: *mut dc_context_t, + msg_id: u32, + decision: libc::c_int, +) -> u32 { + if context.is_null() || msg_id <= constants::DC_MSG_ID_LAST_SPECIAL as u32 { + eprintln!("ignoring careless call to dc_decide_on_contact_request()"); + } + let ctx = &*context; + + match from_prim(decision) { + None => { + warn!(ctx, "{} is not a valid decision, ignoring", decision); + 0 + } + Some(d) => block_on(message::decide_on_contact_request( + ctx, + MsgId::new(msg_id), + d, + )) + .unwrap_or_default() + .to_u32(), + } +} + #[no_mangle] pub unsafe extern "C" fn dc_msg_get_videochat_type(msg: *mut dc_msg_t) -> libc::c_int { if msg.is_null() { diff --git a/src/chat.rs b/src/chat.rs index daacc2736..bb8fbbd49 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -221,6 +221,7 @@ impl ChatId { } } } + Chattype::Mailinglist => bail!("Cannot protect mailing lists"), Chattype::Undefined => bail!("Undefined group type"), }, ProtectionStatus::Unprotected => {} @@ -828,9 +829,13 @@ impl Chat { self.param.exists(Param::Devicetalk) } + pub fn is_mailing_list(&self) -> bool { + self.typ == Chattype::Mailinglist + } + /// Returns true if user can send messages to this chat. pub fn can_send(&self) -> bool { - !self.id.is_special() && !self.is_device_talk() + !self.id.is_special() && !self.is_device_talk() && !self.is_mailing_list() } pub async fn update_param(&mut self, context: &Context) -> Result<(), Error> { @@ -1321,7 +1326,11 @@ pub async fn create_by_msg_id(context: &Context, msg_id: MsgId) -> Result { + let lastcontact = + Contact::load_from_db(context, lastmsg.from_id).await.ok(); + (Some(lastmsg), lastcontact) + } + Chattype::Single | Chattype::Undefined => (Some(lastmsg), None), + } + } + } else { + (None, None) + }; if chat.id.is_archived_link() { ret.text2 = None; diff --git a/src/constants.rs b/src/constants.rs index 3fd7ae8ac..b60f2b418 100644 --- a/src/constants.rs +++ b/src/constants.rs @@ -144,6 +144,7 @@ pub enum Chattype { Undefined = 0, Single = 100, Group = 120, + Mailinglist = 140, } impl Default for Chattype { diff --git a/src/contact.rs b/src/contact.rs index 55d581727..75c6daa3b 100644 --- a/src/contact.rs +++ b/src/contact.rs @@ -81,6 +81,9 @@ pub struct Contact { pub enum Origin { Unknown = 0, + /// Hidden on purpose, e.g. addresses with the word "noreply" in it + Hidden = 0x8, + /// From: of incoming messages of unknown sender IncomingUnknownFrom = 0x10, @@ -344,7 +347,7 @@ impl Contact { context: &Context, name: impl AsRef, addr: impl AsRef, - origin: Origin, + mut origin: Origin, ) -> Result<(u32, Modifier)> { let mut sth_modified = Modifier::None; @@ -378,6 +381,25 @@ impl Contact { bail!("Bad address supplied: {:?}", addr); } + let mut name = name.as_ref(); + #[allow(clippy::collapsible_if)] + if origin <= Origin::OutgoingTo { + // The user may accidentally have written to a "noreply" address with another MUA: + if addr.contains("noreply") + || addr.contains("no-reply") + || addr.starts_with("notifications@") + // Filter out use-once addresses (like reply+AEJDGPOECLAP...@reply.github.com): + || (addr.len() > 50 && addr.contains('+')) + { + info!(context, "hiding contact {}", addr); + origin = Origin::Hidden; + // For these kind of email addresses, sender and address often don't belong together + // (like hocuri ). In this example, hocuri shouldn't + // be saved as the displayname for notifications@github.com. + name = ""; + } + } + let mut update_addr = false; let mut update_name = false; let mut update_authname = false; @@ -393,17 +415,17 @@ impl Contact { let row_origin: Origin = row.get(3)?; let row_authname: String = row.get(4)?; - if !name.as_ref().is_empty() { + if !name.is_empty() { if !row_name.is_empty() { if (origin >= row_origin || row_name == row_authname) - && name.as_ref() != row_name + && name != row_name { update_name = true; } } else { update_name = true; } - if origin == Origin::IncomingUnknownFrom && name.as_ref() != row_authname { + if origin == Origin::IncomingUnknownFrom && name != row_authname { update_authname = true; } } else if origin == Origin::ManuallyCreated && !row_authname.is_empty() { @@ -421,8 +443,8 @@ impl Contact { } if update_name || update_authname || update_addr || origin > row_origin { let new_name = if update_name { - if !name.as_ref().is_empty() { - name.as_ref().to_string() + if !name.is_empty() { + name.to_string() } else { row_authname.clone() } @@ -443,7 +465,7 @@ impl Contact { row_origin }, if update_authname { - name.as_ref().to_string() + name.to_string() } else { row_authname }, @@ -483,10 +505,10 @@ impl Contact { .execute( "INSERT INTO contacts (name, addr, origin, authname) VALUES(?, ?, ?, ?);", paramsv![ - name.as_ref().to_string(), + name.to_string(), addr, origin, - if update_authname { name.as_ref().to_string() } else { "".to_string() } + if update_authname { name.to_string() } else { "".to_string() } ], ) .await diff --git a/src/dc_receive_imf.rs b/src/dc_receive_imf.rs index 2fe7a2bd2..a0e682a34 100644 --- a/src/dc_receive_imf.rs +++ b/src/dc_receive_imf.rs @@ -2,6 +2,8 @@ use anyhow::{bail, ensure, format_err, Result}; use itertools::join; use mailparse::SingleInfo; use num_traits::FromPrimitive; +use once_cell::sync::Lazy; +use regex::Regex; use sha2::{Digest, Sha256}; use crate::chat::{self, Chat, ChatId, ProtectionStatus}; @@ -101,6 +103,8 @@ pub(crate) async fn dc_receive_imf_inner( let mut created_db_entries = Vec::new(); let mut create_event_to_send = Some(CreateEvent::MsgsChanged); + let list_id_header: Option<&String> = mime_parser.get(HeaderDef::ListId); + // helper method to handle early exit and memory cleanup let cleanup = |context: &Context, create_event_to_send: &Option, @@ -126,8 +130,11 @@ pub(crate) async fn dc_receive_imf_inner( // or if From: is equal to SELF (in this case, it is any outgoing messages, // we do not check Return-Path any more as this is unreliable, see // https://github.com/deltachat/deltachat-core/issues/150) + // + // If this is a mailing list email (i.e. list_id_header is some), don't change the displayname because in + // a mailing list the sender displayname sometimes does not belong to the sender email address. let (from_id, _from_id_blocked, incoming_origin) = - from_field_to_contact_id(context, &mime_parser.from).await?; + from_field_to_contact_id(context, &mime_parser.from, list_id_header.is_some()).await?; let incoming = from_id != DC_CONTACT_ID_SELF; @@ -144,6 +151,7 @@ pub(crate) async fn dc_receive_imf_inner( } else { Origin::IncomingUnknownTo }, + list_id_header.is_some(), ) .await?, ); @@ -290,14 +298,18 @@ pub(crate) async fn dc_receive_imf_inner( /// Converts "From" field to contact id. /// /// Also returns whether it is blocked or not and its origin. +/// +/// * `prevent_rename`: passed through to `dc_add_or_lookup_contacts_by_address_list()` pub async fn from_field_to_contact_id( context: &Context, from_address_list: &[SingleInfo], + prevent_rename: bool, ) -> Result<(u32, bool, Origin)> { let from_ids = dc_add_or_lookup_contacts_by_address_list( context, from_address_list, Origin::IncomingUnknownFrom, + prevent_rename, ) .await?; @@ -494,9 +506,27 @@ async fn add_parts( if chat_id.is_unset() { // check if the message belongs to a mailing list - if mime_parser.is_mailinglist_message() { - *chat_id = ChatId::new(DC_CHAT_ID_TRASH); - info!(context, "Message belongs to a mailing list (TRASH)"); + if let Some(list_id_header) = mime_parser.get(HeaderDef::ListId) { + let create_blocked = Blocked::Deaddrop; + + let (new_chat_id, new_chat_id_blocked) = create_or_lookup_mailinglist( + context, + allow_creation, + create_blocked, + list_id_header, + &mime_parser.get_subject().unwrap_or_default(), + ) + .await; + + *chat_id = new_chat_id; + chat_id_blocked = new_chat_id_blocked; + if let Some(from) = mime_parser.from.first() { + if let Some(from_name) = &from.display_name { + for part in mime_parser.parts.iter_mut() { + part.param.set(Param::OverrideSenderDisplayname, from_name); + } + } + } } } @@ -1309,14 +1339,15 @@ async fn create_or_lookup_group( return Ok((ChatId::new(0), Blocked::Not)); } - chat_id = create_group_record( + chat_id = create_multiuser_record( context, + Chattype::Group, &grpid, grpname.as_ref().unwrap(), create_blocked, create_protected, ) - .await; + .await?; chat_id_blocked = create_blocked; recreate_member_list = true; @@ -1445,6 +1476,73 @@ async fn create_or_lookup_group( Ok((chat_id, chat_id_blocked)) } +#[allow(clippy::indexing_slicing)] +async fn create_or_lookup_mailinglist( + context: &Context, + allow_creation: bool, + create_blocked: Blocked, + list_id_header: &str, + subject: &str, +) -> (ChatId, Blocked) { + static LIST_ID: Lazy = Lazy::new(|| Regex::new(r"^(.+)<(.+)>$").unwrap()); + let (mut name, listid) = match LIST_ID.captures(list_id_header) { + Some(cap) => (cap[1].trim().to_string(), cap[2].trim().to_string()), + None => ( + "".to_string(), + list_id_header + .trim() + .trim_start_matches('<') + .trim_end_matches('>') + .to_string(), + ), + }; + + if let Ok((chat_id, _, blocked)) = chat::get_chat_id_by_grpid(context, &listid).await { + return (chat_id, blocked); + } + + static SUBJECT: Lazy = Lazy::new(|| Regex::new(r"^.{0,5}\[(.*.)\]").unwrap()); + if let Some(cap) = SUBJECT.captures(subject) { + name = cap[1].to_string(); + } + + if name.is_empty() { + name = listid.clone(); + } + + if allow_creation { + // list does not exist but should be created + match create_multiuser_record( + context, + Chattype::Mailinglist, + &listid, + &name, + create_blocked, + ProtectionStatus::Unprotected, + ) + .await + { + Ok(chat_id) => { + chat::add_to_chat_contacts_table(context, chat_id, DC_CONTACT_ID_SELF).await; + (chat_id, create_blocked) + } + Err(e) => { + warn!( + context, + "Failed to create mailinglist '{}' for grpid={}: {}", + &name, + &listid, + e.to_string() + ); + (ChatId::new(0), create_blocked) + } + } + } else { + info!(context, "creating list forbidden by caller"); + (ChatId::new(0), Blocked::Not) + } +} + fn try_getting_grpid(mime_parser: &MimeMessage) -> Option { if let Some(optional_field) = mime_parser.get(HeaderDef::ChatGroupId) { return Some(optional_field.clone()); @@ -1484,8 +1582,6 @@ async fn create_adhoc_group( member_ids: &[u32], ) -> Result> { if mime_parser.is_mailinglist_message() { - // XXX we could parse List-* headers and actually create and - // manage a mailing list group, eventually info!( context, "not creating ad-hoc group for mailing list message" @@ -1522,14 +1618,15 @@ async fn create_adhoc_group( .get_subject() .unwrap_or_else(|| "Unnamed group".to_string()); - let new_chat_id: ChatId = create_group_record( + let new_chat_id: ChatId = create_multiuser_record( context, + Chattype::Group, &grpid, grpname, create_blocked, ProtectionStatus::Unprotected, ) - .await; + .await?; for &member_id in member_ids.iter() { chat::add_to_chat_contacts_table(context, new_chat_id, member_id).await; } @@ -1539,49 +1636,40 @@ async fn create_adhoc_group( Ok(Some(new_chat_id)) } -async fn create_group_record( +async fn create_multiuser_record( context: &Context, + chattype: Chattype, grpid: impl AsRef, grpname: impl AsRef, create_blocked: Blocked, create_protected: ProtectionStatus, -) -> ChatId { - if context.sql.execute( +) -> Result { + context.sql.execute( "INSERT INTO chats (type, name, grpid, blocked, created_timestamp, protected) VALUES(?, ?, ?, ?, ?, ?);", paramsv![ - Chattype::Group, + chattype, grpname.as_ref(), grpid.as_ref(), create_blocked, time(), create_protected, ], - ).await - .is_err() - { - warn!( - context, - "Failed to create group '{}' for grpid={}", - grpname.as_ref(), - grpid.as_ref() - ); - return ChatId::new(0); - } + ).await?; + let row_id = context .sql .get_rowid(context, "chats", "grpid", grpid.as_ref()) - .await - .unwrap_or_default(); + .await?; let chat_id = ChatId::new(row_id); info!( context, - "Created group '{}' grpid={} as {}", + "Created group/mailinglist '{}' grpid={} as {}", grpname.as_ref(), grpid.as_ref(), chat_id ); - chat_id + Ok(chat_id) } /// Creates ad-hoc group ID. @@ -1824,15 +1912,25 @@ pub(crate) async fn get_prefetch_parent_message( Ok(None) } +/// * param `prevent_rename`: if true, the display_name of this contact will not be changed. Useful for +/// mailing lists: In some mailing lists, many users write from the same address but with different +/// display names. We don't want the display name to change everytime the user gets a new email from +/// a mailing list. async fn dc_add_or_lookup_contacts_by_address_list( context: &Context, address_list: &[SingleInfo], origin: Origin, + prevent_rename: bool, ) -> Result { let mut contact_ids = ContactIds::new(); for info in address_list.iter() { + let display_name = if prevent_rename { + Some("") + } else { + info.display_name.as_deref() + }; contact_ids.insert( - add_or_lookup_contact_by_addr(context, &info.display_name, &info.addr, origin).await?, + add_or_lookup_contact_by_addr(context, display_name, &info.addr, origin).await?, ); } @@ -1842,17 +1940,14 @@ async fn dc_add_or_lookup_contacts_by_address_list( /// Add contacts to database on receiving messages. async fn add_or_lookup_contact_by_addr( context: &Context, - display_name: &Option, + display_name: Option>, addr: &str, origin: Origin, ) -> Result { if context.is_self_addr(addr).await? { return Ok(DC_CONTACT_ID_SELF); } - let display_name_normalized = display_name - .as_ref() - .map(normalize_name) - .unwrap_or_default(); + let display_name_normalized = display_name.map(normalize_name).unwrap_or_default(); let (row_id, _modified) = Contact::add_or_lookup(context, display_name_normalized, addr, origin).await?; @@ -1880,11 +1975,15 @@ fn dc_create_incoming_rfc724_mid( #[cfg(test)] mod tests { use super::*; - use crate::chat::{ChatItem, ChatVisibility}; - use crate::chatlist::Chatlist; use crate::constants::{DC_CONTACT_ID_INFO, DC_GCL_NO_SPECIALS}; + use crate::message::ContactRequestDecision::*; use crate::message::Message; use crate::test_utils::TestContext; + use crate::{ + chat::{ChatItem, ChatVisibility}, + constants::DC_CHAT_ID_DEADDROP, + }; + use crate::{chatlist::Chatlist, test_utils::get_chat_msg}; #[test] fn test_hex_hash() { @@ -2161,14 +2260,7 @@ mod tests { ) .await .unwrap(); - let msgs = chat::get_chat_msgs(&t, group_id, 0, None).await; - assert_eq!(msgs.len(), 1); - let msg_id = if let ChatItem::Message { msg_id } = msgs.first().unwrap() { - msg_id - } else { - panic!("Wrong item type"); - }; - let msg = message::Message::load_from_db(&t, *msg_id).await.unwrap(); + let msg = get_chat_msg(&t, group_id, 0, 1).await; assert_eq!(msg.is_dc_message, MessengerMessage::Yes); assert_eq!(msg.text.unwrap(), "hello"); assert_eq!(msg.state, MessageState::OutDelivered); @@ -2215,7 +2307,7 @@ mod tests { ) .await.unwrap(); assert_eq!(chat::get_chat_msgs(&t, group_id, 0, None).await.len(), 1); - let msg = message::Message::load_from_db(&t, *msg_id).await.unwrap(); + let msg = message::Message::load_from_db(&t, msg.id).await.unwrap(); assert_eq!(msg.state, MessageState::OutMdnRcvd); // check, the read-receipt has not unarchived the one2one @@ -2293,14 +2385,7 @@ mod tests { .get_authname(), "Имя, Фамилия", ); - let msgs = chat::get_chat_msgs(&t, chat_id, 0, None).await; - assert_eq!(msgs.len(), 1); - let msg_id = if let ChatItem::Message { msg_id } = msgs.first().unwrap() { - msg_id - } else { - panic!("Wrong item type"); - }; - let msg = message::Message::load_from_db(&t, *msg_id).await.unwrap(); + let msg = get_chat_msg(&t, chat_id, 0, 1).await; assert_eq!(msg.is_dc_message, MessengerMessage::Yes); assert_eq!(msg.text.unwrap(), "hello"); assert_eq!(msg.param.get_int(Param::WantsMdn).unwrap(), 1); @@ -2598,6 +2683,272 @@ mod tests { assert_eq!(msg.text.unwrap(), " Guten Abend, \n\n Lots of text \n\n text with Umlaut ä... \n\n MfG [...]"); } + static GH_MAILINGLIST: &[u8] = + b"Received: (Postfix, from userid 1000); Mon, 4 Dec 2006 14:51:39 +0100 (CET)\n\ + From: Max Mustermann \n\ + To: deltachat/deltachat-core-rust \n\ + Subject: Let's put some [brackets here that] have nothing to do with the topic\n\ + Message-ID: <3333@example.org>\n\ + List-ID: deltachat/deltachat-core-rust \n\ + Precedence: list\n\ + Date: Sun, 22 Mar 2020 22:37:57 +0000\n\ + \n\ + hello\n"; + + static GH_MAILINGLIST2: &[u8] = + b"Received: (Postfix, from userid 1000); Mon, 4 Dec 2006 14:51:39 +0100 (CET)\n\ + From: Github \n\ + To: deltachat/deltachat-core-rust \n\ + Subject: [deltachat/deltachat-core-rust] PR run failed\n\ + Message-ID: <3334@example.org>\n\ + List-ID: deltachat/deltachat-core-rust \n\ + Precedence: list\n\ + Date: Sun, 22 Mar 2020 22:37:57 +0000\n\ + \n\ + hello back\n"; + + #[async_std::test] + async fn test_github_mailing_list() { + let t = TestContext::new_alice().await; + t.ctx + .set_config(Config::ShowEmails, Some("2")) + .await + .unwrap(); + + dc_receive_imf(&t.ctx, GH_MAILINGLIST, "INBOX", 1, false) + .await + .unwrap(); + + let chats = Chatlist::try_load(&t.ctx, 0, None, None).await.unwrap(); + assert_eq!(chats.len(), 1); + + let chat_id = chat::create_by_msg_id(&t.ctx, chats.get_msg_id(0).unwrap()) + .await + .unwrap(); + let chat = chat::Chat::load_from_db(&t.ctx, chat_id).await.unwrap(); + + assert!(chat.is_mailing_list()); + assert_eq!(chat.can_send(), false); + assert_eq!(chat.name, "deltachat/deltachat-core-rust"); + assert_eq!(chat::get_chat_contacts(&t.ctx, chat_id).await.len(), 1); + + dc_receive_imf(&t.ctx, GH_MAILINGLIST2, "INBOX", 1, false) + .await + .unwrap(); + + let chats = Chatlist::try_load(&t.ctx, 0, None, None).await.unwrap(); + assert_eq!(chats.len(), 1); + let contacts = Contact::get_all(&t.ctx, 0, None as Option) + .await + .unwrap(); + assert_eq!(contacts.len(), 0); // mailing list recipients and senders do not count as "known contacts" + + let msg1 = get_chat_msg(&t, chat_id, 0, 2).await; + let contact1 = Contact::load_from_db(&t.ctx, msg1.from_id).await.unwrap(); + assert_eq!(contact1.get_addr(), "notifications@github.com"); + assert_eq!(contact1.get_display_name(), "notifications@github.com"); // Make sure this is not "Max Mustermann" or somethinng + + let msg2 = get_chat_msg(&t, chat_id, 1, 2).await; + let contact2 = Contact::load_from_db(&t.ctx, msg2.from_id).await.unwrap(); + assert_eq!(contact2.get_addr(), "notifications@github.com"); + + assert_eq!(msg1.get_override_sender_name().unwrap(), "Max Mustermann"); + assert_eq!(msg2.get_override_sender_name().unwrap(), "Github"); + } + + static DC_MAILINGLIST: &[u8] = b"Received: (Postfix, from userid 1000); Mon, 4 Dec 2006 14:51:39 +0100 (CET)\n\ + From: Bob \n\ + To: delta-dev@codespeak.net\n\ + Subject: Re: [delta-dev] What's up?\n\ + Message-ID: <38942@posteo.org>\n\ + List-ID: \"discussions about and around https://delta.chat developments\" \n\ + Precedence: list\n\ + Date: Sun, 22 Mar 2020 22:37:57 +0000\n\ + \n\ + body\n"; + + static DC_MAILINGLIST2: &[u8] = b"Received: (Postfix, from userid 1000); Mon, 4 Dec 2006 14:51:39 +0100 (CET)\n\ + From: Charlie \n\ + To: delta-dev@codespeak.net\n\ + Subject: Re: [delta-dev] DC is nice!\n\ + Message-ID: <38943@posteo.org>\n\ + List-ID: \"discussions about and around https://delta.chat developments\" \n\ + Precedence: list\n\ + Date: Sun, 22 Mar 2020 22:37:57 +0000\n\ + \n\ + body 4\n"; + + #[async_std::test] + async fn test_classic_mailing_list() { + let t = TestContext::new_alice().await; + t.ctx + .set_config(Config::ShowEmails, Some("2")) + .await + .unwrap(); + dc_receive_imf(&t.ctx, DC_MAILINGLIST, "INBOX", 1, false) + .await + .unwrap(); + let chats = Chatlist::try_load(&t.ctx, 0, None, None).await.unwrap(); + let chat_id = chat::create_by_msg_id(&t.ctx, chats.get_msg_id(0).unwrap()) + .await + .unwrap(); + let chat = Chat::load_from_db(&t.ctx, chat_id).await.unwrap(); + assert_eq!(chat.name, "delta-dev"); + + let msg = get_chat_msg(&t, chat_id, 0, 1).await; + let contact1 = Contact::load_from_db(&t.ctx, msg.from_id).await.unwrap(); + assert_eq!(contact1.get_addr(), "bob@posteo.org"); + } + + #[async_std::test] + async fn test_mailing_list_decide_block() { + let deaddrop = ChatId::new(DC_CHAT_ID_DEADDROP); + let t = TestContext::new_alice().await; + t.ctx + .set_config(Config::ShowEmails, Some("2")) + .await + .unwrap(); + + dc_receive_imf(&t.ctx, DC_MAILINGLIST, "INBOX", 1, false) + .await + .unwrap(); + let chats = Chatlist::try_load(&t.ctx, 0, None, None).await.unwrap(); + assert_eq!(chats.len(), 1); + assert_eq!(chats.get_chat_id(0), deaddrop); // Test that the message is shown in the deaddrop + + let msg = get_chat_msg(&t, deaddrop, 0, 1).await; + + // Answer "Block" on the contact request + message::decide_on_contact_request(&t.ctx, msg.get_id(), Block).await; + + let chats = Chatlist::try_load(&t.ctx, 0, None, None).await.unwrap(); + assert_eq!(chats.len(), 0); // Test that the message disappeared + let msgs = chat::get_chat_msgs(&t.ctx, deaddrop, 0, None).await; + assert_eq!(msgs.len(), 0); + + dc_receive_imf(&t.ctx, DC_MAILINGLIST2, "INBOX", 1, false) + .await + .unwrap(); + + // Test that the mailing list stays disappeared + let chats = Chatlist::try_load(&t.ctx, 0, None, None).await.unwrap(); + assert_eq!(chats.len(), 0); // Test that the message is not shown + let msgs = chat::get_chat_msgs(&t.ctx, deaddrop, 0, None).await; + assert_eq!(msgs.len(), 0); + } + + #[async_std::test] + async fn test_mailing_list_decide_not_now() { + let deaddrop = ChatId::new(DC_CHAT_ID_DEADDROP); + let t = TestContext::new_alice().await; + t.ctx + .set_config(Config::ShowEmails, Some("2")) + .await + .unwrap(); + + dc_receive_imf(&t.ctx, DC_MAILINGLIST, "INBOX", 1, false) + .await + .unwrap(); + + let msg = get_chat_msg(&t, deaddrop, 0, 1).await; + + // Answer "Not now" on the contact request + message::decide_on_contact_request(&t.ctx, msg.get_id(), NotNow).await; + + let chats = Chatlist::try_load(&t.ctx, 0, None, None).await.unwrap(); + assert_eq!(chats.len(), 0); // Test that the message disappeared + let msgs = chat::get_chat_msgs(&t.ctx, deaddrop, 0, None).await; + assert_eq!(msgs.len(), 1); // ...but is still shown in the deaddrop + + dc_receive_imf(&t.ctx, DC_MAILINGLIST2, "INBOX", 1, false) + .await + .unwrap(); + + let chats = Chatlist::try_load(&t.ctx, 0, None, None).await.unwrap(); + assert_eq!(chats.len(), 1); // Test that the new mailing list message is shown again + let msgs = chat::get_chat_msgs(&t.ctx, deaddrop, 0, None).await; + assert_eq!(msgs.len(), 2); + } + + #[async_std::test] + async fn test_mailing_list_decide_accept() { + let deaddrop = ChatId::new(DC_CHAT_ID_DEADDROP); + let t = TestContext::new_alice().await; + t.ctx + .set_config(Config::ShowEmails, Some("2")) + .await + .unwrap(); + + dc_receive_imf(&t.ctx, DC_MAILINGLIST, "INBOX", 1, false) + .await + .unwrap(); + + let msg = get_chat_msg(&t, deaddrop, 0, 1).await; + + // Answer "Start chat" on the contact request + message::decide_on_contact_request(&t.ctx, msg.get_id(), StartChat).await; + + let chats = Chatlist::try_load(&t.ctx, 0, None, None).await.unwrap(); + assert_eq!(chats.len(), 1); // Test that the message is shown + let chat_id = chats.get_chat_id(0); + assert_ne!(chat_id, deaddrop); + + dc_receive_imf(&t.ctx, DC_MAILINGLIST2, "INBOX", 1, false) + .await + .unwrap(); + + let msgs = chat::get_chat_msgs(&t.ctx, chat_id, 0, None).await; + assert_eq!(msgs.len(), 2); + } + + #[async_std::test] + async fn test_dont_show_tokens_in_contacts_list() { + check_dont_show_in_contacts_list( + "reply+OGHVYCLVBEGATYBICAXBIRQATABUOTUCERABERAHNO@reply.github.com", + ) + .await; + } + + #[async_std::test] + async fn test_dont_show_noreply_in_contacts_list() { + check_dont_show_in_contacts_list("noreply@github.com").await; + } + + async fn check_dont_show_in_contacts_list(addr: &str) { + let t = TestContext::new_alice().await; + t.ctx + .set_config(Config::ShowEmails, Some("2")) + .await + .unwrap(); + dc_receive_imf( + &t, + format!( + "Subject: Re: [deltachat/deltachat-core-rust] DC is the best repo on GitHub! +To: {} +References: + +From: alice@example.com +Message-ID: +Date: Tue, 16 Jun 2020 12:04:20 +0200 +MIME-Version: 1.0 +Content-Type: text/plain; charset=utf-8 +Content-Transfer-Encoding: 7bit + +YEAAAAAA!. +", + addr + ) + .as_bytes(), + "Sent", + 1, + false, + ) + .await + .unwrap(); + let contacts = Contact::get_all(&t, 0, None as Option<&str>).await.unwrap(); + assert!(contacts.is_empty()); // The contact should not have been added to the db + } + #[async_std::test] async fn test_pdf_filename_simple() { let t = TestContext::new_alice().await; diff --git a/src/imap/mod.rs b/src/imap/mod.rs index 81ac4b7e4..20fc62354 100644 --- a/src/imap/mod.rs +++ b/src/imap/mod.rs @@ -761,9 +761,12 @@ impl Imap { let msg = fetch?; match get_fetch_headers(&msg) { Ok(headers) => { - let (from_id, _, _) = - from_field_to_contact_id(context, &mimeparser::get_from(&headers)) - .await?; + let (from_id, _, _) = from_field_to_contact_id( + context, + &mimeparser::get_from(&headers), + false, + ) + .await?; if from_id == DC_CONTACT_ID_SELF { result.extend(mimeparser::get_recipients(&headers)); } @@ -1640,7 +1643,9 @@ pub(crate) async fn prefetch_should_download( .is_some(); let (_contact_id, blocked_contact, origin) = - from_field_to_contact_id(context, &mimeparser::get_from(headers)).await?; + from_field_to_contact_id(context, &mimeparser::get_from(headers), true).await?; + // prevent_rename=true as this might be a mailing list message and in this case it would be bad if we rename the contact. + // (prevent_rename is the last argument of from_field_to_contact_id()) let accepted_contact = origin.is_known(); let show = is_autocrypt_setup_message diff --git a/src/job.rs b/src/job.rs index 7ff4f1f31..bf9a5086f 100644 --- a/src/job.rs +++ b/src/job.rs @@ -690,18 +690,21 @@ impl Job { } Ok(c) => c, }; - if chat.typ == Chattype::Group { - // The next lines are actually what we do in - let (test_normal_chat_id, test_normal_chat_id_blocked) = - chat::lookup_by_contact_id(context, msg.from_id) - .await - .unwrap_or_default(); + match chat.typ { + Chattype::Group | Chattype::Mailinglist => { + // The next lines are actually what we do in + let (test_normal_chat_id, test_normal_chat_id_blocked) = + chat::lookup_by_contact_id(context, msg.from_id) + .await + .unwrap_or_default(); - if !test_normal_chat_id.is_unset() - && test_normal_chat_id_blocked == Blocked::Not - { - chat.id.unblock(context).await; + if !test_normal_chat_id.is_unset() + && test_normal_chat_id_blocked == Blocked::Not + { + chat.id.unblock(context).await; + } } + Chattype::Single | Chattype::Undefined => {} } } } diff --git a/src/message.rs b/src/message.rs index e42f07017..708ec783a 100644 --- a/src/message.rs +++ b/src/message.rs @@ -508,6 +508,8 @@ impl Message { self.from_id } + /// get the chat-id, + /// if the message is a contact request, the DC_CHAT_ID_DEADDROP is returned. pub fn get_chat_id(&self) -> ChatId { if self.chat_blocked != Blocked::Not { ChatId::new(DC_CHAT_ID_DEADDROP) @@ -516,6 +518,12 @@ impl Message { } } + /// get the chat-id, also when the message is still a contact request. + /// DC_CHAT_ID_DEADDROP is never returned. + pub fn get_real_chat_id(&self) -> ChatId { + self.chat_id + } + pub fn get_viewtype(&self) -> Viewtype { self.viewtype } @@ -590,8 +598,13 @@ impl Message { return ret; }; - let contact = if self.from_id != DC_CONTACT_ID_SELF as u32 && chat.typ == Chattype::Group { - Contact::get_by_id(context, self.from_id).await.ok() + let contact = if self.from_id != DC_CONTACT_ID_SELF as u32 { + match chat.typ { + Chattype::Group | Chattype::Mailinglist => { + Contact::get_by_id(context, self.from_id).await.ok() + } + Chattype::Single | Chattype::Undefined => None, + } } else { None }; @@ -612,6 +625,29 @@ impl Message { .await } + // It's a little unfortunate that the UI has to first call dc_msg_get_override_sender_name() and then if it was NULL, call + // dc_contact_get_display_name() but this was the best solution: + // - We could load a Contact struct from the db here to call get_display_name() instead of returning None, but then we had a db + // call everytime (and this fn is called a lot while the user is scrolling through a group), so performance would be bad + // - We could pass both a Contact struct and a Message struct in the FFI, but at least on Android we would need to handle raw + // C-data in the Java code (i.e. a `long` storing a C pointer) + // - We can't make a param `SenderDisplayname` for messages as sometimes the display name of a contact changes, and we want to show + // the same display name over all messages from the same sender. + pub fn get_override_sender_name(&self) -> Option { + if let Some(name) = self.param.get(Param::OverrideSenderDisplayname) { + Some(name.to_string()) + } else { + None + } + } + + // Exposing this function over the ffi instead of get_override_sender_name() would mean that at least Android Java code has + // to handle raw C-data (as it is done for dc_msg_get_summary()) + pub fn get_sender_name(&self, contact: &Contact) -> String { + self.get_override_sender_name() + .unwrap_or_else(|| contact.get_display_name().to_string()) + } + pub fn has_deviating_timestamp(&self) -> bool { let cnv_to_local = dc_gm2local_offset(); let sort_timestamp = self.get_sort_timestamp() as i64 + cnv_to_local; @@ -876,6 +912,13 @@ impl Message { } } +#[derive(Display, Debug, FromPrimitive)] +pub enum ContactRequestDecision { + StartChat = 0, + Block = 1, + NotNow = 2, +} + #[derive( Debug, Clone, @@ -1028,23 +1071,31 @@ impl Lot { ); self.text1_meaning = Meaning::Text1Self; } - } else if chat.typ == Chattype::Group { - if msg.is_info() || contact.is_none() { - self.text1 = None; - self.text1_meaning = Meaning::None; - } else { - if chat.id.is_deaddrop() { - if let Some(contact) = contact { - self.text1 = Some(contact.get_display_name().into()); - } else { + } else { + match chat.typ { + Chattype::Group | Chattype::Mailinglist => { + if msg.is_info() || contact.is_none() { self.text1 = None; + self.text1_meaning = Meaning::None; + } else { + if chat.id.is_deaddrop() { + if let Some(contact) = contact { + self.text1 = Some(msg.get_sender_name(contact)); + } else { + self.text1 = None; + } + } else if let Some(contact) = contact { + self.text1 = Some(msg.get_sender_name(contact)); + } else { + self.text1 = None; + } + self.text1_meaning = Meaning::Text1Username; } - } else if let Some(contact) = contact { - self.text1 = Some(contact.get_display_name().into()); - } else { - self.text1 = None; } - self.text1_meaning = Meaning::Text1Username; + Chattype::Single | Chattype::Undefined => { + self.text1 = None; + self.text1_meaning = Meaning::None; + } } } @@ -1071,6 +1122,76 @@ impl Lot { } } +/// Call this when the user decided about a deaddrop message ("Do you want to chat with NAME?"). +/// +/// If the decision is `StartChat`, this will create a new chat and return the chat id. +/// If the decision is `Block`, this will usually block the sender. +/// If the decision is `NotNow`, this will usually mark all messages from this sender as read. +/// +/// If the message belongs to a mailing list, makes sure that all messages from this mailing list are +/// blocked or marked as noticed. +/// +/// The user should be asked whether they want to chat with the _contact_ belonging to the message; +/// the group names may be really weird when taken from the subject of implicit (= ad-hoc) +/// groups and this may look confusing. Moreover, this function also scales up the origin of the contact. +/// +/// If the chat belongs to a mailing list, you can also ask +/// "Would you like to read MAILING LIST NAME in Delta Chat?" +/// (use `Message.get_real_chat_id()` to get the chat-id for the contact request +/// and then `Chat.is_mailing_list()`, `Chat.get_name()` and so on) +pub async fn decide_on_contact_request( + context: &Context, + msg_id: MsgId, + decision: ContactRequestDecision, +) -> Option { + let msg = match Message::load_from_db(context, msg_id).await { + Ok(m) => m, + Err(e) => { + warn!(context, "Can't load message: {}", e); + return None; + } + }; + + let chat = match Chat::load_from_db(context, msg.chat_id).await { + Ok(c) => c, + Err(e) => { + warn!(context, "Can't load chat: {}", e); + return None; + } + }; + + let mut created_chat_id = None; + use ContactRequestDecision::*; + match (decision, chat.is_mailing_list()) { + (StartChat, _) => match chat::create_by_msg_id(context, msg.id).await { + Ok(id) => created_chat_id = Some(id), + Err(e) => warn!(context, "decide_on_contact_request error: {}", e), + }, + + (Block, false) => Contact::block(context, msg.from_id).await, + (Block, true) => { + if !msg.chat_id.set_blocked(context, Blocked::Manually).await { + warn!(context, "Block mailing list failed.") + } + } + + (NotNow, false) => Contact::mark_noticed(context, msg.from_id).await, + (NotNow, true) => { + if let Err(e) = chat::marknoticed_chat(context, msg.chat_id).await { + warn!(context, "Marknoticed failed: {}", e) + } + } + } + + // Multiple chats may have changed, so send 0s + // (performance is not so important because this function is not called very often) + context.emit_event(EventType::MsgsChanged { + chat_id: ChatId::new(0), + msg_id: MsgId::new(0), + }); + created_chat_id +} + pub async fn get_msg_info(context: &Context, msg_id: MsgId) -> String { let mut ret = String::new(); @@ -1736,19 +1857,33 @@ async fn ndn_maybe_add_info_msg( chat_id: ChatId, chat_type: Chattype, ) -> anyhow::Result<()> { - if chat_type == Chattype::Group { - if let Some(failed_recipient) = &failed.failed_recipient { - let contact_id = Contact::lookup_id_by_addr(context, failed_recipient, Origin::Unknown) - .await? - .ok_or_else(|| Error::msg("ndn_maybe_add_info_msg: Contact ID not found"))?; - let contact = Contact::load_from_db(context, contact_id).await?; - // Tell the user which of the recipients failed if we know that (because in a group, this might otherwise be unclear) - let text = context - .stock_string_repl_str(StockMessage::FailedSendingTo, contact.get_display_name()) - .await; - chat::add_info_msg(context, chat_id, text).await; - context.emit_event(EventType::ChatModified(chat_id)); + match chat_type { + Chattype::Group => { + if let Some(failed_recipient) = &failed.failed_recipient { + let contact_id = + Contact::lookup_id_by_addr(context, failed_recipient, Origin::Unknown) + .await? + .ok_or_else(|| { + Error::msg("ndn_maybe_add_info_msg: Contact ID not found") + })?; + let contact = Contact::load_from_db(context, contact_id).await?; + // Tell the user which of the recipients failed if we know that (because in a group, this might otherwise be unclear) + let text = context + .stock_string_repl_str( + StockMessage::FailedSendingTo, + contact.get_display_name(), + ) + .await; + chat::add_info_msg(context, chat_id, text).await; + context.emit_event(EventType::ChatModified(chat_id)); + } } + Chattype::Mailinglist => { + // ndn_maybe_add_info_msg() is about the case when delivery to the group failed. + // If we get an NDN for the mailing list, just issue a warning. + warn!(context, "ignoring NDN for mailing list."); + } + Chattype::Single | Chattype::Undefined => {} } Ok(()) } @@ -1920,6 +2055,7 @@ mod tests { use super::*; use crate::chat::ChatItem; use crate::constants::DC_CONTACT_ID_DEVICE; + use crate::dc_receive_imf::dc_receive_imf; use crate::test_utils as test; use crate::test_utils::TestContext; @@ -2052,7 +2188,6 @@ mod tests { setupmessage: bool, sentbox_move: bool, ) { - use crate::dc_receive_imf::dc_receive_imf; println!("Testing: For folder {}, mvbox_move {}, chat_msg {}, accepted {}, outgoing {}, setupmessage {}", folder, mvbox_move, chat_msg, accepted_chat, outgoing, setupmessage); @@ -2395,4 +2530,33 @@ mod tests { .expect("quoted message not found"); assert!(quoted_msg.get_text() == msg2.quoted_text()); } + + #[async_std::test] + async fn test_get_chat_id() { + // Alice receives a message that pops up as a contact request + let alice = TestContext::new_alice().await; + dc_receive_imf( + &alice, + b"From: Bob \n\ + To: alice@example.com\n\ + Chat-Version: 1.0\n\ + Message-ID: <123@example.com>\n\ + Date: Fri, 29 Jan 2021 21:37:55 +0000\n\ + \n\ + hello\n", + "INBOX", + 123, + false, + ) + .await + .unwrap(); + + // check chat-id of this message + let msg = alice.get_last_msg().await; + assert!(msg.get_chat_id().is_deaddrop()); + assert!(msg.get_chat_id().is_special()); + assert!(!msg.get_real_chat_id().is_deaddrop()); + assert!(!msg.get_real_chat_id().is_special()); + assert_eq!(msg.get_text().unwrap(), "hello".to_string()); + } } diff --git a/src/mimeparser.rs b/src/mimeparser.rs index 488c1282c..8844dfae6 100644 --- a/src/mimeparser.rs +++ b/src/mimeparser.rs @@ -401,23 +401,21 @@ impl MimeMessage { prepend_subject = false } } - if prepend_subject { - let subj = subject - .find('[') - .and_then(|n| subject.get(..n)) - .unwrap_or(subject) - .trim(); - if !subj.is_empty() { - for part in self.parts.iter_mut() { - if !part.msg.is_empty() { - part.msg = format!("{} – {}", subj, part.msg); - break; - } - } + // For mailing lists, always add the subject because sometimes there are different topics + // and otherwise it might be hard to keep track: + if self.get(HeaderDef::ListId).is_some() { + prepend_subject = true; + } + + if prepend_subject && !subject.is_empty() { + let part_with_text = self.parts.iter_mut().find(|part| !part.msg.is_empty()); + if let Some(mut part) = part_with_text { + part.msg = format!("{} – {}", subject, part.msg); } } } + if self.is_forwarded { for part in self.parts.iter_mut() { part.param.set_int(Param::Forwarded, 1); diff --git a/src/param.rs b/src/param.rs index 33411f7ff..ca771b562 100644 --- a/src/param.rs +++ b/src/param.rs @@ -22,6 +22,10 @@ pub enum Param { /// For messages and jobs File = b'f', + /// For messages: This name should be shown instead of contact.get_display_name() + /// (used if this is a mailinglist) + OverrideSenderDisplayname = b'O', + /// For Messages Width = b'w', diff --git a/src/test_utils.rs b/src/test_utils.rs index 8041903b2..1dd3ec96c 100644 --- a/src/test_utils.rs +++ b/src/test_utils.rs @@ -13,10 +13,11 @@ use async_std::future::Future; use async_std::path::PathBuf; use async_std::pin::Pin; use async_std::sync::{Arc, RwLock}; +use chat::ChatItem; use once_cell::sync::Lazy; use tempfile::{tempdir, TempDir}; -use crate::chat::{self, Chat, ChatId, ChatItem}; +use crate::chat::{self, Chat, ChatId}; use crate::chatlist::Chatlist; use crate::config::Config; use crate::constants::{Viewtype, DC_CONTACT_ID_SELF, DC_MSG_ID_DAYMARKER, DC_MSG_ID_MARKER1}; @@ -475,6 +476,26 @@ pub fn bob_keypair() -> key::KeyPair { } } +/// Gets a specific message from a chat and asserts that the chat has a specific length. +/// +/// Panics if the length of the chat is not `asserted_msgs_count` or if the chat item at `index` is not a Message. +#[allow(clippy::indexing_slicing)] +pub(crate) async fn get_chat_msg( + t: &TestContext, + chat_id: ChatId, + index: usize, + asserted_msgs_count: usize, +) -> Message { + let msgs = chat::get_chat_msgs(&t.ctx, chat_id, 0, None).await; + assert_eq!(msgs.len(), asserted_msgs_count); + let msg_id = if let ChatItem::Message { msg_id } = msgs[index] { + msg_id + } else { + panic!("Wrong item type"); + }; + Message::load_from_db(&t.ctx, msg_id).await.unwrap() +} + /// Pretty-print an event to stdout /// /// Done during tests this is captured by `cargo test` and associated with the test itself.