Abhilash Raj pushed to branch master at GNU Mailman / Mailman Core
Commits: 53d9d0ab by Mark Sapiro at 2017-11-11T13:54:27-08:00 Defend against certain malformed messages. Accommodate https://bugs.python.org/issue27321 by overriding as_string() in mailman.email.message.Message to catch the KeyError, get the message as_bytes() and convert it to a string. This obviates !197 as a fix for #256 so that is reverted. Also fix mailman.mta.connection.Connection.sendmail to ensure msgtext is pure ascii. - - - - - 066832bb by Abhilash Raj at 2017-12-14T22:13:58+00:00 Merge branch 'as_string' into 'master' Defend against certain malformed messages. Closes #406 and #256 See merge request mailman/mailman!322 - - - - - 8 changed files: - src/mailman/docs/NEWS.rst - src/mailman/email/message.py - src/mailman/rest/tests/data/__init__.py → src/mailman/email/tests/data/__init__.py - src/mailman/rest/tests/data/bad_email.eml → src/mailman/email/tests/data/bad_email.eml - src/mailman/email/tests/test_message.py - src/mailman/mta/connection.py - src/mailman/rest/post_moderation.py - src/mailman/rest/tests/test_moderation.py Changes: ===================================== src/mailman/docs/NEWS.rst ===================================== --- a/src/mailman/docs/NEWS.rst +++ b/src/mailman/docs/NEWS.rst @@ -41,6 +41,9 @@ Bugs * The ``admin`` subaddress, a synonym for ``bounces`` and deprecated since Mailman 2.1, has been removed. (Closes #435) * Better support for changing the ``list_name`` property. (Closes #428) +* Raw Message text is now coerced to pure ascii before sending, and + https://bugs.python.org/issue27321 is now defended against by overriding + Message.as_string() to cover more cases than before. (Closes #406) Command line ------------ ===================================== src/mailman/email/message.py ===================================== --- a/src/mailman/email/message.py +++ b/src/mailman/email/message.py @@ -47,6 +47,15 @@ class Message(email.message.Message): def __setstate__(self, values): self.__dict__ = values + def as_string(self): + # Work around for https://bugs.python.org/issue27321. + try: + value = email.message.Message.as_string(self) + except KeyError: + value = email.message.Message.as_bytes(self).decode( + 'ascii', 'replace') + return value + @property def sender(self): """The address considered to be the author of the email. ===================================== src/mailman/rest/tests/data/__init__.py → src/mailman/email/tests/data/__init__.py ===================================== ===================================== src/mailman/rest/tests/data/bad_email.eml → src/mailman/email/tests/data/bad_email.eml ===================================== ===================================== src/mailman/email/tests/test_message.py ===================================== --- a/src/mailman/email/tests/test_message.py +++ b/src/mailman/email/tests/test_message.py @@ -19,12 +19,14 @@ import unittest +from email import message_from_binary_file from email.header import Header from email.parser import FeedParser from mailman.app.lifecycle import create_list from mailman.email.message import Message, UserNotification from mailman.testing.helpers import get_queue_messages from mailman.testing.layers import ConfigLayer +from pkg_resources import resource_filename class TestMessage(unittest.TestCase): @@ -88,3 +90,12 @@ Test content msg['From'] = Header('t...@example.com') # Make sure the senders property does not fail self.assertEqual(msg.senders, ['t...@example.com']) + + def test_as_string_python_bug_27321(self): + email_path = resource_filename( + 'mailman.email.tests.data', 'bad_email.eml') + with open(email_path, 'rb') as fp: + msg = message_from_binary_file(fp, Message) + fp.seek(0) + text = fp.read().decode('ascii', 'replace') + self.assertEqual(msg.as_string(), text) ===================================== src/mailman/mta/connection.py ===================================== --- a/src/mailman/mta/connection.py +++ b/src/mailman/mta/connection.py @@ -78,6 +78,10 @@ class Connection: recipients = [config.devmode.recipient] * len(recipients) if self._connection is None: self._connect() + # smtplib.SMTP.sendmail requires the message string to be pure ascii. + # We have seen malformed messages with non-ascii unicodes, so ensure + # we have pure ascii. + msgtext = msgtext.encode('ascii', 'replace').decode('ascii') try: log.debug('envsender: %s, recipients: %s, size(msgtext): %s', envsender, recipients, len(msgtext)) ===================================== src/mailman/rest/post_moderation.py ===================================== --- a/src/mailman/rest/post_moderation.py +++ b/src/mailman/rest/post_moderation.py @@ -74,14 +74,7 @@ class _HeldMessageBase(_ModerationBase): # resource. XXX See LP: #967954 key = resource.pop('key') msg = getUtility(IMessageStore).get_message_by_id(key) - 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' + resource['msg'] = msg.as_string() # 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, ===================================== src/mailman/rest/tests/test_moderation.py ===================================== --- a/src/mailman/rest/tests/test_moderation.py +++ b/src/mailman/rest/tests/test_moderation.py @@ -19,7 +19,6 @@ 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 @@ -32,7 +31,6 @@ 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 @@ -227,24 +225,6 @@ class TestSubscriptionModeration(unittest.TestCase): emails = set(entry['email'] for entry in json['entries']) self.assertEqual(emails, {'a...@example.com', 'b...@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 = 'aper...@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. - json, response = call_api( - 'http://localhost:9001/3.0/lists/a...@example.com/held') - self.assertEqual(response.status_code, 200) - self.assertEqual(len(json['entries']), 1) - self.assertEqual(json['entries'][0]['msg'], - 'This message is defective') - def test_individual_request(self): # We can view an individual request. with transaction(): View it on GitLab: https://gitlab.com/mailman/mailman/compare/06764cdd23f11539ac47b21b6b066b1ca40ababb...066832bb23b744d84710aa35c33908e36021f7ce --- View it on GitLab: https://gitlab.com/mailman/mailman/compare/06764cdd23f11539ac47b21b6b066b1ca40ababb...066832bb23b744d84710aa35c33908e36021f7ce You're receiving this email because of your account on gitlab.com.
_______________________________________________ Mailman-checkins mailing list Mailman-checkins@python.org Unsubscribe: https://mail.python.org/mailman/options/mailman-checkins/archive%40jab.org
