From 64aae8bce1725137aafc5d451bccde619773feae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thibault=20Delavall=C3=A9e?= Date: Thu, 31 Aug 2023 16:58:11 +0200 Subject: [PATCH] [FIX] tools, base, mail: add a fallback when parsing wrongly-formatted emails With input 'name email@domain.com' (missing chevrons allowing to clearly spot the email part) 'getaddresses' returns ('', 'name email@domain.com) i.e. the whole input is considered as being the email. To improve the heuristic we can add a fallback by recalling 'getadresses' on the input with spaces replaced by commas when it found only an email and no name. The new email will be split into sub pairs allowing to find the real email and various name parts, allowing to make a new name / email pair. Emails should not contain spaces thus this is coherent with email formation. This fallback actually comes from a specific code done in '_parse_partner_name' of Partner model. Supporting it directly at tools level make the behavior coherent for all models. Task-2612945 (Mail: Defensive email formatting) X-original-commit: odoo/odoo@18c71edf59fadeefa70faa1acf244bd53efe41da Part-of: odoo/odoo#134934 --- addons/mail/tests/test_res_partner.py | 4 +- .../test_mass_mailing/tests/test_mailing.py | 6 +-- odoo/addons/base/tests/test_mail.py | 20 +++++----- odoo/tools/mail.py | 39 +++++++++++++------ 4 files changed, 42 insertions(+), 27 deletions(-) diff --git a/addons/mail/tests/test_res_partner.py b/addons/mail/tests/test_res_partner.py index 8ea3d294989..d2adfacc70a 100644 --- a/addons/mail/tests/test_res_partner.py +++ b/addons/mail/tests/test_res_partner.py @@ -372,8 +372,8 @@ class TestPartner(MailCommon): # multi email support (False, "Another Customer", "test.different.1@test.dupe.example.com"), (False, "Multi Email", "other.customer.2@test.dupe.example.com"), - (False, "Invalid, Multi Format", "other.customer.😊@test.dupe.example.com"), - (False, "Invalid, Multi Format", "other.customer.😊@test.dupe.example.com"), + (False, "Multi Format", "other.customer.😊@test.dupe.example.com"), + (False, "Multi Format", "other.customer.😊@test.dupe.example.com"), ] with self.mockPartnerCalls(): new_partners = self.env['res.partner'].with_context(lang='en_US')._find_or_create_from_emails( diff --git a/addons/test_mass_mailing/tests/test_mailing.py b/addons/test_mass_mailing/tests/test_mailing.py index 079b1bded80..04df62f2e2a 100644 --- a/addons/test_mass_mailing/tests/test_mailing.py +++ b/addons/test_mass_mailing/tests/test_mailing.py @@ -243,7 +243,7 @@ class TestMassMailing(TestMassMailCommon): 'partner': customer_weird, 'trace_status': 'sent'}, # concatenates everything after domain {'email': 'Weird Format2 test.customer.weird.2@example.com', - 'email_to_recipients': [[f'"{customer_weird_2.name}" ']], + 'email_to_recipients': [[f'"{customer_weird_2.name}" ']], 'failure_type': False, 'partner': customer_weird_2, 'trace_status': 'sent'}, @@ -273,8 +273,8 @@ class TestMassMailing(TestMassMailCommon): 'failure_type': False, 'trace_status': 'sent'}, {'email': 'test.record.weird.2@example.com', - 'email_to_mail': 'weird format2 test.record.weird.2@example.com', - 'email_to_recipients': [['weird format2 test.record.weird.2@example.com']], + 'email_to_mail': 'test.record.weird.2@example.com', + 'email_to_recipients': [['test.record.weird.2@example.com']], 'failure_type': False, 'trace_status': 'sent'}, ], diff --git a/odoo/addons/base/tests/test_mail.py b/odoo/addons/base/tests/test_mail.py index 53473ca0e3c..a45cf31ce1d 100644 --- a/odoo/addons/base/tests/test_mail.py +++ b/odoo/addons/base/tests/test_mail.py @@ -517,7 +517,7 @@ class TestEmailTools(BaseCase): ] expected_list = [ 'deboulonneur@example.com', - 'Déboulonneur deboulonneur@example.com', + 'deboulonneur@example.com', 'deboulonneur@example.comdéboulonneur', False, '@example.com', # funny @@ -527,7 +527,7 @@ class TestEmailTools(BaseCase): ] expected_fmt_utf8_list = [ f'"{format_name}" ', - f'"{format_name}" ', + f'"{format_name}" ', f'"{format_name}" ', f'"{format_name}" <@>', f'"{format_name}" <@example.com>', @@ -537,7 +537,7 @@ class TestEmailTools(BaseCase): ] expected_fmt_ascii_list = [ f'{format_name_ascii} ', - f'{format_name_ascii} ', + f'{format_name_ascii} ', f'{format_name_ascii} ', f'{format_name_ascii} <@>', f'{format_name_ascii} <@example.com>', @@ -624,7 +624,7 @@ class TestEmailTools(BaseCase): ['"Super Déboulonneur" '], # wrong formatting ['"Déboulonneur" '], - ['Déboulonneur deboulonneur@example.com'], # returned as it, a bit strange but hey + ['"Déboulonneur" '], # extra part correctly considered as a name ['deboulonneur@example.comDéboulonneur'], # concatenated, not sure why # multi ['deboulonneur@example.com'], @@ -657,14 +657,14 @@ class TestEmailTools(BaseCase): [('Fredo The Great', 'alfred.astaire@test.example.com'), ('Evelyne The Goat', 'evelyne.gargouillis@test.example.com')], [('Fredo The Great', 'alfred.astaire@test.example.com'), ('', 'evelyne.gargouillis@test.example.com')], [('Fredo The Great', 'alfred.astaire@test.example.com'), ('', 'evelyne.gargouillis@test.example.com')], - # text containing email -> probably not designed for that - [('', 'Hello alfred.astaire@test.example.comhowareyou?')], - [('', 'Hello alfred.astaire@test.example.com')], - # text containing emails -> probably not designed for that + # text containing email -> fallback on parsing to extract text from email + [('Hello', 'alfred.astaire@test.example.comhowareyou?')], + [('Hello', 'alfred.astaire@test.example.com')], [('Hello Fredo', 'alfred.astaire@test.example.com'), ('', 'evelyne.gargouillis@test.example.com')], - [('Hello Fredo', 'alfred.astaire@test.example.com'), ('', 'and evelyne.gargouillis@test.example.com')], + [('Hello Fredo', 'alfred.astaire@test.example.com'), ('and', 'evelyne.gargouillis@test.example.com')], # falsy -> probably not designed for that - [], [('', "j'adore écrire des@gmail.comou"), ('', '@gmail.com')], [], + [], + [('j\'adore écrire', "des@gmail.comou"), ('', '@gmail.com')], [], ] for src, exp in zip(self.sources, expected): diff --git a/odoo/tools/mail.py b/odoo/tools/mail.py index 850470b1196..1c9881306ae 100644 --- a/odoo/tools/mail.py +++ b/odoo/tools/mail.py @@ -526,14 +526,35 @@ def email_split_tuples(text): """ Return a list of (name, email) address tuples found in ``text`` . Note that text should be an email header or a stringified email list as it may give broader results than expected on actual text. """ + def _parse_based_on_spaces(pair): + """ With input 'name email@domain.com' (missing quotes for a formatting) + getaddresses returns ('', 'name email@domain.com). This when having no + name and an email a fallback to enhance parsing is to redo a getaddresses + by replacing spaces by commas. The new email will be split into sub pairs + allowing to find the email and name parts, allowing to make a new name / + email pair. Emails should not contain spaces thus this is coherent with + email formation. """ + name, email = pair + if not name and email and ' ' in email: + inside_pairs = getaddresses([email.replace(' ', ',')]) + name_parts, found_email = [], False + for pair in inside_pairs: + if pair[1] and '@' not in pair[1]: + name_parts.append(pair[1]) + if pair[1] and '@' in pair[1]: + found_email = pair[1] + name, email = (' '.join(name_parts), found_email) if found_email else (name, email) + return (name, email) + if not text: return [] - return [(addr[0], addr[1]) for addr in getaddresses([text]) - # getaddresses() returns '' when email parsing fails, and - # sometimes returns emails without at least '@'. The '@' - # is strictly required in RFC2822's `addr-spec`. - if addr[1] - if '@' in addr[1]] + return list(map(_parse_based_on_spaces, [ + (addr[0], addr[1]) for addr in getaddresses([text]) + # getaddresses() returns '' when email parsing fails, and + # sometimes returns emails without at least '@'. The '@' + # is strictly required in RFC2822's `addr-spec`. + if addr[1] and '@' in addr[1] + ])) def email_split(text): """ Return a list of the email addresses found in ``text`` """ @@ -736,12 +757,6 @@ def parse_contact_from_email(text): split_results = email_split_tuples(text) name, email = split_results[0] if split_results else ('', '') - if email and not name: - fallback_emails = email_split(text.replace(' ', ',')) - if fallback_emails: - email = fallback_emails[0] - name = text[:text.index(email)].replace('"', '').replace('<', '').strip() - if email: email_normalized = email_normalize(email, strict=False) or email else: