From 005e24fada851efaed112fce4244a3af225d8f74 Mon Sep 17 00:00:00 2001 From: Olivier Dony Date: Tue, 6 Jan 2015 15:26:21 +0100 Subject: [PATCH] [FIX] mail.thread: correct matching when finding author + test The previous matching rules were too fuzzy and allowed random prefix-match or tail-match of other user's emails. For example when looking up a partner matching 'foo@bar.com' the system would sometimes find 'dom.foo@bar.com' instead, or 'foo@bar.com.tw'. Fixed by only allowing direct case-insensitive email match of an addr-spec, or substring match of the addr-spec enclosed in angle brackets, within a name-addr pair. See also RFC5322, section 3.4 Also adapted related message_find_partner_from_emails() method to factor out the partner email resolution mechanism to avoid the same problem. Adds corresponding regression test. --- addons/mail/mail_thread.py | 42 +++++++++++++++++--------- addons/mail/tests/test_mail_gateway.py | 10 ++++++ 2 files changed, 38 insertions(+), 14 deletions(-) diff --git a/addons/mail/mail_thread.py b/addons/mail/mail_thread.py index e82ed94c43b..8a336f7cd01 100644 --- a/addons/mail/mail_thread.py +++ b/addons/mail/mail_thread.py @@ -474,18 +474,35 @@ class mail_thread(osv.AbstractModel): ret_dict[model_name] = model._description return ret_dict + def _message_partner_id_by_email(self, cr, uid, email_address, context=None): + """ Find a partner ID corresponding to the given email address """ + partner_obj = self.pool['res.partner'] + # Escape special SQL characters in email_address to avoid invalid matches + email_address = (email_address.replace('\\', '\\\\') + .replace('%', '\\%') + .replace('_', '\\_')) + # exact, case-insensitive match + result = partner_obj.search(cr, uid, [('email', '=ilike', email_address), ('user_ids', '!=', False)], limit=1, context=context) + if not result: + result = partner_obj.search(cr, uid, [('email', '=ilike', email_address)], limit=1, context=context) + # if no match with addr-spec, attempt substring match within name-addr pair (See RFC5322, section 3.4) + email_address = "<%s>" % email_address + if not result: + result = partner_obj.search(cr, uid, [('email', 'ilike', email_address), ('user_ids', '!=', False)], limit=1, context=context) + if not result: + result = partner_obj.search(cr, uid, [('email', 'ilike', email_address)], limit=1, context=context) + return result[0] if result else None + def _message_find_partners(self, cr, uid, message, header_fields=['From'], context=None): """ Find partners related to some header fields of the message. TDE TODO: merge me with other partner finding methods in 8.0 """ - partner_obj = self.pool.get('res.partner') partner_ids = [] s = ', '.join([decode(message.get(h)) for h in header_fields if message.get(h)]) for email_address in tools.email_split(s): - related_partners = partner_obj.search(cr, uid, [('email', 'ilike', email_address), ('user_ids', '!=', False)], limit=1, context=context) - if not related_partners: - related_partners = partner_obj.search(cr, uid, [('email', 'ilike', email_address)], limit=1, context=context) - partner_ids += related_partners + partner_id = self._message_partner_id_by_email(cr, uid, email_address, context=context) + if partner_id: + partner_ids.append(partner_id) return partner_ids def _message_find_user_id(self, cr, uid, message, context=None): @@ -999,7 +1016,6 @@ class mail_thread(osv.AbstractModel): TDE TODO: merge me with other partner finding methods in 8.0 """ mail_message_obj = self.pool.get('mail.message') - partner_obj = self.pool.get('res.partner') result = list() if id and self._name != 'mail.thread': obj = self.browse(cr, SUPERUSER_ID, id, context=context) @@ -1007,10 +1023,10 @@ class mail_thread(osv.AbstractModel): obj = None for email in emails: partner_info = {'full_name': email, 'partner_id': False} - m = re.search(r"((.+?)\s*<)?([^<>]+@[^<>]+)>?", email, re.IGNORECASE | re.DOTALL) - if not m: + split = tools.email_split(email) + if not split: continue - email_address = m.group(3) + email_address = split[0] # first try: check in document's followers if obj: for follower in obj.message_follower_ids: @@ -1018,11 +1034,9 @@ class mail_thread(osv.AbstractModel): partner_info['partner_id'] = follower.id # second try: check in partners if not partner_info.get('partner_id'): - ids = partner_obj.search(cr, SUPERUSER_ID, [('email', 'ilike', email_address), ('user_ids', '!=', False)], limit=1, context=context) - if not ids: - ids = partner_obj.search(cr, SUPERUSER_ID, [('email', 'ilike', email_address)], limit=1, context=context) - if ids: - partner_info['partner_id'] = ids[0] + partner_id = self._message_partner_id_by_email(cr, uid, email_address, context=context) + if partner_id: + partner_info['partner_id'] = partner_id result.append(partner_info) # link mail with this from mail to the new partner id diff --git a/addons/mail/tests/test_mail_gateway.py b/addons/mail/tests/test_mail_gateway.py index c952c6ab076..fcb1890cdad 100644 --- a/addons/mail/tests/test_mail_gateway.py +++ b/addons/mail/tests/test_mail_gateway.py @@ -430,6 +430,16 @@ class TestMailgateway(TestMailBase): self.assertEqual(frog_group.message_ids[0].author_id.id, extra_partner_id, 'message_process: email_from -> author_id wrong') + # Do: post a new message with a non-existant email that is a substring of a partner email + format_and_process(MAIL_TEMPLATE, email_from='Not really Lombrik Lubrik ', + subject='Re: news (2)', + msg_id='', + extra='In-Reply-To: <1198923581.41972151344608186760.JavaMail@agrolait.com>\n') + frog_groups = self.mail_group.search(cr, uid, [('name', '=', 'Frogs')]) + frog_group = self.mail_group.browse(cr, uid, frog_groups[0]) + # Test: author must not be set, otherwise the system is confusing different users + self.assertFalse(frog_group.message_ids[0].author_id, 'message_process: email_from -> mismatching author_id') + # Do: post a new message, with a known partner -> duplicate emails -> user frog_group.message_unsubscribe([extra_partner_id]) raoul_email = self.user_raoul.email