From d4816d740ebbb9e1261bebd9d3c0ac57ea37c80e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Mon, 24 Jan 2022 14:39:48 +0000 Subject: [PATCH] [FIX] mail: limit reply-to name part Python library currently holds a limitation when we use a formatted email that is longer than 78 characters (e.g. "Long Company Name With Long Record Name "). Python folds address if is longer than 78 chars and a bad management of quotes breaks the reply-to. Even if anything should technically be ok with the RFC python seems to incorrectly handle it (please refer to [1] for more details and discussions). Until this is finally sorted out we decided to avoid issues by shortening reply-to. 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. Task-2602862 OPW-2733513 [1] See https://bugs.python.org/issue44637 closes odoo/odoo#89081 X-original-commit: d2d705708a11de68d8dd9d9edf62e0a33ddb066b Signed-off-by: Thibault Delavallee (tde) --- addons/mail/models/models.py | 37 +++++++- addons/test_mail/tests/test_mail_message.py | 95 +++++++++++++++++---- 2 files changed, 112 insertions(+), 20 deletions(-) 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):