Backends: identify source of problem in AnymailInvalidAddress message

Include the name of the field with the the unparsable email address
in AnymailInvalidAddress error messages.

Should help tracking down problems like in #98.
This commit is contained in:
medmunds
2018-04-11 11:50:06 -07:00
parent f0d744a796
commit 5598c87e62
3 changed files with 55 additions and 11 deletions

View File

@@ -268,7 +268,11 @@ class BasePayload(object):
if converter is not None: if converter is not None:
if not callable(converter): if not callable(converter):
converter = getattr(self, converter) converter = getattr(self, converter)
value = converter(value) if converter in (parse_address_list, parse_single_address):
# hack to include field name in error message
value = converter(value, field=attr)
else:
value = converter(value)
if value is not UNSET: if value is not UNSET:
if attr == 'body': if attr == 'body':
setter = self.set_html_body if message.content_subtype == 'html' else self.set_text_body setter = self.set_html_body if message.content_subtype == 'html' else self.set_text_body
@@ -296,7 +300,7 @@ class BasePayload(object):
# message.extra_headers['Reply-To'] will override message.reply_to # message.extra_headers['Reply-To'] will override message.reply_to
# (because the extra_headers attr is processed after reply_to). # (because the extra_headers attr is processed after reply_to).
# This matches the behavior of Django's EmailMessage.message(). # This matches the behavior of Django's EmailMessage.message().
self.set_reply_to(parse_address_list([reply_to])) self.set_reply_to(parse_address_list([reply_to], field="extra_headers['Reply-To']"))
if 'From' in headers: if 'From' in headers:
# If message.extra_headers['From'] is supplied, it should override message.from_email, # If message.extra_headers['From'] is supplied, it should override message.from_email,
@@ -304,8 +308,8 @@ class BasePayload(object):
# - https://code.djangoproject.com/ticket/9214 # - https://code.djangoproject.com/ticket/9214
# - https://github.com/django/django/blob/1.8/django/core/mail/message.py#L269 # - https://github.com/django/django/blob/1.8/django/core/mail/message.py#L269
# - https://github.com/django/django/blob/1.8/django/core/mail/backends/smtp.py#L118 # - https://github.com/django/django/blob/1.8/django/core/mail/backends/smtp.py#L118
header_from = parse_address_list(headers.pop('From')) header_from = parse_address_list(headers.pop('From'), field="extra_headers['From']")
envelope_sender = parse_single_address(self.message.from_email) # must be single address envelope_sender = parse_single_address(self.message.from_email, field="from_email") # must be single
self.set_from_email_list(header_from) self.set_from_email_list(header_from)
self.set_envelope_sender(envelope_sender) self.set_envelope_sender(envelope_sender)

View File

@@ -116,7 +116,7 @@ def update_deep(dct, other):
# (like dict.update(), no return value) # (like dict.update(), no return value)
def parse_address_list(address_list): def parse_address_list(address_list, field=None):
"""Returns a list of EmailAddress objects from strings in address_list. """Returns a list of EmailAddress objects from strings in address_list.
Essentially wraps :func:`email.utils.getaddresses` with better error Essentially wraps :func:`email.utils.getaddresses` with better error
@@ -127,6 +127,8 @@ def parse_address_list(address_list):
:param list[str]|str|None|list[None] address_list: :param list[str]|str|None|list[None] address_list:
the address or addresses to parse the address or addresses to parse
:param str|None field:
optional description of the source of these addresses, for error message
:return list[:class:`EmailAddress`]: :return list[:class:`EmailAddress`]:
:raises :exc:`AnymailInvalidAddress`: :raises :exc:`AnymailInvalidAddress`:
""" """
@@ -151,8 +153,11 @@ def parse_address_list(address_list):
for address in parsed: for address in parsed:
if address.username == '' or address.domain == '': if address.username == '' or address.domain == '':
# Django SMTP allows username-only emails, but they're not meaningful with an ESP # Django SMTP allows username-only emails, but they're not meaningful with an ESP
errmsg = "Invalid email address '%s' parsed from '%s'." % ( errmsg = "Invalid email address '{problem}' parsed from '{source}'{where}.".format(
address.addr_spec, ", ".join(address_list_strings)) problem=address.addr_spec,
source=", ".join(address_list_strings),
where=" in `%s`" % field if field else "",
)
if len(parsed) > len(address_list): if len(parsed) > len(address_list):
errmsg += " (Maybe missing quotes around a display-name?)" errmsg += " (Maybe missing quotes around a display-name?)"
raise AnymailInvalidAddress(errmsg) raise AnymailInvalidAddress(errmsg)
@@ -160,17 +165,20 @@ def parse_address_list(address_list):
return parsed return parsed
def parse_single_address(address): def parse_single_address(address, field=None):
"""Parses a single EmailAddress from str address, or raises AnymailInvalidAddress """Parses a single EmailAddress from str address, or raises AnymailInvalidAddress
:param str address: the fully-formatted email str to parse :param str address: the fully-formatted email str to parse
:param str|None field: optional description of the source of this address, for error message
:return :class:`EmailAddress`: if address contains a single email :return :class:`EmailAddress`: if address contains a single email
:raises :exc:`AnymailInvalidAddress`: if address contains no or multiple emails :raises :exc:`AnymailInvalidAddress`: if address contains no or multiple emails
""" """
parsed = parse_address_list([address]) parsed = parse_address_list([address], field=field)
count = len(parsed) count = len(parsed)
if count > 1: if count > 1:
raise AnymailInvalidAddress("Only one email address is allowed; found %d in %r" % (count, address)) raise AnymailInvalidAddress(
"Only one email address is allowed; found {count} in '{address}'{where}.".format(
count=count, address=address, where=" in `%s`" % field if field else ""))
else: else:
return parsed[0] return parsed[0]

View File

@@ -11,7 +11,7 @@ from django.utils.functional import Promise
from django.utils.timezone import utc from django.utils.timezone import utc
from django.utils.translation import ugettext_lazy from django.utils.translation import ugettext_lazy
from anymail.exceptions import AnymailConfigurationError, AnymailUnsupportedFeature from anymail.exceptions import AnymailConfigurationError, AnymailInvalidAddress, AnymailUnsupportedFeature
from anymail.message import AnymailMessage from anymail.message import AnymailMessage
from .utils import AnymailTestMixin from .utils import AnymailTestMixin
@@ -319,6 +319,38 @@ class CatchCommonErrorsTests(TestBackendTestCase):
with self.assertRaisesMessage(TypeError, '"reply_to" attribute must be a list or other iterable'): with self.assertRaisesMessage(TypeError, '"reply_to" attribute must be a list or other iterable'):
self.message.send() self.message.send()
def test_identifies_source_of_parsing_errors(self):
"""Errors parsing email addresses should say which field had the problem"""
# Note: General email address parsing tests are in test_utils.ParseAddressListTests.
# This just checks the error includes the field name when parsing for sending a message.
self.message.from_email = ''
with self.assertRaisesMessage(AnymailInvalidAddress,
"Invalid email address '' parsed from '' in `from_email`."):
self.message.send()
self.message.from_email = 'from@example.com'
# parse_address_list
self.message.to = ['ok@example.com', 'oops']
with self.assertRaisesMessage(AnymailInvalidAddress,
"Invalid email address 'oops' parsed from 'ok@example.com, oops' in `to`."):
self.message.send()
self.message.to = ['test@example.com']
# parse_single_address
self.message.envelope_sender = 'one@example.com, two@example.com'
with self.assertRaisesMessage(AnymailInvalidAddress,
"Only one email address is allowed; found 2"
" in 'one@example.com, two@example.com' in `envelope_sender`."):
self.message.send()
delattr(self.message, 'envelope_sender')
# process_extra_headers
self.message.extra_headers['From'] = 'Mail, Inc. <mail@example.com>'
with self.assertRaisesMessage(AnymailInvalidAddress,
"Invalid email address 'Mail' parsed from 'Mail, Inc. <mail@example.com>'"
" in `extra_headers['From']`. (Maybe missing quotes around a display-name?)"):
self.message.send()
def flatten_emails(emails): def flatten_emails(emails):
return [str(email) for email in emails] return [str(email) for email in emails]