diff options
| -rw-r--r-- | src/mailman/app/lifecycle.py | 25 | ||||
| -rw-r--r-- | src/mailman/app/tests/test_lifecycle.py | 33 | ||||
| -rw-r--r-- | src/mailman/config/schema.cfg | 7 | ||||
| -rw-r--r-- | src/mailman/docs/NEWS.rst | 3 | ||||
| -rw-r--r-- | src/mailman/interfaces/mailinglist.py | 10 | ||||
| -rw-r--r-- | src/mailman/rest/docs/systemconf.rst | 1 | ||||
| -rw-r--r-- | src/mailman/rest/lists.py | 10 | ||||
| -rw-r--r-- | src/mailman/rest/tests/test_lists.py | 22 | ||||
| -rw-r--r-- | src/mailman/rest/tests/test_systemconf.py | 1 |
9 files changed, 110 insertions, 2 deletions
diff --git a/src/mailman/app/lifecycle.py b/src/mailman/app/lifecycle.py index 79ab20c8a..e79073929 100644 --- a/src/mailman/app/lifecycle.py +++ b/src/mailman/app/lifecycle.py @@ -17,6 +17,7 @@ """Application level list creation.""" +import re import shutil import logging @@ -26,6 +27,7 @@ from mailman.interfaces.address import IEmailValidator from mailman.interfaces.domain import ( BadDomainSpecificationError, IDomainManager) from mailman.interfaces.listmanager import IListManager +from mailman.interfaces.mailinglist import InvalidListNameError from mailman.interfaces.member import MemberRole from mailman.interfaces.styles import IStyleManager from mailman.interfaces.usermanager import IUserManager @@ -35,6 +37,9 @@ from zope.component import getUtility log = logging.getLogger('mailman.error') +# These are the only characters allowed in list names. A more restrictive +# class can be specified in config.mailman.listname_chars. +_listname_chars = re.compile('[-_.+=!$*{}~0-9a-z]', re.IGNORECASE) @public @@ -57,6 +62,8 @@ def create_list(fqdn_listname, owners=None, style_name=None): `fqdn_listname` does not exist. :raises ListAlreadyExistsError: when the mailing list already exists. :raises InvalidEmailAddressError: when the fqdn email address is invalid. + :raises InvalidListNameError: when the fqdn email address is valid but the + listname contains disallowed characters. """ if owners is None: owners = [] @@ -64,6 +71,24 @@ def create_list(fqdn_listname, owners=None, style_name=None): # posting address. Let these percolate up. getUtility(IEmailValidator).validate(fqdn_listname) listname, domain = fqdn_listname.split('@', 1) + # We need to be fussier than just validating the posting address. Various + # legal local-part characters will cause problems in list names. + # First we check our maximally allowed set. + if len(_listname_chars.sub('', listname)) > 0: + raise InvalidListNameError(listname) + # Then if another set is configured, check that. + if config.mailman.listname_chars: + try: + cre = re.compile(config.mailman.listname_chars, re.IGNORECASE) + except re.error as error: + log.error( + 'Bad config.mailman.listname_chars setting: %s: %s', + config.mailman.listname_chars, + getattr(error, 'msg', str(error)) + ) + else: + if len(cre.sub('', listname)) > 0: + raise InvalidListNameError(listname) if domain not in getUtility(IDomainManager): raise BadDomainSpecificationError(domain) mlist = getUtility(IListManager).create(fqdn_listname) diff --git a/src/mailman/app/tests/test_lifecycle.py b/src/mailman/app/tests/test_lifecycle.py index f504e3dfd..dd320c1da 100644 --- a/src/mailman/app/tests/test_lifecycle.py +++ b/src/mailman/app/tests/test_lifecycle.py @@ -21,10 +21,12 @@ import os import shutil import unittest -from mailman.app.lifecycle import create_list, remove_list +from mailman.app.lifecycle import ( + InvalidListNameError, create_list, remove_list) from mailman.interfaces.address import InvalidEmailAddressError from mailman.interfaces.domain import BadDomainSpecificationError from mailman.interfaces.listmanager import IListManager +from mailman.testing.helpers import LogFileMark, configuration from mailman.testing.layers import ConfigLayer from zope.component import getUtility @@ -39,6 +41,35 @@ class TestLifecycle(unittest.TestCase): self.assertRaises(InvalidEmailAddressError, create_list, 'bogus address') + def test_listname_validation(self): + # Creating a mailing list with invalid characters in the listname + # raises an exception. + self.assertRaises(InvalidListNameError, + create_list, 'my/list@example.com') + + @configuration('mailman', listname_chars='[a-z0-9-+\]') + def test_bad_config_listname_chars(self): + mark = LogFileMark('mailman.error') + # This list create should succeed but log an error + mlist = create_list('test@example.com') + # Check the error log. + self.assertRegex( + mark.readline(), + '^.*Bad config\.mailman\.listname_chars setting: ' + '\[a-z0-9-\+\\\]: ' + '(unterminated character set|' + 'unexpected end of regular expression)$' + ) + # Check that the list was actually created. + self.assertIs(os.path.isdir(mlist.data_path), True) + + @configuration('mailman', listname_chars='[a-z]') + def test_listname_with_minimal_listname_chars(self): + # This only allows letters in the listname. A listname with digits + # Raises an exception. + self.assertRaises(InvalidListNameError, + create_list, 'list1@example.com') + def test_unregistered_domain(self): # Creating a list with an unregistered domain raises an exception. self.assertRaises(BadDomainSpecificationError, diff --git a/src/mailman/config/schema.cfg b/src/mailman/config/schema.cfg index 6b6362ab4..fde431b60 100644 --- a/src/mailman/config/schema.cfg +++ b/src/mailman/config/schema.cfg @@ -75,6 +75,13 @@ filtered_messages_are_preservable: no # The command should print the converted text to stdout. html_to_plain_text_command: /usr/bin/lynx -dump $filename +# Specify what characters are allowed in list names. Characters outside of +# the class [-_.+=!$*{}~0-9a-z] matched case insensitively are never allowed, +# but this specifies a subset as the only allowable characters. This must be +# a valid character class regexp or the effect on list creation is +# unpredictable. +listname_chars: [-_.0-9a-z] + [shell] # `mailman shell` (also `withlist`) gives you an interactive prompt that you diff --git a/src/mailman/docs/NEWS.rst b/src/mailman/docs/NEWS.rst index d2d4a39d2..ffd6c16a6 100644 --- a/src/mailman/docs/NEWS.rst +++ b/src/mailman/docs/NEWS.rst @@ -109,6 +109,7 @@ Bugs * Fix ``mailman stop`` not stopping some runners due to PEP 475 interaction. (Closes: #255) * Update documentation links for ``config.cfg`` settings. (Closes: #306) + * Disallow problematic characters in listnames. (Closes: #311) Configuration ------------- @@ -119,6 +120,8 @@ Configuration rules is not yet exposed through the REST API. Given by Aurélien Bompard. * The default languages from Mailman 2.1 have been ported over. Given by Aurélien Bompard. + * There is now a configuration setting to limit the characters that can be + used in list names. Command line ------------ diff --git a/src/mailman/interfaces/mailinglist.py b/src/mailman/interfaces/mailinglist.py index ddc5a13b6..f066ba3c8 100644 --- a/src/mailman/interfaces/mailinglist.py +++ b/src/mailman/interfaces/mailinglist.py @@ -18,12 +18,22 @@ """Interface for a mailing list.""" from enum import Enum +from mailman.interfaces.address import InvalidEmailAddressError from mailman.interfaces.member import MemberRole from public import public from zope.interface import Attribute, Interface @public +class InvalidListNameError(InvalidEmailAddressError): + """List name is invalid.""" + + def __init__(self, listname): + super().__init__('{}@'.format(listname)) + self.listname = listname + + +@public class DMARCMitigateAction(Enum): # Mitigations to apply to messages From: domains publishing an applicable # DMARC policy, or unconditionally depending on settings. diff --git a/src/mailman/rest/docs/systemconf.rst b/src/mailman/rest/docs/systemconf.rst index e61dd2cc9..213929a42 100644 --- a/src/mailman/rest/docs/systemconf.rst +++ b/src/mailman/rest/docs/systemconf.rst @@ -20,6 +20,7 @@ You can also get all the values for a particular section, such as the html_to_plain_text_command: /usr/bin/lynx -dump $filename http_etag: ... layout: testing + listname_chars: [-_.0-9a-z] noreply_address: noreply pending_request_life: 3d post_hook: diff --git a/src/mailman/rest/lists.py b/src/mailman/rest/lists.py index e665515ec..4b467afb3 100644 --- a/src/mailman/rest/lists.py +++ b/src/mailman/rest/lists.py @@ -20,8 +20,10 @@ from lazr.config import as_boolean from mailman.app.digests import ( bump_digest_number_and_volume, maybe_send_digest_now) -from mailman.app.lifecycle import create_list, remove_list +from mailman.app.lifecycle import ( + InvalidListNameError, create_list, remove_list) from mailman.config import config +from mailman.interfaces.address import InvalidEmailAddressError from mailman.interfaces.domain import BadDomainSpecificationError from mailman.interfaces.listmanager import ( IListManager, ListAlreadyExistsError) @@ -246,6 +248,12 @@ class AllLists(_ListBase): except BadDomainSpecificationError as error: reason = 'Domain does not exist: {}'.format(error.domain) bad_request(response, reason.encode('utf-8')) + except InvalidListNameError as error: + reason = 'Invalid list name: {}'.format(error.listname) + bad_request(response, reason.encode('utf-8')) + except InvalidEmailAddressError as error: + reason = 'Invalid list posting address: {}'.format(error.email) + bad_request(response, reason.encode('utf-8')) else: location = self.api.path_to('lists/{0}'.format(mlist.list_id)) created(response, location) diff --git a/src/mailman/rest/tests/test_lists.py b/src/mailman/rest/tests/test_lists.py index 7bb7bd333..75a81f3cb 100644 --- a/src/mailman/rest/tests/test_lists.py +++ b/src/mailman/rest/tests/test_lists.py @@ -129,6 +129,28 @@ class TestLists(unittest.TestCase): self.assertEqual(cm.exception.reason, 'Domain does not exist: no-domain.example.org') + def test_cannot_create_list_with_invalid_posting_address(self): + # You cannot create a mailing list which would have an invalid list + # posting address. + with self.assertRaises(HTTPError) as cm: + call_api('http://localhost:9001/3.0/lists', { + 'fqdn_listname': '@example.com', + }) + self.assertEqual(cm.exception.code, 400) + self.assertEqual(cm.exception.reason, + 'Invalid list posting address: @example.com') + + def test_cannot_create_list_with_invalid_name(self): + # You cannot create a mailing list which would have an invalid list + # posting address. + with self.assertRaises(HTTPError) as cm: + call_api('http://localhost:9001/3.0/lists', { + 'fqdn_listname': 'a/list@example.com', + }) + self.assertEqual(cm.exception.code, 400) + self.assertEqual(cm.exception.reason, + 'Invalid list name: a/list') + def test_cannot_create_duplicate_list(self): # You cannot create a list that already exists. call_api('http://localhost:9001/3.0/lists', { diff --git a/src/mailman/rest/tests/test_systemconf.py b/src/mailman/rest/tests/test_systemconf.py index 570342dea..afa788719 100644 --- a/src/mailman/rest/tests/test_systemconf.py +++ b/src/mailman/rest/tests/test_systemconf.py @@ -43,6 +43,7 @@ class TestSystemConfiguration(unittest.TestCase): filtered_messages_are_preservable='no', html_to_plain_text_command='/usr/bin/lynx -dump $filename', layout='testing', + listname_chars='[-_.0-9a-z]', noreply_address='noreply', pending_request_life='3d', post_hook='', |
