odoo
/
odoo
2
0
Fork 0
Code Pull Requests Releases Activity

[MERGE] [FIX] mail: message_process: do not process incoming emails 

with a message_id already existing in database. Indeed loops can occur when
openerp send notification emails to partners having an email address
matching an alias. The mail was routed in the same thread, leading to
further notification emails sent, causing an email loop.

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.

bzr revid: tde@openerp.com-20130322142916-0q3epz7dpgkyzuj0
This commit is contained in:
Thibault Delavallée 2013-03-22 15:29:16 +01:00
parent 9f29b5da17 4ebbf0e58d
commit 46dc796812
2 changed files with 91 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

43
addons/mail/mail_thread.py
View File

@ -468,6 +468,8 @@ class mail_thread(osv.AbstractModel):
"""
assert isinstance(message, Message), 'message must be an email.message.Message at this point'
message_id = message.get('Message-Id')
email_from = decode_header(message, 'From')
email_to = decode_header(message, 'To')
references = decode_header(message, 'References')
in_reply_to = decode_header(message, 'In-Reply-To')
@ -480,8 +482,8 @@ class mail_thread(osv.AbstractModel):
model_pool = self.pool.get(model)
if thread_id and model and model_pool and model_pool.exists(cr, uid, thread_id) \
and hasattr(model_pool, 'message_update'):
_logger.debug('Routing mail with Message-Id %s: direct reply to model: %s, thread_id: %s, custom_values: %s, uid: %s',
message_id, model, thread_id, custom_values, uid)
_logger.info('Routing mail from %s to %s with Message-Id %s: direct reply to model: %s, thread_id: %s, custom_values: %s, uid: %s',
email_from, email_to, message_id, model, thread_id, custom_values, uid)
return [(model, thread_id, custom_values, uid)]
# Verify whether this is a reply to a private message
@ -489,8 +491,8 @@ class mail_thread(osv.AbstractModel):
message_ids = self.pool.get('mail.message').search(cr, uid, [('message_id', '=', in_reply_to)], limit=1, context=context)
if message_ids:
message = self.pool.get('mail.message').browse(cr, uid, message_ids[0], context=context)
_logger.debug('Routing mail with Message-Id %s: direct reply to a private message: %s, custom_values: %s, uid: %s',
message_id, message.id, custom_values, uid)
_logger.info('Routing mail from %s to %s with Message-Id %s: direct reply to a private message: %s, custom_values: %s, uid: %s',
email_from, email_to, message_id, message.id, custom_values, uid)
return [(message.model, message.res_id, custom_values, uid)]
# 2. Look for a matching mail.alias entry
@ -517,10 +519,11 @@ class mail_thread(osv.AbstractModel):
# Note: recognized partners will be added as followers anyway
# user_id = self._message_find_user_id(cr, uid, message, context=context)
user_id = uid
_logger.debug('No matching user_id for the alias %s', alias.alias_name)
_logger.info('No matching user_id for the alias %s', alias.alias_name)
routes.append((alias.alias_model_id.model, alias.alias_force_thread_id, \
eval(alias.alias_defaults), user_id))
_logger.debug('Routing mail with Message-Id %s: direct alias match: %r', message_id, routes)
_logger.info('Routing mail from %s to %s with Message-Id %s: direct alias match: %r',
email_from, email_to, message_id, routes)
return routes
# 3. Fallback to the provided parameters, if they work
@ -535,14 +538,14 @@ class mail_thread(osv.AbstractModel):
except:
thread_id = False
assert thread_id and hasattr(model_pool, 'message_update') or hasattr(model_pool, 'message_new'), \
"No possible route found for incoming message with Message-Id %s. " \
"Create an appropriate mail.alias or force the destination model." % message_id
"No possible route found for incoming message from %s to %s (Message-Id %s:)." \
"Create an appropriate mail.alias or force the destination model." % (email_from, email_to, message_id)
if thread_id and not model_pool.exists(cr, uid, thread_id):
_logger.warning('Received mail reply to missing document %s! Ignoring and creating new document instead for Message-Id %s',
thread_id, message_id)
thread_id, message_id)
thread_id = None
_logger.debug('Routing mail with Message-Id %s: fallback to model:%s, thread_id:%s, custom_values:%s, uid:%s',
message_id, model, thread_id, custom_values, uid)
_logger.info('Routing mail from %s to %s with Message-Id %s: fallback to model:%s, thread_id:%s, custom_values:%s, uid:%s',
email_from, email_to, message_id, model, thread_id, custom_values, uid)
return [(model, thread_id, custom_values, uid)]
def message_process(self, cr, uid, model, message, custom_values=None,
@ -592,12 +595,24 @@ 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:
_logger.info('Ignored mail from %s to %s with Message-Id %s:: found duplicated Message-Id during processing',
msg.get('from'), msg.get('to'), msg.get('message_id'))
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', [])

79
addons/mail/tests/test_mail_gateway.py
View File

@ -116,8 +116,11 @@ class TestMailgateway(TestMailBase):
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 mailgateway administrator
self.assertTrue(len(frog_groups) == 1)
self.assertEqual(len(frog_groups), 1, 'message_process: a new mail.group should have been created')
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
self.assertTrue(len(frog_groups) == 1)
# Test: one group created by Raoul
self.assertEqual(len(frog_groups), 1, 'message_process: a new mail.group should have been created')
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')
@ -175,8 +181,8 @@ class TestMailgateway(TestMailBase):
# Do: incoming email from a known partner that is also an user that can create a mail.group
self.res_users.create(cr, uid, {'partner_id': p1id, 'login': 'sylvie', 'groups_id': [(6, 0, [self.group_employee_id])]})
frog_groups = format_and_process(MAIL_TEMPLATE, to='groups@example.com, other@gmail.com')
# Test: one group created by Sylvie
self.assertTrue(len(frog_groups) == 1)
# Test: one group created by Raoul (or Sylvie maybe, if we implement it)
self.assertEqual(len(frog_groups), 1, 'message_process: a new mail.group should have been created')
frog_group = self.mail_group.browse(cr, uid, frog_groups[0])
# Test: one message that is the incoming email
self.assertEqual(len(frog_group.message_ids), 1,
@ -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 <test_raoul@email.com>',
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 <test_raoul@email.com>',
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 <test_raoul@email.com>',
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,23 +293,32 @@ 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='<deadcafe.1337@smtp.agrolait.com>')
frog_groups = format_and_process(MAIL_TEMPLATE_PLAINTEXT,
to='groups@example.com', subject='Frogs Return', extra='',
msg_id='<deadcafe.1337@smtp.agrolait.com>')
# Test: one group created with one message
self.assertTrue(len(frog_groups) == 1)
self.assertEqual(len(frog_groups), 1, 'message_process: a new mail.group should have been created')
frog_group = self.mail_group.browse(cr, uid, frog_groups[0])
msg = frog_group.message_ids[0]
# Test: plain text content should be wrapped and stored as html
@ -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 <group+pigs@example.com>', extra='In-Reply-To: %s' % msg1.message_id)
reply_msg1 = format(MAIL_TEMPLATE, to='Pretty Pigs <group+pigs@example.com>',
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 [<ID>] + 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()