Email crashes cryptically when emptystring passed to django.core.mail.message.sanitize_address

[Jarek Glowacki]

In [10]: sanitize_address('', 'utf-8')
---------------------------------------------------------------------------
IndexError                                Traceback (most recent call last)
<ipython-input-10-ad3601147d6a> in <module>()
----> 1 sanitize_address('', 'utf-8')

/usr/local/lib/python3.5/site-packages/django/core/mail/message.py in sanitize_address(addr, encoding)
    159
    160     try:
--> 161         address = Address(nm, addr_spec=addr)
    162     except (InvalidHeaderDefect, NonASCIILocalPartDefect):
    163         localpart, domain = split_addr(addr, encoding)

/usr/local/lib/python3.5/email/headerregistry.py in __init__(self, display_name, username, domain, addr_spec)
     40                 raise TypeError("addrspec specified when username and/or "
     41                                 "domain also specified")
---> 42             a_s, rest = parser.get_addr_spec(addr_spec)
     43             if rest:
     44                 raise ValueError("Invalid addr_spec; only '{}' "

/usr/local/lib/python3.5/email/_header_value_parser.py in get_addr_spec(value)
   1986     """
   1987     addr_spec = AddrSpec()
-> 1988     token, value = get_local_part(value)
   1989     addr_spec.append(token)
   1990     if not value or value[0] != '@':

/usr/local/lib/python3.5/email/_header_value_parser.py in get_local_part(value)
   1798     local_part = LocalPart()
   1799     leader = None
-> 1800     if value[0] in CFWS_LEADER:
   1801         leader, value = get_cfws(value)
   1802     if not value:

IndexError: string index out of range

Could we reraise some more intuitive error back to the user for this case?

A single @ symbol also raises an exception for this method, but at least it yields a more meaningful error that the dev can track down:
HeaderParseError: Expected 'atom' or 'quoted-string' but found '@'
Though maybe this could be beefed up to something nicer also.

[Wagner Macedo]

What the most proper exception to raise here?

[Jarek Glowacki]

I'd be happy with any of:
InvalidEmailException
EmptyEmailException
InvalidEmailAddressException
EmptyEmailAddressException

Or maybe even silencing the exception and having something else in the food chain worry about it.

I don't know much about the Mail component of Django, so best for someone else to make that call, so that it's consistent with the way other kinds of invalid emails behave.

[Claude Paroz]

I'd like to know a bit more about the use case to pass an empty string to sanitize_address, please.

[Jarek Glowacki]

We don't use sanitize_addresss directly, it ends up getting called during an email.send() call on our end.

We have a pretty complex django project that has numerous apps set up to do some form of emailing. The email 'to' and 'cc' are set by the end user on a Contact model, both fields are allowed to be empty. The system then builds emails in various configurations, in some cases ccing contacts in on one email, in other cases creating separate emails per contact. Long story short the logic is nontrivial.

A couple of times now (usually after rounds of refactoring) we've hit a case where either the 'to' or 'cc' list contains an emptystring, causing this cryptic error to surface when email.send() is called. Each time it's been someone else in the office hitting it, consuming a moderate amount of time to track down the cause.

It would be nice if this raised a proper error, so that we could either have something nice to catch and handle during runtime, or be able to quickly understand what's going if it shows up again in our error logs.

Worst of all, this doesn't show up when we test the project locally, as we swap out the smtp backend with a dummy one in debug mode. So another idea could maybe be to catch this problem earlier to make it backend-independent.

This is a non issue for those that are careful and validate email addresses before trying to call .send(), but for those that do fall into this trap, they probably shouldn't be hit with an unhelpful IndexError: string index out of range.

[Claude Paroz]

Thanks for the explanation, it's also useful for building an appropriate regression test.

[Claude Paroz]

In that ​PR, I chose to prevent empty addresses in message.recipients(). Hopefully solves your issue!

[Wagner Macedo]

A regression test isn't needed in this case?

EDIT: Sorry, a regression test is there, I didn't pay attention to your PR.

[Tim Graham]

Assuming the PR fulfills the use case, the patch looks good to me.

[Claude Paroz <claude@…>]

In 8858631:

Fixed #27469 -- Prevented sending email to empty addresses

Thanks Jarek Glowacki for the report.