Merge pull request #1360 from fyfe/feature/improve-email-address-parser

improve e-mail address splitting functions

modoboa/lib/email_utils.py

@@ -5,11 +5,12 @@ from __future__ import unicode_literals
 import email
 from email.header import Header
 from email.mime.text import MIMEText
-from email.utils import make_msgid, formatdate, parseaddr
+from email.utils import make_msgid, formataddr, formatdate, getaddresses
 import re
 import smtplib
 import time
 
+from django.utils import six
 from django.utils.encoding import smart_str, smart_text
 from django.utils.html import conditional_escape, escape
 from django.template.loader import render_to_string
@@ -262,6 +263,47 @@ class Email(object):
         return render_to_string("common/mailheaders.html", context)
 
 
+def split_address(address):
+    """Split an e-mail address into local part and domain."""
+    assert isinstance(address, six.text_type),\
+        "address should be of type %s" % six.text_type.__name__
+    if "@" not in address:
+        local_part = address
+        domain = None
+    else:
+        local_part, domain = address.rsplit("@", 1)
+    return (local_part, domain)
+
+
+def split_local_part(local_part, delimiter=None):
+    """Split a local part into local part and extension."""
+    assert isinstance(local_part, six.text_type),\
+        "local_part should be of type %s" % six.text_type.__name__
+    assert isinstance(delimiter, six.text_type) or delimiter is None,\
+        "delimiter should be of type %s" % six.text_type.__name__
+    extension = None
+    if local_part.lower() in ["mailer-daemon", "double-bounce"]:
+        # never split these special addresses
+        pass
+    elif (
+        delimiter == "-"
+        and (local_part.startswith("owner-")
+             or local_part.endswith("-request"))
+    ):
+        # don't split owner-* or *-request if - is the delimiter, they are
+        # special addresses used by mail lists.
+        pass
+    elif (
+        delimiter
+        and delimiter in local_part
+        and local_part[0] != delimiter
+        and local_part[-1] != delimiter
+    ):
+        local_part, extension = local_part.split(delimiter, 1)
+
+    return (local_part, extension)
+
+
 def split_mailbox(mailbox, return_extension=False):
     """Try to split an address into parts (local part and domain name).
 
@@ -271,23 +313,11 @@ def split_mailbox(mailbox, return_extension=False):
     :return: a tuple (local part, domain<, extension>)
 
     """
-    domain = None
-    if "@" not in mailbox:
-        localpart = mailbox
-    else:
-        parts = mailbox.split("@")
-        if len(parts) == 2:
-            localpart = parts[0]
-            domain = parts[1]
-        else:
-            domain = parts[-1]
-            localpart = "@".join(parts[:-1])
+    local_part, domain = split_address(mailbox)
     if not return_extension:
-        return (localpart, domain)
-    extension = None
-    if "+" in localpart:
-        localpart, extension = localpart.split("+", 1)
-    return (localpart, domain, extension)
+        return (local_part, domain)
+    local_part, extension = split_local_part(local_part, delimiter="+")
+    return (local_part, domain, extension)
 
 
 def decode(string, encodings=None, charset=None):
@@ -311,22 +341,18 @@ def decode(string, encodings=None, charset=None):
 def prepare_addresses(addresses, usage="header"):
     """Prepare addresses before using them
 
-    FIXME: We need a real address parser here! If a name contains a
-    separator, it creates two wrong addresses.
-
     :param list addresses: a list of addresses
     :param string usage: how those addresses are going to be used
     :return: a string or a list depending on usage value
     """
     result = []
-    for address in re.split('[,;]', addresses):
+    for name, address in getaddresses(addresses):
         if not address:
             continue
-        name, addr = parseaddr(address)
         if name and usage == "header":
-            result.append("%s <%s>" % (Header(name, 'utf8'), addr))
+            result.append(formataddr((name, address)))
         else:
-            result.append(addr)
+            result.append(address)
     if usage == "header":
         return ",".join(result)
     return result

modoboa/lib/tests/test_email_utils.py

@@ -9,7 +9,8 @@ import os
 from django.test import SimpleTestCase
 from django.utils.encoding import smart_bytes, smart_text
 
-from ..email_utils import Email
+from ..email_utils import (
+    Email, prepare_addresses, split_address, split_local_part)
 
 SAMPLES_DIR = os.path.realpath(
     os.path.join(os.path.dirname(__file__), "sample_messages"))
@@ -106,3 +107,80 @@ class EmailTests(SimpleTestCase):
     def test_email_multipart_without_links(self):
         """display the text/html part of a multipart message with links removed"""
         self._test_email("multipart", dformat="html", links=False)
+
+
+class EmailAddressParserTests(SimpleTestCase):
+
+    """Tests for split_address() and split_local_part()."""
+
+    def test_split_address_with_domain(self):
+        """Split an e-mail address with domain."""
+        address = "User@sub.exAMPLE.COM"
+        expected_output = ("User", "sub.exAMPLE.COM")
+        output = split_address(address)
+        self.assertEqual(output, expected_output)
+
+    def test_split_address_without_domain(self):
+        """Split an e-mail address with domain."""
+        address = "User"
+        expected_output = ("User", None)
+        output = split_address(address)
+        self.assertEqual(output, expected_output)
+
+    def test_split_local_part_without_delimiter(self):
+        """Split a local part without delimiter."""
+        local_part = "User+Foo"
+        expected_output = ("User+Foo", None)
+        output = split_local_part(local_part)
+        self.assertEqual(output, expected_output)
+
+    def test_split_local_part_with_delimiter(self):
+        """Split a local part with delimiter."""
+        local_part = "User+Foo"
+        expected_output = ("User", "Foo")
+        output = split_local_part(local_part, "+")
+        self.assertEqual(output, expected_output)
+
+    def test_split_local_part_mailing_list_address(self):
+        """Check special case addresses used by mailing lists."""
+        local_part = "owner-modoboa"
+        expected_output = ("owner-modoboa", None)
+        output = split_local_part(local_part, "-")
+        self.assertEqual(output, expected_output)
+
+    def test_split_local_part_special_address(self):
+        """Check special case addresses."""
+        local_part = "mailer-daemon"
+        expected_output = ("mailer-daemon", None)
+        output = split_local_part(local_part, "-")
+        self.assertEqual(output, expected_output)
+
+    def test_prepare_addresses(self):
+        """Check a list of e-mail addresses is sepearted correctly."""
+        # value is an array with one long string not 3 sepearte values.
+        value = [
+            "\"Doe, John\" <doe.john@sub.example.com>;"
+            "\"John Smith\" <john.smith@sub.example.com>,"
+            "admin@sub.example.com"
+        ]
+        expected_output = "\"Doe, John\" <doe.john@sub.example.com>,"\
+                          "John Smith <john.smith@sub.example.com>,"\
+                          "admin@sub.example.com"
+        output = prepare_addresses(value)
+        self.assertEqual(output, expected_output)
+
+    def test_prepare_addresses_usage_envelope(self):
+        """Check a list of e-mail addresses is sepearted correctly."""
+        # value is an array with one long string not 3 sepearte values.
+        value = [
+            "\"Doe, John\" <doe.john@sub.example.com>;"
+            "\"John Smith\" <john.smith@sub.example.com>,"
+            "admin@sub.example.com"
+        ]
+        expected_output = [
+            "doe.john@sub.example.com",
+            "john.smith@sub.example.com",
+            "admin@sub.example.com",
+        ]
+        output = prepare_addresses(value, usage="envelope")
+        self.assertEqual(output, expected_output)