Merge lp:~black-perl/mailman/fix-mailman into lp:mailman

[Ankush Sharma]

Discussed here https://mail.python.org/pipermail/mailman-developers/2015-March/024474.html about the bug and its fix.
Added test to call mailman REST api using case sensitive email address in 'test_try_to_join_a_list_twice' method.

[Barry Warsaw (barry)]

I think you've done a great job in finding the root cause. I have just a couple of inlined comments, but I am happy to fix those myself when I merge to trunk.

review: Approve

[Barry Warsaw (barry)]

Ah, the fix isn't right after all. The problem is that the call to mlist.subscribe() after breaking out of the loop can subscribe the original lower case address, but it should always subscribe the case preserved address. I have to think about this.

review: Needs Fixing

[Barry Warsaw (barry)]

What this means is that people cannot subscribe two email addresses to a mailing list that differ only in case. In some sense this conflicts with the model, since the case-different email addresses are considered different address objects, but I still think given the stated policy of case-preserving case-insensitive addresses, this will be acceptable.

It will still probably be possible to link two addresses to the user which differ only in case, but since these can't both be subscribed to the same mailing list (with the same role), I suppose we'll have to accept this oddity.

It's also possible to subscribe a user with a preferred address that differs in case with another address object linked to the user. This is consistent with the existing semantics.

[Barry Warsaw (barry)]

I'm not sure how much of your original branch survived, but I gave you credit for finding the bug. Thanks! You might take a look at r7304 for details. This was a tricky and fun one.

[Ankush Sharma (black-perl)]

Thank you Barry for your time. I am glad you appreciated the bug find. I will surely take a look.

=== modified file 'src/mailman/app/membership.py'
--- src/mailman/app/membership.py	2015-01-05 01:22:39 +0000
+++ src/mailman/app/membership.py	2015-03-04 09:46:29 +0000
@@ -98,7 +98,7 @@
     else:
         # The user exists and is linked to the address.
         for address in user.addresses:
-            if address.email == email:
+            if address.email == email.lower():
                 break
         else:
             raise AssertionError(
 
=== modified file 'src/mailman/rest/tests/test_membership.py'
--- src/mailman/rest/tests/test_membership.py	2015-01-05 01:40:47 +0000
+++ src/mailman/rest/tests/test_membership.py	2015-03-04 09:46:29 +0000
@@ -98,6 +98,15 @@
         self.assertEqual(cm.exception.code, 409)
         self.assertEqual(cm.exception.reason, b'Member already subscribed')
 
+        # Member subscription using case sensitive email
+        with self.assertRaises(HTTPError) as cm:
+            call_api('http://localhost:9001/3.0/members', {
+                'list_id': 'test.example.com',
+                'subscriber': '[e-mail]',
+                })
+        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', {