diff --git a/addons/mail/models/models.py b/addons/mail/models/models.py index 6d762f5ff5c..11781fc53f9 100644 --- a/addons/mail/models/models.py +++ b/addons/mail/models/models.py @@ -138,11 +138,11 @@ class BaseModel(models.AbstractModel): if catchall: result_email.update(dict((rid, '%s@%s' % (catchall, alias_domain)) for rid in left_ids)) - # compute name of reply-to - TDE tocheck: quotes and stuff like that - company_name = self.env.company.name for res_id in result_email: - name = '%s%s%s' % (company_name, ' ' if doc_names.get(res_id) else '', doc_names.get(res_id, '')) - result[res_id] = tools.formataddr((name, result_email[res_id])) + result[res_id] = self._notify_get_reply_to_formatted_email( + result_email[res_id], + doc_names.get(res_id) or '', + ) left_ids = set(_res_ids) - set(result_email) if left_ids: @@ -150,6 +150,35 @@ class BaseModel(models.AbstractModel): return result + def _notify_get_reply_to_formatted_email(self, record_email, record_name): + """ Compute formatted email for reply_to and try to avoid refold issue + with python that splits the reply-to over multiple lines. It is due to + 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). + + 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 + 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. + """ + # address itself is too long for 78 chars limit: return only email + if len(record_email) >= 78: + return record_email + + company_name = self.env.company.name + + # try company_name + record_name, or record_name alone (or company_name alone) + name = f"{company_name} {record_name}" if record_name else company_name + + formatted_email = tools.formataddr((name, record_email)) + if len(formatted_email) > 78: + formatted_email = tools.formataddr((record_name or company_name, record_email)) + if len(formatted_email) > 78: + formatted_email = record_email + return formatted_email + # ------------------------------------------------------------ # ALIAS MANAGEMENT # ------------------------------------------------------------ diff --git a/addons/test_mail/tests/test_mail_message.py b/addons/test_mail/tests/test_mail_message.py index 0ebbc5e9f05..2f6410c552e 100644 --- a/addons/test_mail/tests/test_mail_message.py +++ b/addons/test_mail/tests/test_mail_message.py @@ -9,9 +9,10 @@ from odoo.addons.test_mail.tests.common import TestMailCommon from odoo.addons.test_mail.models.test_mail_models import MailTestSimple from odoo.exceptions import AccessError from odoo.tools import mute_logger, formataddr -from odoo.tests import tagged +from odoo.tests import tagged, users +@tagged('mail_message') class TestMessageValues(TestMailCommon): @classmethod @@ -59,8 +60,79 @@ class TestMessageValues(TestMailCommon): res = message.with_user(self.user_employee).message_format() self.assertEqual(res[0].get('record_name'), 'Test1') + def test_mail_message_values_body_base64_image(self): + msg = self.env['mail.message'].with_user(self.user_employee).create({ + 'body': 'taratata ', + }) + self.assertEqual(len(msg.attachment_ids), 1) + self.assertEqual( + msg.body, + '

taratata image0 ' + 'image0

'.format(attachment=msg.attachment_ids[0]) + ) + @mute_logger('odoo.models.unlink') - def test_mail_message_values_no_document_values(self): + @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 + + 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) + test_record.write({ + 'name': 'Super Long Name That People May Enter "Even with an internal quoting of stuff"' + }) + msg = self.env['mail.message'].create({ + 'model': test_record._name, + 'res_id': test_record.id + }) + 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') + + # name + company_name would make it blow up: keep record_name in formatting + test_record.write({'name': 'Name that would be more than 78 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') + + # no record_name: keep company_name in formatting if ok + test_record.write({'name': ''}) + msg = self.env['mail.message'].create({ + 'model': test_record._name, + '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') + + # 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"'}) + msg = self.env['mail.message'].create({ + 'model': test_record._name, + 'res_id': test_record.id + }) + self.assertEqual(msg.reply_to, reply_to_email, + 'Reply-To: use only email when formataddr > 78 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', + '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' + msg = self.env['mail.message'].create({ + 'model': test_record._name, + 'res_id': test_record.id + }) + self.assertEqual(msg.reply_to, f"{sanitized_alias_name}@{self.alias_domain}", + 'Reply-To: even a long email is ok as only formataddr is problematic') + + @mute_logger('odoo.models.unlink') + def test_mail_message_values_fromto_no_document_values(self): msg = self.Message.create({ 'reply_to': 'test.reply@example.com', 'email_from': 'test.from@example.com', @@ -70,7 +142,7 @@ class TestMessageValues(TestMailCommon): self.assertEqual(msg.email_from, 'test.from@example.com') @mute_logger('odoo.models.unlink') - def test_mail_message_values_no_document(self): + def test_mail_message_values_fromto_no_document(self): msg = self.Message.create({}) self.assertIn('-private', msg.message_id.split('@')[0], 'mail_message: message_id for a void message should be a "private" one') reply_to_name = self.env.user.company_id.name @@ -96,7 +168,7 @@ class TestMessageValues(TestMailCommon): self.assertEqual(msg.email_from, formataddr((self.user_employee.name, self.user_employee.email))) @mute_logger('odoo.models.unlink') - def test_mail_message_values_document_alias(self): + def test_mail_message_values_fromto_document_alias(self): msg = self.Message.create({ 'model': 'mail.test.container', 'res_id': self.alias_record.id @@ -133,7 +205,7 @@ class TestMessageValues(TestMailCommon): self.assertEqual(msg.email_from, formataddr((self.user_employee.name, self.user_employee.email))) @mute_logger('odoo.models.unlink') - def test_mail_message_values_document_no_alias(self): + def test_mail_message_values_fromto_document_no_alias(self): test_record = self.env['mail.test.simple'].create({'name': 'Test', 'email_from': 'ignasse@example.com'}) msg = self.Message.create({ @@ -147,7 +219,7 @@ class TestMessageValues(TestMailCommon): self.assertEqual(msg.email_from, formataddr((self.user_employee.name, self.user_employee.email))) @mute_logger('odoo.models.unlink') - def test_mail_message_values_document_manual_alias(self): + def test_mail_message_values_fromto_document_manual_alias(self): test_record = self.env['mail.test.simple'].create({'name': 'Test', 'email_from': 'ignasse@example.com'}) alias = self.env['mail.alias'].create({ 'alias_name': 'MegaLias', @@ -168,7 +240,7 @@ class TestMessageValues(TestMailCommon): self.assertEqual(msg.reply_to, formataddr((reply_to_name, reply_to_email))) self.assertEqual(msg.email_from, formataddr((self.user_employee.name, self.user_employee.email))) - def test_mail_message_values_reply_to_force_new(self): + def test_mail_message_values_fromto_reply_to_force_new(self): msg = self.Message.create({ 'model': 'mail.test.container', 'res_id': self.alias_record.id, @@ -178,15 +250,6 @@ class TestMessageValues(TestMailCommon): self.assertNotIn('mail.test.container', msg.message_id.split('@')[0]) self.assertNotIn('-%d-' % self.alias_record.id, msg.message_id.split('@')[0]) - def test_mail_message_base64_image(self): - msg = self.env['mail.message'].with_user(self.user_employee).create({ - 'body': 'taratata ', - }) - self.assertEqual(len(msg.attachment_ids), 1) - body = '

taratata image0 image0

' - body = body % (msg.attachment_ids[0].id, msg.attachment_ids[0].access_token, msg.attachment_ids[0].id, msg.attachment_ids[0].access_token) - self.assertEqual(msg.body, body) - class TestMessageAccess(TestMailCommon):