[REF] mail: remove plus addressing on bounce alias

Starting from this commit bounce addresses set in emails will not use any
plus addressing. They will always be ``bounce_alias#@alias_domain`` and
not ``bounce_alias+<mail_id>-<mail_model>-<mail_res_id>@alias_domain``.

Reasons are

  * this plus addressing adds information we do not use anymore since a long
    time as bounced messages are found using their message ID and not any
    information coming from this bounce address;
  * this generates unnecessary noise;
  * not all email providers really support plus addressing as a mean to
    generate "fake" additional email addresses;

This is the followup of odoo/odoo#71244 where a solution to avoid plus
addressing was done in table. We can now safely remove this feature in
master as a cleaning step.

Task ID-2577328

closes odoo/odoo#73363

Signed-off-by: Thibault Delavallee (tde) <tde@openerp.com>
This commit is contained in:
Thibault Delavallée
2021-07-07 10:36:36 +00:00
parent c8c6e38f97
commit 38bf2c2de5
4 changed files with 10 additions and 91 deletions
+1 -7
View File
@@ -314,15 +314,9 @@ class MailMail(models.Model):
headers = {}
ICP = self.env['ir.config_parameter'].sudo()
bounce_alias = ICP.get_param("mail.bounce.alias")
bounce_alias_static = tools.str2bool(ICP.get_param("mail.bounce.alias.static", "False"))
catchall_domain = ICP.get_param("mail.catchall.domain")
if bounce_alias and catchall_domain:
if bounce_alias_static:
headers['Return-Path'] = '%s@%s' % (bounce_alias, catchall_domain)
elif mail.mail_message_id.is_thread_message():
headers['Return-Path'] = '%s+%d-%s-%d@%s' % (bounce_alias, mail.id, mail.model, mail.res_id, catchall_domain)
else:
headers['Return-Path'] = '%s+%d@%s' % (bounce_alias, mail.id, catchall_domain)
headers['Return-Path'] = '%s@%s' % (bounce_alias, catchall_domain)
if mail.headers:
try:
headers.update(ast.literal_eval(mail.headers))
+2 -11
View File
@@ -873,11 +873,9 @@ class MailThread(models.AbstractModel):
raise TypeError('message must be an email.message.EmailMessage at this point')
catchall_alias = self.env['ir.config_parameter'].sudo().get_param("mail.catchall.alias")
bounce_alias = self.env['ir.config_parameter'].sudo().get_param("mail.bounce.alias")
bounce_alias_static = tools.str2bool(self.env['ir.config_parameter'].sudo().get_param("mail.bounce.alias.static", "False"))
fallback_model = model
# get email.message.Message variables for future processing
local_hostname = socket.gethostname()
message_id = message_dict['message_id']
# compute references to find if message is a reply to an existing thread
@@ -908,20 +906,13 @@ class MailThread(models.AbstractModel):
rcpt_tos_valid_localparts = [to for to in rcpt_tos_localparts]
# 0. Handle bounce: verify whether this is a bounced email and use it to collect bounce data and update notifications for customers
# Bounce regex: typical form of bounce is bounce_alias+128-crm.lead-34@domain
# group(1) = the mail ID; group(2) = the model (if any); group(3) = the record ID
# Bounce alias: if any To contains bounce_alias@domain
# Bounce message (not alias)
# See http://datatracker.ietf.org/doc/rfc3462/?include_text=1
# As all MTA does not respect this RFC (googlemail is one of them),
# we also need to verify if the message come from "mailer-daemon"
# If not a bounce: reset bounce information
if bounce_alias and any(email.startswith(bounce_alias) for email in email_to_localparts):
bounce_re = re.compile("%s\+(\d+)-?([\w.]+)?-?(\d+)?" % re.escape(bounce_alias), re.UNICODE)
bounce_match = bounce_re.search(email_to)
if bounce_match:
self._routing_handle_bounce(message, message_dict)
return []
if bounce_alias and bounce_alias_static and any(email == bounce_alias for email in email_to_localparts):
if bounce_alias and any(email == bounce_alias for email in email_to_localparts):
self._routing_handle_bounce(message, message_dict)
return []
if message.get_content_type() == 'multipart/report' or email_from_localpart == 'mailer-daemon':
+7 -55
View File
@@ -778,69 +778,21 @@ class TestMailgateway(TestMailCommon):
with self.mock_mail_gateway():
new_recs = self.format_and_process(
MAIL_TEMPLATE, self.partner_1.email_formatted,
'%s+%s-%s-%s@%s' % (
self.alias_bounce, self.fake_email.id,
self.fake_email.model, self.fake_email.res_id,
self.alias_domain
),
'%s@%s' % (self.alias_bounce, self.alias_domain),
subject='Should bounce',
)
self.assertFalse(new_recs)
self.assertNotSentEmail()
@mute_logger('odoo.addons.mail.models.mail_thread', 'odoo.models')
def test_message_route_bounce_if_static_but_still_has_plus_addressing(self):
"""Incoming email: bounce using bounce alias without plus addressing: keep old behavior."""
self.env['ir.config_parameter'].set_param('mail.bounce.alias.static', True)
with self.mock_mail_gateway():
new_recs = self.format_and_process(
MAIL_TEMPLATE, self.partner_1.email_formatted,
'%s+%s-%s-%s@%s' % (
self.alias_bounce, self.fake_email.id,
self.fake_email.model, self.fake_email.res_id,
self.alias_domain
),
subject='Should bounce',
)
self.assertFalse(new_recs)
self.assertEqual(len(self._mails), 0, 'message_process: incoming bounce produces no mails')
@mute_logger('odoo.addons.mail.models.mail_thread', 'odoo.models')
def test_message_route_bounce_if_static_without_plus_addressing(self):
"""Incoming email: bounce using bounce alias without plus addressing: bounce it."""
self.env['ir.config_parameter'].set_param('mail.bounce.alias.static', True)
with self.mock_mail_gateway():
new_recs = self.format_and_process(
MAIL_TEMPLATE, self.partner_1.email_formatted,
'%s@%s' % (self.alias_bounce, self.alias_domain),
subject='Should bounce',
)
self.assertFalse(new_recs)
self.assertEqual(len(self._mails), 0, 'message_process: incoming bounce produces no mails')
@mute_logger('odoo.addons.mail.models.mail_thread', 'odoo.models')
def test_message_route_no_bounce_if_not_static_without_plus_addressing(self):
"""Incoming email: bounce using bounce alias without plus addressing: raise as
considering as a direct write to bounce alias -> invalid """
self.env['ir.config_parameter'].set_param('mail.bounce.alias.static', False)
with self.assertRaises(ValueError):
self.format_and_process(
MAIL_TEMPLATE, self.partner_1.email_formatted,
'%s@%s' % (self.alias_bounce, self.alias_domain),
subject="Should fail because it is not a bounce and there's no alias",
)
@mute_logger('odoo.addons.mail.models.mail_thread', 'odoo.models')
def test_message_route_bounce_other_recipients(self):
"""Incoming email: bounce processing: bounce should be computed even if not first recipient """
with self.mock_mail_gateway():
new_recs = self.format_and_process(
MAIL_TEMPLATE, self.partner_1.email_formatted,
'%s@%s, %s+%s-%s-%s@%s' % (
'%s@%s, %s@%s' % (
self.alias.alias_name, self.alias_domain,
self.alias_bounce, self.fake_email.id,
self.fake_email.model, self.fake_email.res_id,
self.alias_domain
self.alias_bounce, self.alias_domain
),
subject='Should bounce',
)
@@ -896,7 +848,7 @@ class TestMailgateway(TestMailCommon):
self.assertEqual(self.test_record.message_bounce, 0)
bounced_mail_id = 4442
bounce_email_to = '%s+%s-%s-%s@%s' % ('bounce.test', bounced_mail_id, self.test_record._name, self.test_record.id, 'test.com')
bounce_email_to = '%s@%s' % ('bounce.test', 'test.com')
record = self.format_and_process(MAIL_TEMPLATE, self.partner_1.email_formatted, bounce_email_to, subject='Undelivered Mail Returned to Sender')
self.assertFalse(record)
# No information found in bounce email -> not possible to do anything except avoiding email
@@ -922,7 +874,7 @@ class TestMailgateway(TestMailCommon):
self.assertEqual(self.test_record.message_bounce, 0)
bounced_mail_id = 4442
bounce_email_to = '%s+%s-%s-%s@%s' % ('bounce.test', bounced_mail_id, self.test_record._name, self.test_record.id, 'test.com')
bounce_email_to = '%s@%s' % ('bounce.test', 'test.com')
record = self.format_and_process(test_mail_data.MAIL_BOUNCE, self.partner_1.email_formatted, bounce_email_to, subject='Undelivered Mail Returned to Sender')
self.assertFalse(record)
# Missing in reply to message_id -> cannot find original record
@@ -936,7 +888,7 @@ class TestMailgateway(TestMailCommon):
self.assertEqual(self.test_record.message_bounce, 0)
bounced_mail_id = 4442
bounce_email_to = '%s+%s-%s-%s@%s' % ('bounce.test', bounced_mail_id, self.test_record._name, self.test_record.id, 'test.com')
bounce_email_to = '%s@%s' % ('bounce.test', 'test.com')
extra = self.fake_email.message_id
record = self.format_and_process(test_mail_data.MAIL_BOUNCE, self.partner_1.email_formatted, bounce_email_to, subject='Undelivered Mail Returned to Sender', extra=extra)
self.assertFalse(record)
@@ -950,7 +902,7 @@ class TestMailgateway(TestMailCommon):
self.assertEqual(self.test_record.message_bounce, 0)
bounced_mail_id = 4442
bounce_email_to = '%s+%s-%s-%s@%s' % ('bounce.test', bounced_mail_id, self.test_record._name, self.test_record.id, 'test.com')
bounce_email_to = '%s@%s' % ('bounce.test', 'test.com')
extra = self.fake_email.message_id
record = self.format_and_process(test_mail_data.MAIL_BOUNCE, 'Whatever <what@ever.com>', bounce_email_to, subject='Undelivered Mail Returned to Sender', extra=extra)
self.assertFalse(record)
-18
View File
@@ -42,24 +42,6 @@ class TestMailMail(TestMailCommon):
'email_to': 'test@example.com',
}
mail = self.env['mail.mail'].create(base_values)
with self.mock_mail_gateway():
mail.send()
self.assertEqual(self._mails[0]['headers']['Return-Path'], '%s+%d@%s' % (self.alias_bounce, mail.id, self.alias_domain))
# mail on thread-enabled record
mail = self.env['mail.mail'].create(dict(base_values, **{
'model': self.test_record._name,
'res_id': self.test_record.id,
}))
with self.mock_mail_gateway():
mail.send()
self.assertEqual(self._mails[0]['headers']['Return-Path'], '%s+%d-%s-%s@%s' % (self.alias_bounce, mail.id, self.test_record._name, self.test_record.id, self.alias_domain))
# force static addressing on bounce alias
self.env['ir.config_parameter'].set_param('mail.bounce.alias.static', True)
# mail without thread-enabled record
mail = self.env['mail.mail'].create(base_values)
with self.mock_mail_gateway():
mail.send()