From 6cfb01d7c9bbfb4181ec401821617572f2a42c3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Fri, 22 Mar 2013 13:48:09 +0100 Subject: [PATCH] [FIX] mail: message_process: do not process incoming emails with a message_id already existing in database. Before trying to find possible routes, check that the incoming email's message_id is not already present in mail.message table. If it is the case, return False. Parsing of the message has been moved before routing, to avoid looking for routes for emails we will discard. Tests have been added and updated. bzr revid: tde@openerp.com-20130322124809-ven2p5kxpqfjqxb5 --- addons/mail/mail_thread.py | 16 ++++-- addons/mail/tests/test_mail_gateway.py | 69 +++++++++++++++++++++----- 2 files changed, 70 insertions(+), 15 deletions(-) diff --git a/addons/mail/mail_thread.py b/addons/mail/mail_thread.py index f42c969722f..c5ef16d1f5f 100644 --- a/addons/mail/mail_thread.py +++ b/addons/mail/mail_thread.py @@ -592,12 +592,22 @@ class mail_thread(osv.AbstractModel): if isinstance(message, unicode): message = message.encode('utf-8') msg_txt = email.message_from_string(message) - routes = self.message_route(cr, uid, msg_txt, model, - thread_id, custom_values, - context=context) + + # parse the message, verify we are not in a loop by checking message_id is not duplicated msg = self.message_parse(cr, uid, msg_txt, save_original=save_original, context=context) if strip_attachments: msg.pop('attachments', None) + if msg.get('message_id'): # should always be True as message_parse generate one if missing + existing_msg_ids = self.pool.get('mail.message').search(cr, SUPERUSER_ID, [ + ('message_id', '=', msg.get('message_id')), + ], context=context) + if existing_msg_ids: + return False + + # find possible routes for the message + routes = self.message_route(cr, uid, msg_txt, model, + thread_id, custom_values, + context=context) # postpone setting msg.partner_ids after message_post, to avoid double notifications partner_ids = msg.pop('partner_ids', []) diff --git a/addons/mail/tests/test_mail_gateway.py b/addons/mail/tests/test_mail_gateway.py index eb6ba22067b..a6b7df08aab 100644 --- a/addons/mail/tests/test_mail_gateway.py +++ b/addons/mail/tests/test_mail_gateway.py @@ -118,6 +118,9 @@ class TestMailgateway(TestMailBase): # Test: one group created by mailgateway administrator self.assertTrue(len(frog_groups) == 1) frog_group = self.mail_group.browse(cr, uid, frog_groups[0]) + res = self.mail_group.perm_read(cr, uid, [frog_group.id], details=False) + self.assertEqual(res[0].get('create_uid'), uid, + 'message_process: group should have been created by uid as alias_user__id is False on the alias') # Test: one message that is the incoming email self.assertEqual(len(frog_group.message_ids), 1, 'message_process: newly created group should have the incoming email in message_ids') @@ -150,9 +153,12 @@ class TestMailgateway(TestMailBase): self._init_mock_build_email() frog_groups = format_and_process(MAIL_TEMPLATE, to='groups@example.com, other@gmail.com') sent_emails = self._build_email_kwargs_list - # Test: one group created by raoul + # Test: one group created by Raoul self.assertTrue(len(frog_groups) == 1) frog_group = self.mail_group.browse(cr, uid, frog_groups[0]) + res = self.mail_group.perm_read(cr, uid, [frog_group.id], details=False) + self.assertEqual(res[0].get('create_uid'), self.user_raoul_id, + 'message_process: group should have been created by alias_user_id') # Test: one message that is the incoming email self.assertEqual(len(frog_group.message_ids), 1, 'message_process: newly created group should have the incoming email in message_ids') @@ -195,6 +201,7 @@ class TestMailgateway(TestMailBase): # Do: even with a wrong destination, a reply should end up in the correct thread frog_groups = format_and_process(MAIL_TEMPLATE, email_from='other@gmail.com', + msg_id='<1198923581.41972151344608186760.JavaMail.diff1@agrolait.com>', to='erroneous@example.com>', subject='Re: news', extra='In-Reply-To: <12321321-openerp-%d-mail.group@example.com>\n' % frog_group.id) # Test: no group 'Re: news' created, still only 1 Frogs group @@ -205,12 +212,30 @@ class TestMailgateway(TestMailBase): 'message_process: reply on Frogs should not have created a duplicate group with old subject') frog_group = self.mail_group.browse(cr, uid, frog_groups[0]) # Test: one new message - self.assertTrue(len(frog_group.message_ids) == 2, 'message_process: group should contain 2 messages after reply') + self.assertEqual(len(frog_group.message_ids), 2, 'message_process: group should contain 2 messages after reply') # Test: author (and not recipient) added as follower frog_follower_ids = set([p.id for p in frog_group.message_follower_ids]) self.assertEqual(frog_follower_ids, set([p1id, p2id]), 'message_process: after reply, group should have 2 followers') + # Do: due to some issue, same email goes back into the mailgateway + frog_groups = format_and_process(MAIL_TEMPLATE, email_from='other@gmail.com', + msg_id='<1198923581.41972151344608186760.JavaMail.diff1@agrolait.com>', + subject='Re: news', extra='In-Reply-To: <12321321-openerp-%d-mail.group@example.com>\n' % frog_group.id) + # Test: no group 'Re: news' created, still only 1 Frogs group + self.assertEqual(len(frog_groups), 0, + 'message_process: reply on Frogs should not have created a new group with new subject') + frog_groups = self.mail_group.search(cr, uid, [('name', '=', 'Frogs')]) + self.assertEqual(len(frog_groups), 1, + 'message_process: reply on Frogs should not have created a duplicate group with old subject') + frog_group = self.mail_group.browse(cr, uid, frog_groups[0]) + # Test: no new message + self.assertEqual(len(frog_group.message_ids), 2, 'message_process: message with already existing message_id should not have been duplicated') + # Test: message_id is still unique + msg_ids = self.mail_message.search(cr, uid, [('message_id', 'ilike', '<1198923581.41972151344608186760.JavaMail.diff1@agrolait.com>')]) + self.assertEqual(len(msg_ids), 1, + 'message_process: message with already existing message_id should not have been duplicated') + # -------------------------------------------------- # Test3: email_from and partner finding # -------------------------------------------------- @@ -223,6 +248,7 @@ class TestMailgateway(TestMailBase): # Do: post a new message, with a known partner -> duplicate emails -> partner format_and_process(MAIL_TEMPLATE, email_from='Lombrik Lubrik ', to='erroneous@example.com>', subject='Re: news (2)', + msg_id='<1198923581.41972151344608186760.JavaMail.new1@agrolait.com>', extra='In-Reply-To: <12321321-openerp-%d-mail.group@example.com>\n' % frog_group.id) frog_groups = self.mail_group.search(cr, uid, [('name', '=', 'Frogs')]) frog_group = self.mail_group.browse(cr, uid, frog_groups[0]) @@ -236,6 +262,7 @@ class TestMailgateway(TestMailBase): self.res_users.write(cr, uid, self.user_raoul_id, {'email': 'test_raoul@email.com'}) format_and_process(MAIL_TEMPLATE, email_from='Lombrik Lubrik ', to='erroneous@example.com>', subject='Re: news (3)', + msg_id='<1198923581.41972151344608186760.JavaMail.new2@agrolait.com>', extra='In-Reply-To: <12321321-openerp-%d-mail.group@example.com>\n' % frog_group.id) frog_groups = self.mail_group.search(cr, uid, [('name', '=', 'Frogs')]) frog_group = self.mail_group.browse(cr, uid, frog_groups[0]) @@ -250,6 +277,7 @@ class TestMailgateway(TestMailBase): self.res_users.write(cr, uid, self.user_raoul_id, {'email': 'test_raoul@email.com'}) format_and_process(MAIL_TEMPLATE, email_from='Lombrik Lubrik ', to='erroneous@example.com>', subject='Re: news (3)', + msg_id='<1198923581.41972151344608186760.JavaMail.new3@agrolait.com>', extra='In-Reply-To: <12321321-openerp-%d-mail.group@example.com>\n' % frog_group.id) frog_groups = self.mail_group.search(cr, uid, [('name', '=', 'Frogs')]) frog_group = self.mail_group.browse(cr, uid, frog_groups[0]) @@ -265,21 +293,30 @@ class TestMailgateway(TestMailBase): # Do: incoming email with model that does not accepts incoming emails must raise self.assertRaises(AssertionError, - format_and_process, - MAIL_TEMPLATE, to='noone@example.com', subject='spam', extra='', model='res.country') + format_and_process, + MAIL_TEMPLATE, + to='noone@example.com', subject='spam', extra='', model='res.country', + msg_id='<1198923581.41972151344608186760.JavaMail.new4@agrolait.com>') # Do: incoming email without model and without alias must raise self.assertRaises(AssertionError, - format_and_process, - MAIL_TEMPLATE, to='noone@example.com', subject='spam', extra='') + format_and_process, + MAIL_TEMPLATE, + to='noone@example.com', subject='spam', extra='', + msg_id='<1198923581.41972151344608186760.JavaMail.new5@agrolait.com>') # Do: incoming email with model that accepting incoming emails as fallback - frog_groups = format_and_process(MAIL_TEMPLATE, to='noone@example.com', subject='Spammy', extra='', model='mail.group') + frog_groups = format_and_process(MAIL_TEMPLATE, + to='noone@example.com', + subject='Spammy', extra='', model='mail.group', + msg_id='<1198923581.41972151344608186760.JavaMail.new6@agrolait.com>') self.assertEqual(len(frog_groups), 1, 'message_process: erroneous email but with a fallback model should have created a new mail.group') # Do: incoming email in plaintext should be stored as html - frog_groups = format_and_process(MAIL_TEMPLATE_PLAINTEXT, to='groups@example.com', subject='Frogs Return', extra='', msg_id='') + frog_groups = format_and_process(MAIL_TEMPLATE_PLAINTEXT, + to='groups@example.com', subject='Frogs Return', extra='', + msg_id='') # Test: one group created with one message self.assertTrue(len(frog_groups) == 1) frog_group = self.mail_group.browse(cr, uid, frog_groups[0]) @@ -305,19 +342,27 @@ class TestMailgateway(TestMailBase): # Reply to msg1, make sure the reply is properly attached using the various reply identification mechanisms # 0. Direct alias match - reply_msg1 = format(MAIL_TEMPLATE, to='Pretty Pigs ', extra='In-Reply-To: %s' % msg1.message_id) + reply_msg1 = format(MAIL_TEMPLATE, to='Pretty Pigs ', + extra='In-Reply-To: %s' % msg1.message_id, + msg_id='<1198923581.41972151344608186760.JavaMail.2@agrolait.com>') self.mail_group.message_process(cr, uid, None, reply_msg1) # 1. In-Reply-To header - reply_msg2 = format(MAIL_TEMPLATE, to='erroneous@example.com', extra='In-Reply-To: %s' % msg1.message_id) + reply_msg2 = format(MAIL_TEMPLATE, to='erroneous@example.com', + extra='In-Reply-To: %s' % msg1.message_id, + msg_id='<1198923581.41972151344608186760.JavaMail.3@agrolait.com>') self.mail_group.message_process(cr, uid, None, reply_msg2) # 2. References header - reply_msg3 = format(MAIL_TEMPLATE, to='erroneous@example.com', extra='References: <2233@a.com>\r\n\t<3edss_dsa@b.com> %s' % msg1.message_id) + reply_msg3 = format(MAIL_TEMPLATE, to='erroneous@example.com', + extra='References: <2233@a.com>\r\n\t<3edss_dsa@b.com> %s' % msg1.message_id, + msg_id='<1198923581.41972151344608186760.JavaMail.4@agrolait.com>') self.mail_group.message_process(cr, uid, None, reply_msg3) # 3. Subject contains [] + model passed to message+process -> only attached to group, but not to mail (not in msg1.child_ids) - reply_msg4 = format(MAIL_TEMPLATE, to='erroneous@example.com', extra='', subject='Re: [%s] 1' % self.group_pigs_id) + reply_msg4 = format(MAIL_TEMPLATE, to='erroneous@example.com', + extra='', subject='Re: [%s] 1' % self.group_pigs_id, + msg_id='<1198923581.41972151344608186760.JavaMail.5@agrolait.com>') self.mail_group.message_process(cr, uid, 'mail.group', reply_msg4) group_pigs.refresh()