Support RFC6530 on email formatting

We have found emails with ç character producing the following traceback:

UnicodeEncodeError: 'ascii' codec can't encode character '\xe7' in position 8: ordinal not in range(128)
  File "trytond/protocols/dispatcher.py", line 186, in _dispatch
    result = rpc.result(meth(inst, *c_args, **c_kwargs))
  File "trytond/modules/party/ir.py", line 47, in get
    return super().get(record)
  File "trytond/ir/email_.py", line 419, in get
    values[key] = self.get_addresses(value)
  File "trytond/ir/email_.py", line 524, in get_addresses
    for name, email in filter(None, addresses)
  File "trytond/ir/email_.py", line 525, in <listcomp>
    if email]
  File "trytond/ir/email_.py", line 58, in _formataddr
    address.encode('ascii')

added trytond type::crash + 1 deleted label

I would say it is a feature to support RFC6530

added type::feature label and removed type::crash label

By a feature you mean that we should validate that the address follows the RFC before allowing to save them on Tryton?

In such case, probably it will be better to delegate the validation to an external librarly, like email-validator

No I mean that having non-ASCII in email is an extension (RFC6530) of the email standard so supporting it is a new feature (not a bug).

From section7.1 I see that utf-8 can be used as fallback when the address is non ascii (and the SMTP server should take care of handling that or reject the message if its not supported).

But I see this is already supported by the email message:

>>> from email.message import Message
>>> msg = Message()
>>> msg['from'] = 'adreç@kopen.es'
>>> msg.as_string()
'from: =?utf-8?b?YWRyZcOnQGtvcGVuLmVz?=\n\n'
>>> msg = Message()
>>> msg['from'] = 'adreca@kopen.es'
>>> msg.as_string()
'from: adreca@kopen.es\n\n'

Also the returned encoded address is never used on our source code.

So for me we should consider this as a bug, and remove the enconding. Here is review358591002 which implements it.

assigned to @pokoli

added type::crash + 1 deleted label and removed type::feature + 1 deleted label

New review358591002 at https://codereview.tryton.org/358591002/#ps369431002

I do not see how the requirement of a RFC6530 that is not explicitly stated as supported qualify the non-support as a bug.

added type::feature label and removed type::crash label

What can qualify as a bug is that the exception is not catch or the decode does not ignore non-ascii.

For me it's a bug because it crashes when it should not, that's why added the backport flag, as this crash is reproducible on released series. Should I create a new review to catch the error? I do not see so much difference with the current proposal because the address field will be used without the encoding, so the behaviour will be exactly the same.

This requires further analysis. We do not just fix crash by hiding them. The encoding is there because normally email address should be ASCII (if RFC6530 is not implemented).

El 11/8/21 a les 11:40, Cédric Krier ha escrit:

The encoding is there because normally email address should be ASCII (if RFC6530 is not implemented). The RFC says that if there is some server not supporting the standard the email should be returned to the sender so the email can be upgraded to a valid form.

With this fact I do not think we should do anything in Tryton, just delegate the handling to the SMTP server.

review358591002 updated at https://codereview.tryton.org/358591002/#ps373191003

Indeed the encoding reference in the comment is related to the Python standard email.utils.formataddr which takes a charset parameter to encode the name part which we do not want when displaying to user for selection. The problem is that _formataddr is based on email.utils.formataddr to replicate the same behavior except for encoding. And it also checks that the address is ASCII: https://github.com/python/cpython/blob/main/Lib/email/utils.py#L76. Fixing only in Tryton's _formataddr will not solve the problem because latter the email.utils.formataddr will be used any and raise the same exception. So for me for now we can only prevent the crash when retrieving the addresses by ignoring the non-ASCII emails (and this is a new bug fix that can be backported). And in a second time we must push a fix to Python to support RFC6530 for non-ASCII emails before we can remove the same constraint.

I filled #11261 (closed) with the bugfix to be backported

unassigned @pokoli

added 1 deleted label and removed 1 deleted label

Indeed if email name contains ç (or accents, ñ, ...) the smtp server returns error.

mentioned in issue #11261 (closed)

mentioned in commit c7b9a8fd8864

assigned to @ced

mentioned in commit a4db957039d5

closed with merge request !494 (merged)