[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 <email@domain.com>"). 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) <tde@openerp.com>
This commit is contained in:
Thibault Delavallée
2022-04-19 19:40:14 +02:00
parent 5472bec197
commit d4816d740e
2 changed files with 112 additions and 20 deletions
+33 -4
View File
@@ -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
# ------------------------------------------------------------
+79 -16
View File
@@ -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 <img src="data:image/png;base64,iV/+OkI=" width="2"> <img src="data:image/png;base64,iV/+OkI=" width="2">',
})
self.assertEqual(len(msg.attachment_ids), 1)
self.assertEqual(
msg.body,
'<p>taratata <img src="/web/image/{attachment.id}?access_token={attachment.access_token}" alt="image0" width="2"> '
'<img src="/web/image/{attachment.id}?access_token={attachment.access_token}" alt="image0" width="2"></p>'.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 <img src="data:image/png;base64,iV/+OkI=" width="2"> <img src="data:image/png;base64,iV/+OkI=" width="2">',
})
self.assertEqual(len(msg.attachment_ids), 1)
body = '<p>taratata <img src="/web/image/%s?access_token=%s" alt="image0" width="2"> <img src="/web/image/%s?access_token=%s" alt="image0" width="2"></p>'
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):