From b5f22227aba047041b8e2d2b6dd6392c2554b2b7 Mon Sep 17 00:00:00 2001 From: Hardik Zinzuvadiya <25708027+Z4nzu@users.noreply.github.com> Date: Sun, 7 Dec 2025 06:38:12 +0000 Subject: [PATCH] feat(utils): replace custom email parsing with RFC-compliant validation (fixes #27337) Replaced the old comma-split based email parsing with `email.utils.getaddresses` to correctly handle RFC 5322 formatted addresses, including display names containing commas (e.g. `"Last, First" `). The previous implementation incorrectly split on commas and treated parts of display names as standalone emails, causing valid inputs to fail validation and breaking Communication creation (e.g. `"Gritton, Howard" `). The new validator: - correctly parses display names with commas - handles multiple addresses and multiline input - skips undisclosed recipients - preserves existing EMAIL_MATCH_PATTERN validation - returns only the email addresses (same as before) Added additional test cases covering RFC-compliant inputs and empty-addr scenarios. Fixes frappe/frappe#27337. --- frappe/tests/test_utils.py | 33 +++++++++++++++++++++ frappe/utils/__init__.py | 59 ++++++++++++++++++-------------------- 2 files changed, 61 insertions(+), 31 deletions(-) diff --git a/frappe/tests/test_utils.py b/frappe/tests/test_utils.py index 475e79e1d5..7c7f68812c 100644 --- a/frappe/tests/test_utils.py +++ b/frappe/tests/test_utils.py @@ -577,6 +577,39 @@ class TestValidationUtils(IntegrationTestCase): "erp+Job%20Applicant=JA00004@frappe.com", ) + # RFC 5322 format - Display name with comma (main bug fix) + self.assertEqual( + validate_email_address('"Lastname, Firstname" '), "test@example.com" + ) + self.assertEqual(validate_email_address('"Doe, John" '), "john.doe@example.com") + + # RFC 5322 format - Display name without comma + self.assertEqual(validate_email_address("Test User "), "test@example.com") + + # RFC 5322 format - Multiple emails + self.assertEqual( + validate_email_address('"Last, First" , "Another, Name" '), + "test1@example.com, test2@example.com", + ) + + # RFC 5322 format - Mixed with plain emails + self.assertEqual( + validate_email_address("Test User , plain@example.com"), + "test@example.com, plain@example.com", + ) + + # Emails with newlines + self.assertEqual( + validate_email_address("test1@example.com\ntest2@example.com"), + "test1@example.com, test2@example.com", + ) + + # Undisclosed recipients should be filtered + self.assertEqual(validate_email_address("undisclosed-recipients:;"), "") + self.assertEqual( + validate_email_address("test@example.com, undisclosed-recipients:;"), "test@example.com" + ) + def test_valid_phone(self): valid_phones = ["+91 1234567890", ""] diff --git a/frappe/utils/__init__.py b/frappe/utils/__init__.py index b5bd7397b7..a729f1a44e 100644 --- a/frappe/utils/__init__.py +++ b/frappe/utils/__init__.py @@ -18,7 +18,7 @@ from collections.abc import ( Sequence, ) from email.header import decode_header, make_header -from email.utils import formataddr, parseaddr +from email.utils import formataddr, getaddresses, parseaddr from typing import Any, Generic, TypeAlias, TypedDict import orjson @@ -180,45 +180,42 @@ def validate_name(name, throw=False): def validate_email_address(email_str, throw=False): """Validates the email string""" - email = email_str = (email_str or "").strip() - def _check(e): - _valid = True - if not e: - _valid = False + email_str = (email_str or "").strip() + out = [] - if "undisclosed-recipient" in e: - return False + # Replace newlines with commas so getaddresses can handle them + # getaddresses expects comma-separated values + email_str = email_str.replace("\n", ",").replace("\r", ",") - elif " " in e and "<" not in e: - # example: "test@example.com test2@example.com" will return "test@example.comtest2" after parseaddr!!! - _valid = False + # Parse using stdlib (handles commas in display names correctly) + addresses = getaddresses([email_str]) - else: - email_id = extract_email_id(e) - match = EMAIL_MATCH_PATTERN.match(email_id) if email_id else None - - if not match: - _valid = False - - if not _valid: + for name, addr in addresses: + if not addr: if throw: - invalid_email = frappe.utils.escape_html(e) frappe.throw( - frappe._("{0} is not a valid Email Address").format(invalid_email), + frappe._("{0} is not a valid Email Address").format( + frappe.utils.escape_html(name or email_str) + ), frappe.InvalidEmailAddressError, ) - return None - else: - return email_id - - out = [] - for e in email_str.split(","): - if not e: continue - email = _check(e.strip()) - if email: - out.append(email) + + # Skip undisclosed recipients + if "undisclosed-recipient" in addr: + continue + + match = EMAIL_MATCH_PATTERN.match(addr) + if not match: + if throw: + frappe.throw( + frappe._("{0} is not a valid Email Address").format(frappe.utils.escape_html(addr)), + frappe.InvalidEmailAddressError, + ) + continue + + out.append(addr) return ", ".join(out)