From 4ea9cd3cb2b56aa3bc349f327108d552ad2e2b73 Mon Sep 17 00:00:00 2001 From: Anton Khirnov Date: Wed, 24 Nov 2021 15:02:13 +0100 Subject: mail/reply: rewrite recipient selection Thoroughly ensure that To+Cc contains neither our own address (except when it is wanted) nor any duplicates. Use structured headers provided by email.headerregistry to simplify dealing with addr-spec vs. mailbox (display name + ). --- alot/commands/thread.py | 9 +- alot/db/message.py | 14 ++++ alot/mail/policy.py | 12 ++- alot/mail/reply.py | 217 +++++++++++++++++++++++++++++------------------- 4 files changed, 159 insertions(+), 93 deletions(-) diff --git a/alot/commands/thread.py b/alot/commands/thread.py index 5d8006a1..a2a33ef1 100644 --- a/alot/commands/thread.py +++ b/alot/commands/thread.py @@ -96,9 +96,10 @@ class ReplyCommand(Command): reply.ReplyMode.AUTHOR to, cc = reply.determine_recipients(message, account, mode) - headers[HDR.TO] = to + # TODO keep those as lists? + headers[HDR.TO] = str(to) if cc: - headers[HDR.CC] = cc + headers[HDR.CC] = str(cc) # set In-Reply-To + References headers headers[HDR.IN_REPLY_TO] = '<%s>' % message.id @@ -107,9 +108,9 @@ class ReplyCommand(Command): # if any of the recipients is a mailinglist that we are subscribed to, # set Mail-Followup-To header so that duplicates are avoided # to and cc are already cleared of our own address - mft = reply.mail_followup_to([to, cc]) + mft = reply.mail_followup_to(to + cc) if mft: - headers[HDR.MAIL_FOLLOWUP_TO] = mft + headers[HDR.MAIL_FOLLOWUP_TO] = str(mft) body_text = reply.body_text(message, ui) diff --git a/alot/db/message.py b/alot/db/message.py index 97ab991f..01d7c7f1 100644 --- a/alot/db/message.py +++ b/alot/db/message.py @@ -119,6 +119,20 @@ class _MessageHeaders: return [] raise + def pick_addrlist_header(self, *args): + """ + Given a list of "address-list" header names as positional arguments, + return the addresses from the first one that is present. + + :returns: addresses from the first existing header, or an empty tuple + :rtype: tuple(headerregistry.Address) + """ + for h in args: + if h in self: + return self.get(h).addresses + + return () + class _MimeTree: _part = None diff --git a/alot/mail/policy.py b/alot/mail/policy.py index 1b9f07c3..6125f8f7 100644 --- a/alot/mail/policy.py +++ b/alot/mail/policy.py @@ -6,13 +6,17 @@ import email.headerregistry as _hdr from . import headers as HDR +# here we define our custom policy that handles additional headers + _headers = { - HDR.X_BEEN_THERE : _hdr.UniqueAddressHeader, - HDR.X_MAILING_LIST : _hdr.UniqueAddressHeader, + HDR.X_BEEN_THERE : _hdr.UniqueAddressHeader, + HDR.X_MAILING_LIST : _hdr.UniqueAddressHeader, + HDR.MAIL_FOLLOWUP_TO : _hdr.UniqueAddressHeader, + HDR.MAIL_REPLY_TO : _hdr.UniqueAddressHeader, } -# here we define our custom policy that handles additional headers -# derive from SMTP +# derive from SMTP, but create a new instance to get our own +# instance of header_factory p = _p.EmailPolicy() + _p.SMTP for h, t in _headers.items(): diff --git a/alot/mail/reply.py b/alot/mail/reply.py index 4a69ecee..43381602 100644 --- a/alot/mail/reply.py +++ b/alot/mail/reply.py @@ -2,7 +2,7 @@ # For further details see the COPYING file import email.policy -from email.utils import getaddresses, parseaddr +from email.utils import getaddresses from enum import Enum, auto import logging @@ -75,92 +75,140 @@ class ReplyMode(Enum): GROUP = auto() LIST = auto() -def _ensure_unique_address(recipients): +class AddrList: """ - clean up a list of name,address pairs so that - no address appears multiple times. + An ordered set of unique addresses. """ - res = dict() - for name, address in getaddresses(recipients): - res[address] = name - logging.debug(res) - urecipients = [formataddr((n, a)) for a, n in res.items()] - return sorted(urecipients) - -def _clear_my_address(my_account, value): - """return recipient header without the addresses in my_account - - :param my_account: my account - :type my_account: :class:`Account` - :param value: a list of recipient or sender strings (with or without - real names as taken from email headers) - :type value: list(str) - :returns: a new, potentially shortend list - :rtype: list(str) - """ - new_value = [] - for name, address in getaddresses(value): - if not my_account.matches_address(address): - new_value.append(formataddr((name, address))) - return new_value -def determine_recipients(message, account, mode): - # set To - sender = (message.headers.get(HDR.REPLY_TO) or - message.headers.get(HDR.FROM) or '') - sender_address = parseaddr(sender)[1] - cc = [] - - # check if reply is to self sent message - if account.matches_address(sender_address): - recipients = message.headers.get_all(HDR.TO) - emsg = 'Replying to own message, set recipients to: %s' \ - % recipients - logging.debug(emsg) + # list of Address objects + _addrs = None + # mapping of addr-spec -> Address + _addr_specs = None + + def __init__(self, addresses): + self._addrs = [] + self._addr_specs = {} + + for addr in addresses: + self.add(addr) + + def __str__(self): + return ', '.join((str(a) for a in self._addrs)) + + def __len__(self): + return len(self._addrs) + + def __iter__(self): + return iter(self._addrs) + + def _addr_to_addr_spec(self, addr): + if hasattr(addr, 'addr_spec'): + # headerregistry.Address + return addr.addr_spec + + # account.Address + return str(addr) + + def __contains__(self, addr): + return self._addr_to_addr_spec(addr) in self._addr_specs + + def __delitem__(self, addr): + addr_spec = self._addr_to_addr_spec(addr) + idx = self._addrs.index(addr_spec) + del self._addrs[idx] + del self._addr_specs[addr_spec] + + def __add__(self, other): + ret = self.__class__(self) + for a in other: + ret.add(a) + return ret + + def add(self, addr): + if not addr.addr_spec: + raise ValueError('Missing addr-spec') + + if addr in self: + idx = self._addrs.index(self._addr_specs[addr.addr_spec]) + + # if the address we already have is missing display name, + # replace it with the new one + if not self._addrs[idx].display_name and addr.display_name: + self._addrs[idx] = addr + self._addr_specs[addr.addr_spec] = addr + + return + + self._addrs.append(addr) + self._addr_specs[addr.addr_spec] = addr + +def _reply_list(message): + # To choose the target of the reply --list + # Reply-To is standart reply target RFC 2822:, RFC 1036: 2.2.1 + # X-BeenThere is needed by sourceforge ML also winehq + # X-Mailing-List is also standart and is used by git-send-mail + # Some mail server (gmail) will not resend you own mail, so we + # fall back to 'To' as last resort + to = message.headers.pick_addrlist_header( + HDR.X_BEEN_THERE, HDR.X_MAILING_LIST, HDR.REPLY_TO, HDR.TO) + + # copy original Cc + cc = message.headers.pick_addrlist_header(HDR.CC) + + return to, cc + +def _reply_group(message, senders): + # make sure that our own address is not included + # if the message was self-sent, then our address is not included + MFT = message.headers.pick_addrlist_header(HDR.MAIL_FOLLOWUP_TO) + if MFT and settings.get('honor_followup_to'): + logging.debug('honor followup to: %s', MFT) + to = MFT + cc = () else: - recipients = [sender] - - if mode == ReplyMode.GROUP: - # make sure that our own address is not included - # if the message was self-sent, then our address is not included - MFT = message.headers.get_all(HDR.MAIL_FOLLOWUP_TO) - followupto = _clear_my_address(account, MFT) - if followupto and settings.get('honor_followup_to'): - logging.debug('honor followup to: %s', ', '.join(followupto)) - recipients = followupto - # since Mail-Followup-To was set, ignore the Cc header - else: - if sender != message.headers.get(HDR.FROM): - recipients.append(message.headers.get(HDR.FROM)) - - # append To addresses if not replying to self sent message - if not account.matches_address(sender_address): - cleared = _clear_my_address(account, - message.headers.get_all(HDR.TO)) - recipients.extend(cleared) - - # copy cc for group-replies - if HDR.CC in message.headers: - cc = _clear_my_address(account, message.headers.get_all(HDR.CC)) - - to = ', '.join(_ensure_unique_address(recipients)) - cc = ', '.join(cc) - logging.debug('reply to: %s', to) + to = message.headers.pick_addrlist_header(HDR.TO) + senders + cc = message.headers.pick_addrlist_header(HDR.CC) + + return to, cc + +def determine_recipients(message, account, mode): + # TODO: detect reply-to munging by ML here? + senders = message.headers.pick_addrlist_header(HDR.MAIL_REPLY_TO, + HDR.REPLY_TO, HDR.FROM) + + self_reply = len(senders) == 1 and account.matches_address(senders[0].addr_spec) + if self_reply: + senders = message.headers.get(HDR.TO) + senders = senders.addresses if senders else () + logging.debug('Replying to own message, invert senders to: %s', + senders) if mode == ReplyMode.LIST: - # To choose the target of the reply --list - # Reply-To is standart reply target RFC 2822:, RFC 1036: 2.2.1 - # X-BeenThere is needed by sourceforge ML also winehq - # X-Mailing-List is also standart and is used by git-send-mail - to = (message.headers.get(HDR.REPLY_TO) or - message.headers.get(HDR.X_BEEN_THERE) or - message.headers.get(HDR.X_MAILING_LIST)) - - # Some mail server (gmail) will not resend you own mail, so you - # have to deal with the one in sent - if to is None: - to = message.headers[HDR.TO] - logging.debug('mail list reply to: %s', to) + to, cc = _reply_list(message) + elif mode == ReplyMode.GROUP: + to, cc = _reply_group(message, senders) + else: # mode == ReplyMode.AUTHOR + to = senders + cc = () + + to = AddrList(to) + cc = AddrList(cc) + + # remove duplicates across to/cc + for a in to: + if a in cc: + del cc[a] + + # remove sending account from the address lists + for a in [account.address] + account.aliases: + # keep own address if it's the only recipient + # i.e. the user deliberately wants to self-send the reply + if a in to and len(to) > 1: + del to[a] + if a in cc: + del cc[a] + + logging.debug('%s reply to: %s %s', mode, str(to), str(cc)) return to, cc @@ -217,7 +265,6 @@ def mail_followup_to(recipients): lists = settings.get('mailinglists') # check if any recipient address matches a known mailing list - if any(addr in lists for n, addr in getaddresses(recipients)): - followupto = ', '.join(recipients) - logging.debug('mail followup to: %s', followupto) - return followupto + if any(a.addr_spec in lists for a in recipients): + logging.debug('mail followup to: %s', recipients) + return recipients -- cgit v1.2.3