diff options
Diffstat (limited to 'src/mailman')
| -rw-r--r-- | src/mailman/docs/NEWS.rst | 6 | ||||
| -rw-r--r-- | src/mailman/handlers/mime_delete.py | 7 | ||||
| -rw-r--r-- | src/mailman/handlers/tests/test_mimedel.py | 82 |
3 files changed, 88 insertions, 7 deletions
diff --git a/src/mailman/docs/NEWS.rst b/src/mailman/docs/NEWS.rst index 2bc2335f3..df8032284 100644 --- a/src/mailman/docs/NEWS.rst +++ b/src/mailman/docs/NEWS.rst @@ -12,6 +12,12 @@ Here is a history of user visible changes to Mailman. ============================== (201X-XX-XX) +Bugs +---- + * A missing html_to_plain_text_command is now properly detected and logged. + (closes #345) + + 3.1.0 -- "Between The Wheels" ============================= (2017-05-25) diff --git a/src/mailman/handlers/mime_delete.py b/src/mailman/handlers/mime_delete.py index 14aee1a0e..662c773ae 100644 --- a/src/mailman/handlers/mime_delete.py +++ b/src/mailman/handlers/mime_delete.py @@ -28,7 +28,6 @@ import os import shutil import logging import tempfile -import subprocess from contextlib import ExitStack, suppress from email.iterators import typed_subpart_iterator @@ -46,6 +45,7 @@ from mailman.utilities.string import oneline from mailman.version import VERSION from public import public from string import Template +from subprocess import CalledProcessError, check_output from zope.interface import implementer @@ -277,9 +277,8 @@ def to_plaintext(msg): template = Template(config.mailman.html_to_plain_text_command) command = template.safe_substitute(filename=filename).split() try: - stdout = subprocess.check_output( - command, universal_newlines=True) - except subprocess.CalledProcessError: + stdout = check_output(command, universal_newlines=True) + except (CalledProcessError, FileNotFoundError, PermissionError): log.exception('HTML -> text/plain command error') else: # Replace the payload of the subpart with the converted text diff --git a/src/mailman/handlers/tests/test_mimedel.py b/src/mailman/handlers/tests/test_mimedel.py index 3a50852da..cde557d78 100644 --- a/src/mailman/handlers/tests/test_mimedel.py +++ b/src/mailman/handlers/tests/test_mimedel.py @@ -43,22 +43,34 @@ from zope.component import getUtility @contextmanager -def dummy_script(): +def dummy_script(arg=''): + exe = sys.executable + extra = '' + if arg == 'scripterr': + extra = 'error' with ExitStack() as resources: tempdir = tempfile.mkdtemp() resources.callback(shutil.rmtree, tempdir) filter_path = os.path.join(tempdir, 'filter.py') + if arg in ('noperm', 'nonexist'): + exe = filter_path with open(filter_path, 'w', encoding='utf-8') as fp: print("""\ import sys +if len(sys.argv) > 2: + sys.exit(1) print('Converted text/html to text/plain') print('Filename:', sys.argv[1]) """, file=fp) config.push('dummy script', """\ [mailman] -html_to_plain_text_command = {exe} {script} $filename -""".format(exe=sys.executable, script=filter_path)) +html_to_plain_text_command = {exe} {script} {extra} $filename +""".format(exe=exe, script=filter_path, extra=extra)) resources.callback(config.pop, 'dummy script') + if arg == 'nonexist': + os.rename(filter_path, filter_path + 'xxx') + elif arg == 'noperm': + os.chmod(filter_path, 0o644) yield @@ -220,6 +232,70 @@ MIME-Version: 1.0 payload_lines = msg.get_payload().splitlines() self.assertEqual(payload_lines[0], 'Converted text/html to text/plain') + def test_convert_html_to_plaintext_error_return(self): + # Calling a script which returns an error status is properly logged. + msg = mfs("""\ +From: aperson@example.com +Content-Type: text/html +MIME-Version: 1.0 + +<html><head></head> +<body></body></html> +""") + process = config.handlers['mime-delete'].process + mark = LogFileMark('mailman.error') + with dummy_script('scripterr'): + process(self._mlist, msg, {}) + line = mark.readline()[:-1] + self.assertTrue(line.endswith('HTML -> text/plain command error')) + self.assertEqual(msg.get_content_type(), 'text/html') + self.assertIsNone(msg['x-content-filtered-by']) + payload_lines = msg.get_payload().splitlines() + self.assertEqual(payload_lines[0], '<html><head></head>') + + def test_missing_html_to_plain_text_command(self): + # Calling a missing html_to_plain_text_command is properly logged. + msg = mfs("""\ +From: aperson@example.com +Content-Type: text/html +MIME-Version: 1.0 + +<html><head></head> +<body></body></html> +""") + process = config.handlers['mime-delete'].process + mark = LogFileMark('mailman.error') + with dummy_script('nonexist'): + process(self._mlist, msg, {}) + line = mark.readline()[:-1] + self.assertTrue(line.endswith('HTML -> text/plain command error')) + self.assertEqual(msg.get_content_type(), 'text/html') + self.assertIsNone(msg['x-content-filtered-by']) + payload_lines = msg.get_payload().splitlines() + self.assertEqual(payload_lines[0], '<html><head></head>') + + def test_no_permission_html_to_plain_text_command(self): + # Calling an html_to_plain_text_command without permission is + # properly logged. + msg = mfs("""\ +From: aperson@example.com +Content-Type: text/html +MIME-Version: 1.0 + +<html><head></head> +<body></body></html> +""") + process = config.handlers['mime-delete'].process + mark = LogFileMark('mailman.error') + with dummy_script('noperm'): + process(self._mlist, msg, {}) + line = mark.readline()[:-1] + self.assertTrue(line.endswith('HTML -> text/plain command error')) + self.assertEqual(msg.get_content_type(), 'text/html') + self.assertIsNone(msg['x-content-filtered-by']) + payload_lines = msg.get_payload().splitlines() + self.assertEqual(payload_lines[0], '<html><head></head>') + class TestMiscellaneous(unittest.TestCase): """Test various miscellaneous filtering actions.""" |