diff --git a/addons/mail/models/models.py b/addons/mail/models/models.py index 3da323376b8..ddfbb9a4b11 100644 --- a/addons/mail/models/models.py +++ b/addons/mail/models/models.py @@ -8,6 +8,9 @@ from markupsafe import Markup from odoo import api, models, tools, _ from odoo.addons.mail.tools.alias_error import AliasError +import logging + +_logger = logging.getLogger(__name__) class BaseModel(models.AbstractModel): _inherit = 'base' @@ -290,9 +293,13 @@ class BaseModel(models.AbstractModel): a bad management of quotes (missing quotes after refold). This appears therefore only when having quotes (aka not simple names, and not when being unicode encoded). + Another edge-case produces a linebreak (CRLF) immediately after the + colon character separating the header name from the header value. + This creates an issue in certain DKIM tech stacks that will + incorrectly read the reply-to value as empty and fail the verification. - To avoid that issue when formataddr would return more than 78 chars we - return a simplified name/email to try to stay under 78 chars. If not + To avoid that issue when formataddr would return more than 68 chars we + return a simplified name/email to try to stay under 68 chars. If not possible we return only the email and skip the formataddr which causes the issue in python. We do not use hacks like crop the name part as encoding and quoting would be error prone. @@ -301,8 +308,14 @@ class BaseModel(models.AbstractModel): complete name in formataddr. Otherwise fallback on 'company_id' of self or environment company; """ - # address itself is too long for 78 chars limit: return only email - if len(record_email) >= 78: + length_limit = 68 # 78 - len('Reply-To: '), 78 per RFC + # address itself is too long : return only email and log warning + if len(record_email) >= length_limit: + _logger.warning('Notification email address for reply-to is longer than 68 characters. ' + 'This might create non-compliant folding in the email header in certain DKIM ' + 'verification tech stacks. It is advised to shorten it if possible. ' + 'Record name (if set): %s ' + 'Reply-To: %s ', record_name, record_email) return record_email if not company: @@ -315,9 +328,9 @@ class BaseModel(models.AbstractModel): name = f"{company.name} {record_name}" if record_name else company.name formatted_email = tools.formataddr((name, record_email)) - if len(formatted_email) > 78: + if len(formatted_email) > length_limit: formatted_email = tools.formataddr((record_name or company.name, record_email)) - if len(formatted_email) > 78: + if len(formatted_email) > length_limit: formatted_email = record_email return formatted_email diff --git a/addons/test_mail/tests/test_mail_gateway.py b/addons/test_mail/tests/test_mail_gateway.py index 9c09aa26afe..bc16c9296fe 100644 --- a/addons/test_mail/tests/test_mail_gateway.py +++ b/addons/test_mail/tests/test_mail_gateway.py @@ -1557,6 +1557,10 @@ class TestMailgateway(MailCommon): @mute_logger('odoo.addons.mail.models.mail_thread', 'odoo.models.unlink', 'odoo.addons.mail.models.mail_mail') def test_message_process_reply_to_new_thread(self): """ Test replies not being considered as replies but use destination information instead (aka, mass post + specific reply to using aliases) """ + # shorten company name to prevent 68 character formatting from + # triggering and making the assert missmatch. + # See _notify_get_reply_to_formatted_email method + self.user_employee.company_id.name = "Forced" first_record = self.env['mail.test.simple'].with_user(self.user_employee).create({'name': 'Replies to Record'}) record_msg = first_record.message_post( subject='Discussion', diff --git a/addons/test_mail/tests/test_mail_message.py b/addons/test_mail/tests/test_mail_message.py index 32452e8b9aa..ca74c97c9d8 100644 --- a/addons/test_mail/tests/test_mail_message.py +++ b/addons/test_mail/tests/test_mail_message.py @@ -141,11 +141,12 @@ class TestMessageValues(MailCommon): 'image0

'.format(attachment=msg.attachment_ids[0]) ) - @mute_logger('odoo.models.unlink') + @mute_logger('odoo.models.unlink', 'odoo.addons.mail.models.models') @users('employee') def test_mail_message_values_fromto_long_name(self): - """ Long headers may break in python if above 78 chars as folding is not - done correctly (see ``_notify_get_reply_to_formatted_email`` docstring + """ Long headers may break in python if above 68 chars for certain + DKIM verification stacks as folding is not done correctly + (see ``_notify_get_reply_to_formatted_email`` docstring + commit linked to this test). """ # name would make it blow up: keep only email test_record = self.env['mail.test.container'].browse(self.alias_record.ids) @@ -158,17 +159,17 @@ class TestMessageValues(MailCommon): }) reply_to_email = f"{test_record.alias_name}@{self.alias_domain}" self.assertEqual(msg.reply_to, reply_to_email, - 'Reply-To: use only email when formataddr > 78 chars') + 'Reply-To: use only email when formataddr > 68 chars') # name + company_name would make it blow up: keep record_name in formatting self.company_admin.name = "Company name being about 33 chars" - test_record.write({'name': 'Name that would be more than 78 with company name'}) + test_record.write({'name': 'Being more than 68 with company name'}) msg = self.env['mail.message'].create({ 'model': test_record._name, 'res_id': test_record.id }) self.assertEqual(msg.reply_to, formataddr((test_record.name, reply_to_email)), - 'Reply-To: use recordname as name in format if recordname + company > 78 chars') + 'Reply-To: use recordname as name in format if recordname + company > 68 chars') # no record_name: keep company_name in formatting if ok test_record.write({'name': ''}) @@ -177,7 +178,7 @@ class TestMessageValues(MailCommon): 'res_id': test_record.id }) self.assertEqual(msg.reply_to, formataddr((self.env.user.company_id.name, reply_to_email)), - 'Reply-To: use company as name in format when no record name and still < 78 chars') + 'Reply-To: use company as name in format when no record name and still < 68 chars') # no record_name and company_name make it blow up: keep only email self.env.user.company_id.write({'name': 'Super Long Name That People May Enter "Even with an internal quoting of stuff"'}) @@ -186,15 +187,15 @@ class TestMessageValues(MailCommon): 'res_id': test_record.id }) self.assertEqual(msg.reply_to, reply_to_email, - 'Reply-To: use only email when formataddr > 78 chars') + 'Reply-To: use only email when formataddr > 68 chars') # whatever the record and company names, email is too long: keep only email test_record.write({ - 'alias_name': 'Waaaay too long alias name that should make any reply-to blow the 78 characters limit', + 'alias_name': 'Waaaay too long alias name that should make any reply-to blow the 68 characters limit', 'name': 'Short', }) self.env.user.company_id.write({'name': 'Comp'}) - sanitized_alias_name = 'waaaay-too-long-alias-name-that-should-make-any-reply-to-blow-the-78-characters-limit' + sanitized_alias_name = 'waaaay-too-long-alias-name-that-should-make-any-reply-to-blow-the-68-characters-limit' msg = self.env['mail.message'].create({ 'model': test_record._name, 'res_id': test_record.id diff --git a/addons/test_mail/tests/test_performance.py b/addons/test_mail/tests/test_performance.py index 40c3a49cc82..5d7d62e7576 100644 --- a/addons/test_mail/tests/test_performance.py +++ b/addons/test_mail/tests/test_performance.py @@ -739,10 +739,13 @@ class TestBaseAPIPerformance(BaseMailPerformance): @users('admin', 'employee') @warmup def test_notification_reply_to_batch(self): + # overwrite company name to keep it short/simple + # and not trigger the 68 character reply_to formatting + self.env.user.company_id.name = "Forced" test_records_sudo = self.env['mail.test.container'].sudo().create([ - {'alias_name': 'alias.test.%s.%d' % (self.env.user.name, index), + {'alias_name': 'a.%s.%d' % (self.env.user.name, index), 'customer_id': self.customer.id, - 'name': 'Test_%d' % index, + 'name': 'T_%d' % index, } for index in range(10) ])