8 files changed, 166 insertions, 49 deletions

diff --git a/src/mailman/app/lifecycle.py b/src/mailman/app/lifecycle.py
index 829112791..2b063d21d 100644
--- a/src/mailman/app/lifecycle.py
+++ b/src/mailman/app/lifecycle.py
@@ -47,7 +47,21 @@ log = logging.getLogger('mailman.error')
 
 
 def create_list(fqdn_listname, owners=None):
-    """Create the named list and apply styles."""
+    """Create the named list and apply styles.
+
+    The mailing may not exist yet, but the domain specified in `fqdn_listname`
+    must exist.
+
+    :param fqdn_listname: The fully qualified name for the new mailing list.
+    :type fqdn_listname: string
+    :param owners: The mailing list owners.
+    :type owners: list of string email addresses
+    :return: The new mailing list.
+    :rtype: `IMailingList`
+    :raises `BadDomainSpecificationError`: when the hostname part of
+        `fqdn_listname` does not exist.
+    :raises `ListAlreadyExistsError`: when the mailing list already exists.
+    """
     if owners is None:
         owners = []
     validate(fqdn_listname)
diff --git a/src/mailman/interfaces/listmanager.py b/src/mailman/interfaces/listmanager.py
index f83791079..f24230852 100644
--- a/src/mailman/interfaces/listmanager.py
+++ b/src/mailman/interfaces/listmanager.py
@@ -73,25 +73,6 @@ class IListManager(Interface):
         :raise `ListAlreadyExistsError` if the named list already exists.
         """
 
-
-    def new(fqdn_listname):
-        """Add a new mailing list.
-
-        The mailing may not exist yet, but the domain specified in
-        `fqdn_listname` must exist.  This is a higher level interface than
-        create() and should generally be used instead of that method.
-
-        :param fqdn_listname: The fully qualified name for the new
-            mailing list.
-        :type fqdn_listname: string
-        :return: The new mailing list
-        :rtype: `IMailingList`
-        :raises `BadDomainSpecificationError`: when the hostname part of
-            `fqdn_listname` does not exist.
-        :raises `ListAlreadyExistsError`: when the mailing list already
-            exists.
-        """
-
     def get(fqdn_listname):
         """Return the mailing list with the given name, if it exists.
 
diff --git a/src/mailman/model/listmanager.py b/src/mailman/model/listmanager.py
index 3d0448e8e..e123c47f4 100644
--- a/src/mailman/model/listmanager.py
+++ b/src/mailman/model/listmanager.py
@@ -58,11 +58,6 @@ class ListManager:
         config.db.store.add(mlist)
         return mlist
 
-    def new(self, fqdn_listname):
-        """See `IListManager."""
-        from mailman.app.lifecycle import create_list
-        return create_list(fqdn_listname)
-
     def get(self, fqdn_listname):
         """See `IListManager`."""
         listname, at, hostname = fqdn_listname.partition('@')
diff --git a/src/mailman/rest/docs/helpers.txt b/src/mailman/rest/docs/helpers.txt
index 9304bbb17..7b9aa9863 100644
--- a/src/mailman/rest/docs/helpers.txt
+++ b/src/mailman/rest/docs/helpers.txt
@@ -57,3 +57,87 @@ dictionary after tagging, since that's almost always what you want.
     geddy    : bass
     http_etag: "43942176d8d5bb4414ccf35e2720ccd5251e66da"
     neil     : drums
+
+
+POST unpacking
+==============
+
+Another helper unpacks POST request variables, validating and converting their
+values.
+
+    >>> from mailman.rest.helpers import Validator
+    >>> validator = Validator(one=int, two=unicode, three=bool)
+
+    >>> class FakeRequest:
+    ...     POST = {}
+    >>> FakeRequest.POST = dict(one='1', two='two', three='yes')
+
+On valid input, the validator can be used as a **kw argument.
+
+    >>> def print_request(one, two, three):
+    ...     print repr(one), repr(two), repr(three)
+    >>> print_request(**validator(FakeRequest))
+    1 u'two' True
+
+On invalid input, an exception is raised.
+
+    >>> FakeRequest.POST['one'] = 'hello'
+    >>> print_request(**validator(FakeRequest))
+    Traceback (most recent call last):
+    ...
+    ValueError: Cannot convert parameters: one
+
+On missing input, an exception is raised.
+
+    >>> del FakeRequest.POST['one']
+    >>> print_request(**validator(FakeRequest))
+    Traceback (most recent call last):
+    ...
+    ValueError: Missing parameters: one
+
+If more than one key is missing, it will be reflected in the error message.
+
+    >>> del FakeRequest.POST['two']
+    >>> print_request(**validator(FakeRequest))
+    Traceback (most recent call last):
+    ...
+    ValueError: Missing parameters: one, two
+
+Extra keys are also not allowed.
+
+    >>> FakeRequest.POST = dict(one='1', two='two', three='yes',
+    ...                         four='', five='')
+    >>> print_request(**validator(FakeRequest))
+    Traceback (most recent call last):
+    ...
+    ValueError: Unexpected parameters: five, four
+
+However, if optional keys are missing, it's okay.
+
+    >>> validator = Validator(one=int, two=unicode, three=bool,
+    ...                       four=int, five=int,
+    ...                       _optional=('four', 'five'))
+
+    >>> FakeRequest.POST = dict(one='1', two='two', three='yes',
+    ...                         four='4', five='5')
+    >>> def print_request(one, two, three, four=None, five=None):
+    ...     print repr(one), repr(two), repr(three), repr(four), repr(five)
+    >>> print_request(**validator(FakeRequest))
+    1 u'two' True 4 5
+
+    >>> del FakeRequest.POST['four']
+    >>> print_request(**validator(FakeRequest))
+    1 u'two' True None 5
+
+    >>> del FakeRequest.POST['five']
+    >>> print_request(**validator(FakeRequest))
+    1 u'two' True None None
+
+But if the optional values are present, they must of course also be valid.
+
+    >>> FakeRequest.POST = dict(one='1', two='two', three='yes',
+    ...                         four='no', five='maybe')
+    >>> print_request(**validator(FakeRequest))
+    Traceback (most recent call last):
+    ...
+    ValueError: Cannot convert parameters: five, four
diff --git a/src/mailman/rest/domains.py b/src/mailman/rest/domains.py
index 8b7cd2699..8bc68a6c1 100644
--- a/src/mailman/rest/domains.py
+++ b/src/mailman/rest/domains.py
@@ -31,7 +31,7 @@ from zope.component import getUtility
 
 from mailman.interfaces.domain import (
     BadDomainSpecificationError, IDomainManager)
-from mailman.rest.helpers import CollectionMixin, etag, path_to
+from mailman.rest.helpers import CollectionMixin, Validator, etag, path_to
 
 
 
@@ -75,16 +75,19 @@ class AllDomains(_DomainBase):
     @resource.POST()
     def create(self, request):
         """Create a new domain."""
-        # XXX 2010-02-23 barry Sanity check the POST arguments by
-        # introspection of the target method, or via descriptors.
         domain_manager = getUtility(IDomainManager)
         try:
-            # webob gives this to us as a string, but we need unicodes.
-            kws = dict((key, unicode(value))
-                       for key, value in request.POST.items())
-            domain = domain_manager.add(**kws)
+            validator = Validator(email_host=unicode,
+                                  description=unicode,
+                                  base_url=unicode,
+                                  contact_address=unicode,
+                                  _optional=('description', 'base_url',
+                                             'contact_address'))
+            domain = domain_manager.add(**validator(request))
         except BadDomainSpecificationError:
-            return http.bad_request([], 'Domain exists')
+            return http.bad_request([], b'Domain exists')
+        except ValueError as error:
+            return http.bad_request([], str(error))
         location = path_to('domains/{0}'.format(domain.email_host))
         # Include no extra headers or body.
         return http.created(location, [], None)
diff --git a/src/mailman/rest/helpers.py b/src/mailman/rest/helpers.py
index 87614d1f0..369eebffa 100644
--- a/src/mailman/rest/helpers.py
+++ b/src/mailman/rest/helpers.py
@@ -33,6 +33,8 @@ import hashlib
 from lazr.config import as_boolean
 from mailman.config import config
 
+COMMASPACE = ', '
+
 
 
 def path_to(resource):
@@ -121,3 +123,42 @@ class CollectionMixin:
                 total_size=len(collection),
                 entries=entries,
                 )
+
+
+
+class Validator:
+    """A validator of parameter input."""
+
+    def __init__(self, **kws):
+        if '_optional' in kws:
+            self._optional = set(kws.pop('_optional'))
+        else:
+            self._optional = set()
+        self._converters = kws.copy()
+
+    def __call__(self, request):
+        values = {}
+        extras = set()
+        cannot_convert = set()
+        for key, value in request.POST.items():
+            try:
+                values[key] = self._converters[key](value)
+            except KeyError:
+                extras.add(key)
+            except (TypeError, ValueError):
+                cannot_convert.add(key)
+        # Make sure there are no unexpected values.
+        if len(extras) != 0:
+            extras = COMMASPACE.join(sorted(extras))
+            raise ValueError('Unexpected parameters: {0}'.format(extras))
+        # Make sure everything could be converted.
+        if len(cannot_convert) != 0:
+            bad = COMMASPACE.join(sorted(cannot_convert))
+            raise ValueError('Cannot convert parameters: {0}'.format(bad))
+        # Make sure nothing's missing.
+        value_keys = set(values)
+        required_keys = set(self._converters) - self._optional
+        if value_keys & required_keys != required_keys:
+            missing = COMMASPACE.join(sorted(required_keys - value_keys))
+            raise ValueError('Missing parameters: {0}'.format(missing))
+        return values
diff --git a/src/mailman/rest/lists.py b/src/mailman/rest/lists.py
index b232cc5bc..947869cd9 100644
--- a/src/mailman/rest/lists.py
+++ b/src/mailman/rest/lists.py
@@ -29,11 +29,12 @@ __all__ = [
 from restish import http, resource
 from zope.component import getUtility
 
+from mailman.app.lifecycle import create_list
 from mailman.interfaces.domain import BadDomainSpecificationError
 from mailman.interfaces.listmanager import (
     IListManager, ListAlreadyExistsError)
 from mailman.interfaces.member import MemberRole
-from mailman.rest.helpers import CollectionMixin, etag, path_to
+from mailman.rest.helpers import CollectionMixin, Validator, etag, path_to
 from mailman.rest.members import AMember
 
 
@@ -104,19 +105,16 @@ class AllLists(_ListBase):
     @resource.POST()
     def create(self, request):
         """Create a new mailing list."""
-        # XXX 2010-02-23 barry Sanity check the POST arguments by
-        # introspection of the target method, or via descriptors.
-        list_manager = getUtility(IListManager)
         try:
-            # webob gives this to us as a string, but we need unicodes.
-            kws = dict((key, unicode(value))
-                       for key, value in request.POST.items())
-            mlist = list_manager.new(**kws)
+            validator = Validator(fqdn_listname=unicode)
+            mlist = create_list(**validator(request))
         except ListAlreadyExistsError:
             return http.bad_request([], b'Mailing list exists')
         except BadDomainSpecificationError as error:
             return http.bad_request([], b'Domain does not exist {0}'.format(
                 error.domain))
+        except ValueError as error:
+            return http.bad_request([], str(error))
         # wsgiref wants headers to be bytes, not unicodes.
         location = path_to('lists/{0}'.format(mlist.fqdn_listname))
         # Include no extra headers or body.
diff --git a/src/mailman/rest/members.py b/src/mailman/rest/members.py
index ace1ff3cb..a5a3dd02e 100644
--- a/src/mailman/rest/members.py
+++ b/src/mailman/rest/members.py
@@ -32,9 +32,10 @@ from zope.component import getUtility
 from mailman.app.membership import delete_member
 from mailman.interfaces.address import InvalidEmailAddressError
 from mailman.interfaces.listmanager import NoSuchListError
-from mailman.interfaces.member import AlreadySubscribedError, MemberRole
+from mailman.interfaces.member import (
+    AlreadySubscribedError, DeliveryMode, MemberRole)
 from mailman.interfaces.membership import ISubscriptionService
-from mailman.rest.helpers import CollectionMixin, etag, path_to
+from mailman.rest.helpers import CollectionMixin, Validator, etag, path_to
 
 
 
@@ -98,14 +99,14 @@ class AllMembers(_MemberBase):
     @resource.POST()
     def create(self, request):
         """Create a new member."""
-        # XXX 2010-02-23 barry Sanity check the POST arguments by
-        # introspection of the target method, or via descriptors.
         service = getUtility(ISubscriptionService)
         try:
-            # webob gives this to us as a string, but we need unicodes.
-            kws = dict((key, unicode(value))
-                       for key, value in request.POST.items())
-            member = service.join(**kws)
+            validator = Validator(fqdn_listname=unicode,
+                                  address=unicode,
+                                  real_name=unicode,
+                                  delivery_mode=unicode,
+                                  _optional=('real_name', 'delivery_mode'))
+            member = service.join(**validator(request))
         except AlreadySubscribedError:
             return http.bad_request([], b'Member already subscribed')
         except NoSuchListError: