3 files changed, 86 insertions, 7 deletions

diff --git a/src/mailman/commands/cli_import.py b/src/mailman/commands/cli_import.py
index 30aeb7894..344d5baee 100644
--- a/src/mailman/commands/cli_import.py
+++ b/src/mailman/commands/cli_import.py
@@ -25,6 +25,7 @@ __all__ = [
 import sys
 import pickle
 
+from contextlib import ExitStack, contextmanager
 from mailman.core.i18n import _
 from mailman.database.transaction import transactional
 from mailman.interfaces.command import ICLISubCommand
@@ -35,6 +36,25 @@ from zope.interface import implementer
 
 
 
+# A fake Bouncer class from Mailman 2.1, we don't use it but there are
+# instances in the .pck files.
+class Bouncer:
+    class _BounceInfo:
+        pass
+
+
+@contextmanager
+def hacked_sys_modules():
+    assert 'Mailman.Bouncer' not in sys.modules
+    sys.modules['Mailman.Bouncer'] = Bouncer
+    try:
+        yield
+    finally:
+        del sys.modules['Mailman.Bouncer']
+
+
+
+
 @implementer(ICLISubCommand)
 class Import21:
     """Import Mailman 2.1 list data."""
@@ -74,10 +94,13 @@ class Import21:
         assert len(args.pickle_file) == 1, (
             'Unexpected positional arguments: %s' % args.pickle_file)
         filename = args.pickle_file[0]
-        with open(filename, 'rb') as fp:
+        with ExitStack() as resources:
+            fp = resources.enter_context(open(filename, 'rb'))
+            resources.enter_context(hacked_sys_modules())
             while True:
                 try:
-                    config_dict = pickle.load(fp)
+                    config_dict = pickle.load(
+                        fp, encoding='utf-8', errors='ignore')
                 except EOFError:
                     break
                 except pickle.UnpicklingError:
diff --git a/src/mailman/utilities/importer.py b/src/mailman/utilities/importer.py
index 8590d9b1b..bb4273b12 100644
--- a/src/mailman/utilities/importer.py
+++ b/src/mailman/utilities/importer.py
@@ -32,6 +32,7 @@ from mailman.config import config
 from mailman.core.errors import MailmanError
 from mailman.handlers.decorate import decorate, decorate_template
 from mailman.interfaces.action import Action, FilterAction
+from mailman.interfaces.address import IEmailValidator
 from mailman.interfaces.archiver import ArchivePolicy
 from mailman.interfaces.autorespond import ResponseAction
 from mailman.interfaces.bans import IBanManager
@@ -387,11 +388,17 @@ def import_config_pck(mlist, config_dict):
     regulars_set = set(config_dict.get('members', {}))
     digesters_set = set(config_dict.get('digest_members', {}))
     members = regulars_set.union(digesters_set)
-    import_roster(mlist, config_dict, members, MemberRole.member)
-    import_roster(mlist, config_dict, config_dict.get('owner', []),
-                  MemberRole.owner)
-    import_roster(mlist, config_dict, config_dict.get('moderator', []),
-                  MemberRole.moderator)
+    # Don't send welcome messages when we import the rosters.
+    send_welcome_message = mlist.send_welcome_message
+    mlist.send_welcome_message = False
+    try:
+        import_roster(mlist, config_dict, members, MemberRole.member)
+        import_roster(mlist, config_dict, config_dict.get('owner', []),
+                      MemberRole.owner)
+        import_roster(mlist, config_dict, config_dict.get('moderator', []),
+                      MemberRole.moderator)
+    finally:
+        mlist.send_welcome_message = send_welcome_message
 
 
 
@@ -408,6 +415,7 @@ def import_roster(mlist, config_dict, members, role):
     :type role: MemberRole enum
     """
     usermanager = getUtility(IUserManager)
+    validator = getUtility(IEmailValidator)
     roster = mlist.get_roster(role)
     for email in members:
         # For owners and members, the emails can have a mixed case, so
@@ -427,8 +435,13 @@ def import_roster(mlist, config_dict, members, role):
                 merged_members.update(config_dict.get('digest_members', {}))
                 if merged_members.get(email, 0) != 0:
                     original_email = bytes_to_str(merged_members[email])
+                    if not validator.is_valid(original_email):
+                        original_email = email
                 else:
                     original_email = email
+                if not validator.is_valid(original_email):
+                    # Skip this one entirely.
+                    continue
                 address = usermanager.create_address(original_email)
                 address.verified_on = datetime.datetime.now()
             user.link(address)
diff --git a/src/mailman/utilities/tests/test_import.py b/src/mailman/utilities/tests/test_import.py
index b0ab9938d..938ef7d2e 100644
--- a/src/mailman/utilities/tests/test_import.py
+++ b/src/mailman/utilities/tests/test_import.py
@@ -38,6 +38,7 @@ from mailman.app.lifecycle import create_list
 from mailman.config import config
 from mailman.handlers.decorate import decorate
 from mailman.interfaces.action import Action, FilterAction
+from mailman.interfaces.address import InvalidEmailAddressError
 from mailman.interfaces.archiver import ArchivePolicy
 from mailman.interfaces.autorespond import ResponseAction
 from mailman.interfaces.bans import IBanManager
@@ -747,6 +748,48 @@ class TestRosterImport(unittest.TestCase):
         anne = self._usermanager.get_user('anne@example.com')
         self.assertTrue(anne.controls('anne@example.com'))
 
+    def test_invalid_original_email(self):
+        # When the member has an original email address (i.e. the
+        # case-preserved version) that is invalid, their new address record's
+        # original_email attribute will only be the case insensitive version.
+        self._pckdict['members']['anne@example.com'] = b'invalid email address'
+        try:
+            import_config_pck(self._mlist, self._pckdict)
+        except InvalidEmailAddressError as error:
+            self.fail(error)
+        self.assertIn('anne@example.com',
+                      [a.email for a in self._mlist.members.addresses])
+        anne = self._usermanager.get_address('anne@example.com')
+        self.assertEqual(anne.original_email, 'anne@example.com')
+
+    def test_invalid_email(self):
+        # When a member's email address is invalid, that member is skipped
+        # during the import.
+        self._pckdict['members'] = {
+            'anne@example.com': 0,
+            'invalid email address': b'invalid email address'
+            }
+        self._pckdict['digest_members'] = {}
+        try:
+            import_config_pck(self._mlist, self._pckdict)
+        except InvalidEmailAddressError as error:
+            self.fail(error)
+        self.assertEqual(['anne@example.com'],
+                         [a.email for a in self._mlist.members.addresses])
+
+    def test_no_email_sent(self):
+        # No welcome message is sent to newly imported members.
+        self.assertTrue(self._mlist.send_welcome_message)
+        import_config_pck(self._mlist, self._pckdict)
+        self.assertIn('anne@example.com',
+                      [a.email for a in self._mlist.members.addresses])
+        # There are no messages in any of the queues.
+        for queue, switchboard in config.switchboards.items():
+            file_count = len(switchboard.files)
+            self.assertEqual(file_count, 0,
+                             "Unexpected queue '{}' file count: {}".format(
+                                 queue, file_count))
+        self.assertTrue(self._mlist.send_welcome_message)