Merge branch 'master' into py36

7 files changed, 43 insertions, 12 deletions

diff --git a/src/mailman/rest/docs/lists.rst b/src/mailman/rest/docs/lists.rst
index 247b2f4e7..6a034df94 100644
--- a/src/mailman/rest/docs/lists.rst
+++ b/src/mailman/rest/docs/lists.rst
@@ -275,7 +275,6 @@ archivers are available, and whether they are enabled for this mailing list.
     http_etag: "..."
     mail-archive: True
     mhonarc: True
-    prototype: True
 
 You can set all the archiver states by putting new state flags on the
 resource.
@@ -285,7 +284,6 @@ resource.
     ...     'http://localhost:9001/3.0/lists/dog@example.com/archivers', {
     ...         'mail-archive': False,
     ...         'mhonarc': True,
-    ...         'prototype': False,
     ...         }, method='PUT')
     content-length: 0
     date: ...
@@ -296,7 +294,6 @@ resource.
     http_etag: "..."
     mail-archive: False
     mhonarc: True
-    prototype: False
 
 You can change the state of a subset of the list archivers.
 ::
@@ -314,7 +311,6 @@ You can change the state of a subset of the list archivers.
     http_etag: "..."
     mail-archive: False
     mhonarc: False
-    prototype: False
 
 
 List digests
diff --git a/src/mailman/rest/lists.py b/src/mailman/rest/lists.py
index 8952f1156..1fdf595fb 100644
--- a/src/mailman/rest/lists.py
+++ b/src/mailman/rest/lists.py
@@ -337,13 +337,15 @@ class ListArchivers:
         """Get all the archiver statuses."""
         archiver_set = IListArchiverSet(self._mlist)
         resource = {archiver.name: archiver.is_enabled
-                    for archiver in archiver_set.archivers}
+                    for archiver in archiver_set.archivers
+                    if archiver.system_archiver.is_enabled}
         okay(response, etag(resource))
 
     def patch_put(self, request, response, is_optional):
         archiver_set = IListArchiverSet(self._mlist)
         kws = {archiver.name: ArchiverGetterSetter(self._mlist)
-               for archiver in archiver_set.archivers}
+               for archiver in archiver_set.archivers
+               if archiver.system_archiver.is_enabled}
         if is_optional:
             # For a PATCH, all attributes are optional.
             kws['_optional'] = kws.keys()
diff --git a/src/mailman/rest/post_moderation.py b/src/mailman/rest/post_moderation.py
index fc38af359..33a32de20 100644
--- a/src/mailman/rest/post_moderation.py
+++ b/src/mailman/rest/post_moderation.py
@@ -71,7 +71,14 @@ class _HeldMessageBase(_ModerationBase):
         # resource.  XXX See LP: #967954
         key = resource.pop('key')
         msg = getUtility(IMessageStore).get_message_by_id(key)
-        resource['msg'] = msg.as_string()
+        try:
+            resource['msg'] = msg.as_string()
+        except KeyError:
+            # If the message can't be parsed, return a generic message instead
+            # of raising an error.
+            #
+            # See http://bugs.python.org/issue27321 and GL#256
+            resource['msg'] = 'This message is defective'
         # Some of the _mod_* keys we want to rename and place into the JSON
         # resource.  Others we can drop.  Since we're mutating the dictionary,
         # we need to make a copy of the keys.  When you port this to Python 3,
diff --git a/src/mailman/rest/tests/data/__init__.py b/src/mailman/rest/tests/data/__init__.py
new file mode 100644
index 000000000..e69de29bb
--- /dev/null
+++ b/src/mailman/rest/tests/data/__init__.py
diff --git a/src/mailman/rest/tests/data/bad_email.eml b/src/mailman/rest/tests/data/bad_email.eml
new file mode 100644
index 000000000..83f9337dd
--- /dev/null
+++ b/src/mailman/rest/tests/data/bad_email.eml
@@ -0,0 +1,8 @@
+To: <test@example.com>
+Subject: =?koi8-r?B?UF9AX/NfQ1/5X+xfS1/p?=
+From: =?koi8-r?B?8sXL0sXB1MnXzs/FIMHHxc7U09TXzw==?=
+Content-Type: text/plain; charset=koi8-r
+Message-Id: <20160614102505.9OFQ19L1C>
+
+ώτο ταλοε ςελμανξαρ ςασσωμλα?
+λΑΛΟΚ ΟΤΛΜΙΛ ΦΔΑΤΨ ΟΤ άΤΟΗΟ ΝΕΤΟΔΑ ΠΟΙΣΛΑ ΛΜΙΕΞΤΟΧ?
diff --git a/src/mailman/rest/tests/test_lists.py b/src/mailman/rest/tests/test_lists.py
index da6e95f3b..49764f86f 100644
--- a/src/mailman/rest/tests/test_lists.py
+++ b/src/mailman/rest/tests/test_lists.py
@@ -339,7 +339,6 @@ class TestListArchivers(unittest.TestCase):
         self.assertEqual(resource, {
             'mail-archive': True,
             'mhonarc': True,
-            'prototype': True,
             })
 
     def test_archiver_statuses_on_missing_lists(self):
@@ -375,7 +374,7 @@ class TestListArchivers(unittest.TestCase):
 
     def test_put_incomplete_statuses(self):
         # PUT requires the full resource representation.  This one forgets to
-        # specify the prototype and mhonarc archiver.
+        # specify the mhonarc archiver.
         with self.assertRaises(HTTPError) as cm:
             call_api(
                 'http://localhost:9001/3.0/lists/ant.example.com/archivers', {
@@ -384,7 +383,7 @@ class TestListArchivers(unittest.TestCase):
                 method='PUT')
         self.assertEqual(cm.exception.code, 400)
         self.assertEqual(cm.exception.reason,
-                         b'Missing parameters: mhonarc, prototype')
+                         b'Missing parameters: mhonarc')
 
     def test_patch_bogus_status(self):
         # Archiver statuses must be interpretable as booleans.
@@ -392,8 +391,7 @@ class TestListArchivers(unittest.TestCase):
             call_api(
                 'http://localhost:9001/3.0/lists/ant.example.com/archivers', {
                     'mail-archive': 'sure',
-                    'mhonarc': False,
-                    'prototype': 'no'
+                    'mhonarc': 'no'
                     },
                 method='PATCH')
         self.assertEqual(cm.exception.code, 400)
diff --git a/src/mailman/rest/tests/test_moderation.py b/src/mailman/rest/tests/test_moderation.py
index e0c3f1ccf..21129207c 100644
--- a/src/mailman/rest/tests/test_moderation.py
+++ b/src/mailman/rest/tests/test_moderation.py
@@ -19,6 +19,7 @@
 
 import unittest
 
+from email import message_from_binary_file
 from mailman.app.lifecycle import create_list
 from mailman.app.moderator import hold_message
 from mailman.database.transaction import transaction
@@ -31,6 +32,7 @@ from mailman.testing.helpers import (
     call_api, get_queue_messages, set_preferred,
     specialized_message_from_string as mfs)
 from mailman.testing.layers import RESTLayer
+from pkg_resources import resource_filename
 from urllib.error import HTTPError
 from zope.component import getUtility
 
@@ -206,6 +208,24 @@ class TestSubscriptionModeration(unittest.TestCase):
         emails = set(json['email'] for json in content['entries'])
         self.assertEqual(emails, {'anne@example.com', 'bart@example.com'})
 
+    def test_view_malformed_held_message(self):
+        # Opening a bad (i.e. bad structure) email and holding it.
+        email_path = resource_filename(
+            'mailman.rest.tests.data', 'bad_email.eml')
+        with open(email_path, 'rb') as fp:
+            msg = message_from_binary_file(fp)
+        msg.sender = 'aperson@example.com'
+        with transaction():
+            hold_message(self._mlist, msg)
+        # Now trying to access held messages from REST API should not give
+        # 500 server error if one of the messages can't be parsed properly.
+        content, response = call_api(
+            'http://localhost:9001/3.0/lists/ant@example.com/held')
+        self.assertEqual(response.status, 200)
+        self.assertEqual(len(content['entries']), 1)
+        self.assertEqual(content['entries'][0]['msg'],
+                         'This message is defective')
+
     def test_individual_request(self):
         # We can view an individual request.
         with transaction():