4 files changed, 94 insertions, 15 deletions

diff --git a/src/mailman/app/membership.py b/src/mailman/app/membership.py
index c50169a7c..996493dc4 100644
--- a/src/mailman/app/membership.py
+++ b/src/mailman/app/membership.py
@@ -32,7 +32,8 @@ from mailman.core.i18n import _
 from mailman.email.message import OwnerNotification
 from mailman.interfaces.bans import IBanManager
 from mailman.interfaces.member import (
-    MemberRole, MembershipIsBannedError, NotAMemberError, SubscriptionEvent)
+    AlreadySubscribedError, MemberRole, MembershipIsBannedError,
+    NotAMemberError, SubscriptionEvent)
 from mailman.interfaces.usermanager import IUserManager
 from mailman.utilities.i18n import make
 from zope.component import getUtility
@@ -96,15 +97,32 @@ def add_member(mlist, email, display_name, password, delivery_mode, language,
         member = mlist.subscribe(address, role)
         member.preferences.delivery_mode = delivery_mode
     else:
-        # The user exists and is linked to the address.
+        # The user exists and is linked to the case-insensitive address.
+        # We're looking for two versions of the email address, the case
+        # preserved version and the case insensitive version.   We'll
+        # subscribe the version with matching case if it exists, otherwise
+        # we'll use one of the matching case-insensitively ones.  It's
+        # undefined which one we pick.
+        case_preserved = None
+        case_insensitive = None
         for address in user.addresses:
-            if address.email == email:
-                break
-        else:
-            raise AssertionError(
-                'User should have had linked address: {0}'.format(address))
-        # Create the member and set the appropriate preferences.
-        member = mlist.subscribe(address, role)
+            if address.original_email == email:
+                case_preserved = address
+            if address.email == email.lower():
+                case_insensitive = address
+        assert case_preserved is not None or case_insensitive is not None, (
+            'Could not find a linked address for: {}'.format(email))
+        address = (case_preserved if case_preserved is not None
+                   else case_insensitive)
+        # Create the member and set the appropriate preferences.  It's
+        # possible we're subscribing the lower cased version of the address;
+        # if that's already subscribed re-issue the exception with the correct
+        # email address (i.e. the one passed in here).
+        try:
+            member = mlist.subscribe(address, role)
+        except AlreadySubscribedError as error:
+            raise AlreadySubscribedError(
+                error.fqdn_listname, email, error.role)
         member.preferences.preferred_language = language
         member.preferences.delivery_mode = delivery_mode
     return member
diff --git a/src/mailman/app/tests/test_membership.py b/src/mailman/app/tests/test_membership.py
index 9b42c21d6..cdf0641ea 100644
--- a/src/mailman/app/tests/test_membership.py
+++ b/src/mailman/app/tests/test_membership.py
@@ -166,6 +166,32 @@ class TestAddMember(unittest.TestCase):
         self.assertEqual(member_1.role, MemberRole.member)
         self.assertEqual(member_2.role, MemberRole.owner)
 
+    def test_add_member_with_mixed_case_email(self):
+        # LP: #1425359 - Mailman is case-perserving, case-insensitive.  This
+        # test subscribes the lower case address and ensures the original
+        # mixed case address can't be subscribed.
+        email = 'APerson@example.com'
+        add_member(self._mlist, email.lower(), 'Ann Person', '123',
+                   DeliveryMode.regular, system_preferences.preferred_language)
+        with self.assertRaises(AlreadySubscribedError) as cm:
+            add_member(self._mlist, email, 'Ann Person', '123',
+                       DeliveryMode.regular,
+                       system_preferences.preferred_language)
+        self.assertEqual(cm.exception.email, email)
+            
+    def test_add_member_with_lower_case_email(self):
+        # LP: #1425359 - Mailman is case-perserving, case-insensitive.  This
+        # test subscribes the mixed case address and ensures the lower cased
+        # address can't be added.
+        email = 'APerson@example.com'
+        add_member(self._mlist, email, 'Ann Person', '123',
+                   DeliveryMode.regular, system_preferences.preferred_language)
+        with self.assertRaises(AlreadySubscribedError) as cm:
+            add_member(self._mlist, email.lower(), 'Ann Person', '123',
+                       DeliveryMode.regular,
+                       system_preferences.preferred_language)
+        self.assertEqual(cm.exception.email, email.lower())
+
 
 
 class TestAddMemberPassword(unittest.TestCase):
diff --git a/src/mailman/docs/NEWS.rst b/src/mailman/docs/NEWS.rst
index dbbe5ad8a..381cd5e03 100644
--- a/src/mailman/docs/NEWS.rst
+++ b/src/mailman/docs/NEWS.rst
@@ -21,6 +21,9 @@ Bugs
    (LP: #1418280)
  * When deleting a user via REST, make sure all linked addresses are deleted.
    Found by Andrew Stuart.  (LP: #1419519)
+ * When trying to subscribe an address to a mailing list through the REST API
+   where a case-differing version of the address is already subscribed, return
+   a 409 error instead of a 500 error.  Found by Ankush Sharma.  (LP: #1425359)
 
 Configuration
 -------------
diff --git a/src/mailman/rest/tests/test_membership.py b/src/mailman/rest/tests/test_membership.py
index e1bff833b..a77dea3b5 100644
--- a/src/mailman/rest/tests/test_membership.py
+++ b/src/mailman/rest/tests/test_membership.py
@@ -39,6 +39,12 @@ from urllib.error import HTTPError
 from zope.component import getUtility
 
 
+def _set_preferred(user):
+    preferred = list(user.addresses)[0]
+    preferred.verified_on = now()
+    user.preferred_address = preferred
+
+
 
 class TestMembership(unittest.TestCase):
     layer = RESTLayer
@@ -98,6 +104,36 @@ class TestMembership(unittest.TestCase):
         self.assertEqual(cm.exception.code, 409)
         self.assertEqual(cm.exception.reason, b'Member already subscribed')
 
+    def test_add_member_with_mixed_case_email(self):
+        # LP: #1425359 - Mailman is case-perserving, case-insensitive.  This
+        # test subscribes the lower case address and ensures the original mixed
+        # case address can't be subscribed.
+        with transaction():
+            anne = self._usermanager.create_address('anne@example.com')
+            self._mlist.subscribe(anne)
+        with self.assertRaises(HTTPError) as cm:
+            call_api('http://localhost:9001/3.0/members', {
+                'list_id': 'test.example.com',
+                'subscriber': 'ANNE@example.com',
+                })
+        self.assertEqual(cm.exception.code, 409)
+        self.assertEqual(cm.exception.reason, b'Member already subscribed')
+
+    def test_add_member_with_lower_case_email(self):
+        # LP: #1425359 - Mailman is case-perserving, case-insensitive.  This
+        # test subscribes the mixed case address and ensures the lower cased
+        # address can't be added.
+        with transaction():
+            anne = self._usermanager.create_address('ANNE@example.com')
+            self._mlist.subscribe(anne)
+        with self.assertRaises(HTTPError) as cm:
+            call_api('http://localhost:9001/3.0/members', {
+                'list_id': 'test.example.com',
+                'subscriber': 'anne@example.com',
+                })
+        self.assertEqual(cm.exception.code, 409)
+        self.assertEqual(cm.exception.reason, b'Member already subscribed')
+
     def test_join_with_invalid_delivery_mode(self):
         with self.assertRaises(HTTPError) as cm:
             call_api('http://localhost:9001/3.0/members', {
@@ -129,9 +165,7 @@ class TestMembership(unittest.TestCase):
     def test_join_as_user_with_preferred_address(self):
         with transaction():
             anne = self._usermanager.create_user('anne@example.com')
-            preferred = list(anne.addresses)[0]
-            preferred.verified_on = now()
-            anne.preferred_address = preferred
+            _set_preferred(anne)
             self._mlist.subscribe(anne)
         content, response = call_api('http://localhost:9001/3.0/members')
         self.assertEqual(response.status, 200)
@@ -150,9 +184,7 @@ class TestMembership(unittest.TestCase):
     def test_member_changes_preferred_address(self):
         with transaction():
             anne = self._usermanager.create_user('anne@example.com')
-            preferred = list(anne.addresses)[0]
-            preferred.verified_on = now()
-            anne.preferred_address = preferred
+            _set_preferred(anne)
             self._mlist.subscribe(anne)
         # Take a look at Anne's current membership.
         content, response = call_api('http://localhost:9001/3.0/members')