From 5dfe5a2831adbe3ec129d2ede1d9039739e98b71 Mon Sep 17 00:00:00 2001 From: Anton Khirnov Date: Sat, 30 Jan 2021 13:33:27 +0100 Subject: db/envelope: switch to the "new" (EmailMessage) python API email.mime is a part of the old API, which does not mix well with the new one (i.e. when email.policy.SMTP is used), specifically when non-ASCII headers are used. Additionally, clean the APIs that accept either EmailMessage or a str to only expect EmailMessage. Supporting both just adds confusion and complexity. --- alot/account.py | 14 +++--- alot/commands/envelope.py | 20 ++------ alot/db/envelope.py | 114 +++++++++++++++++++++++----------------------- tests/db/test_envelope.py | 18 +++++--- 4 files changed, 79 insertions(+), 87 deletions(-) diff --git a/alot/account.py b/alot/account.py index 1db65a5f..87c71390 100644 --- a/alot/account.py +++ b/alot/account.py @@ -5,6 +5,7 @@ # For further details see the COPYING file import abc import asyncio +from email import policy import glob import logging import mailbox @@ -305,7 +306,7 @@ class Account: def store_sent_mail(self, mail): """ - stores mail (:class:`email.message.Message` or str) in send-store if + stores mail (:class:`email.message.EmailMessage`) in send-store if :attr:`sent_box` is set. """ if self.sent_box is not None: @@ -313,7 +314,7 @@ class Account: def store_draft_mail(self, mail): """ - stores mail (:class:`email.message.Message` or str) as draft if + stores mail (:class:`email.message.EmailMessage`) as draft if :attr:`draft_box` is set. """ if self.draft_box is not None: @@ -325,7 +326,7 @@ class Account: sends given mail :param mail: the mail to send - :type mail: :class:`email.message.Message` or string + :type mail: :class:`email.message.EmailMessage` :raises SendingMailFailed: if sending fails """ pass @@ -347,13 +348,14 @@ class SendmailAccount(Account): """Pipe the given mail to the configured sendmail command. Display a short message on success or a notification on error. :param mail: the mail to send out - :type mail: bytes + :type mail: email.message.EmailMessage :raises: class:`SendingMailFailed` if sending failes """ - P = asyncio.subprocess.PIPE + data = mail.as_bytes() + P = asyncio.subprocess.PIPE child = await asyncio.create_subprocess_shell(self.cmd, P, P, P) - out, err = await child.communicate(mail) + out, err = await child.communicate(data) if child.returncode != 0: msg = 'Calling sendmail command %s failed with code %s' % \ diff --git a/alot/commands/envelope.py b/alot/commands/envelope.py index c146dacb..47e7463e 100644 --- a/alot/commands/envelope.py +++ b/alot/commands/envelope.py @@ -5,7 +5,6 @@ import argparse import datetime import email -import email.policy import glob import logging import os @@ -130,8 +129,7 @@ class SaveCommand(Command): mail = envelope.construct_mail() # store mail locally - path = account.store_draft_mail( - mail.as_string(policy=email.policy.SMTP)) + path = account.store_draft_mail(mail) msg = 'draft saved successfully' @@ -239,7 +237,6 @@ class SendCommand(Command): try: mail = envelope.construct_mail() - mail = mail.as_string(policy=email.policy.SMTP) except GPGProblem as e: ui.clear_notify(clearme) ui.notify(str(e), priority='error') @@ -248,11 +245,7 @@ class SendCommand(Command): ui.clear_notify(clearme) # determine account to use for sending - msg = mail - if not isinstance(msg, email.message.Message): - msg = email.message_from_string( - mail, policy=email.policy.SMTP) - address = msg.get('Resent-From', False) or msg.get('From', '') + address = mail.get('Resent-From', False) or mail.get('From', '') logging.debug("FROM: \"%s\"" % address) try: account = settings.account_matching_address(address, @@ -267,15 +260,8 @@ class SendCommand(Command): if envelope is not None: envelope.sending = True - # FIXME XXX horrible hack, fix the type fuckery - to_send = mail - if isinstance(to_send, str): - to_send = to_send.encode('ascii') - else: - to_send = to_send.as_bytes() - try: - await account.send_mail(to_send) + await account.send_mail(mail) except SendingMailFailed as e: if envelope is not None: envelope.account = account diff --git a/alot/db/envelope.py b/alot/db/envelope.py index 8340ba4e..f9909308 100644 --- a/alot/db/envelope.py +++ b/alot/db/envelope.py @@ -8,12 +8,11 @@ import re import email import email.policy from email.encoders import encode_7or8bit +from email.message import MIMEPart from email.mime.audio import MIMEAudio from email.mime.base import MIMEBase from email.mime.image import MIMEImage from email.mime.text import MIMEText -from email.mime.multipart import MIMEMultipart -from email.mime.application import MIMEApplication import email.charset as charset from urllib.parse import unquote @@ -292,29 +291,33 @@ class Envelope: def construct_mail(self): """ compiles the information contained in this envelope into a - :class:`email.Message`. + :class:`email.message.EmailMessage`. """ - # Build body text part. To properly sign/encrypt messages later on, we - # convert the text to its canonical format (as per RFC 2015). - canonical_format = self.body.encode('utf-8') - textpart = MIMEText(canonical_format, 'plain', 'utf-8') - - # wrap it in a multipart container if necessary - if self.attachments: - inner_msg = MIMEMultipart() - inner_msg.attach(textpart) - # add attachments - for a in self.attachments: - inner_msg.attach(a.get_mime_representation()) - else: - inner_msg = textpart + # make suire everything is 7-bit clean to avoid + # compatibility problems + # TODO: consider SMTPUTF8 support? + policy = email.policy.SMTP.clone(cte_type = '7bit') + + # we actually use MIMEPart instead of EmailMessage, to + # avoid the subparts getting spurious MIME-Version everywhere + mail = MIMEPart(policy = policy) + mail.set_content(self.body, subtype = 'plain', charset = 'utf-8') + + # add attachments + for a in self.attachments: + maintype, _, subtype = a.get_content_type().partition('/') + fname = a.get_filename() + data = a.get_data() + mail.add_attachment(data, filename = fname, + maintype = maintype, subtype = subtype) if self.sign: - plaintext = inner_msg.as_bytes(policy=email.policy.SMTP) + to_sign = mail + plaintext = to_sign.as_bytes() logging.debug('signing plaintext: %s', plaintext) try: - signatures, signature_str = crypto.detached_signature_for( + signatures, signature_blob = crypto.detached_signature_for( plaintext, [self.sign_key]) if len(signatures) != 1: raise GPGProblem("Could not sign message (GPGME " @@ -334,55 +337,47 @@ class Envelope: code=GPGCode.BAD_PASSPHRASE) raise GPGProblem(str(e), code=GPGCode.KEY_CANNOT_SIGN) - micalg = crypto.RFC3156_micalg_from_algo(signatures[0].hash_algo) - unencrypted_msg = MIMEMultipart( - 'signed', micalg=micalg, protocol='application/pgp-signature') - - # wrap signature in MIMEcontainter - stype = 'pgp-signature; name="signature.asc"' - signature_mime = MIMEApplication( - _data=signature_str.decode('ascii'), - _subtype=stype, - _encoder=encode_7or8bit) + signature_mime = MIMEPart(policy = to_sign.policy) + signature_mime.set_content(signature_blob, maintype = 'application', + subtype = 'pgp-signature') + signature_mime.set_param('name', 'signature.asc') signature_mime['Content-Description'] = 'signature' - signature_mime.set_charset('us-ascii') - # add signed message and signature to outer message - unencrypted_msg.attach(inner_msg) - unencrypted_msg.attach(signature_mime) - unencrypted_msg['Content-Disposition'] = 'inline' - else: - unencrypted_msg = inner_msg + # FIXME: this uses private methods, because + # python's "new" EmailMessage API does not + # allow arbitrary multipart constructs + mail = MIMEPart(policy = to_sign.policy) + mail._make_multipart('signed', (), None) + mail.set_param('protocol', 'application/pgp-signature') + mail.set_param('micalg', crypto.RFC3156_micalg_from_algo(signatures[0].hash_algo)) + mail.attach(to_sign) + mail.attach(signature_mime) if self.encrypt: - plaintext = unencrypted_msg.as_bytes(policy=email.policy.SMTP) + to_encrypt = mail + plaintext = to_encrypt.as_bytes() logging.debug('encrypting plaintext: %s', plaintext) try: - encrypted_str = crypto.encrypt( + encrypted_blob = crypto.encrypt( plaintext, list(self.encrypt_keys.values())) except gpg.errors.GPGMEError as e: raise GPGProblem(str(e), code=GPGCode.KEY_CANNOT_ENCRYPT) - outer_msg = MIMEMultipart('encrypted', - protocol='application/pgp-encrypted') + version_str = b'Version: 1' + encryption_mime = MIMEPart(policy = to_encrypt.policy) + encryption_mime.set_content(version_str, maintype = 'application', + subtype = 'pgp-encrypted') - version_str = 'Version: 1' - encryption_mime = MIMEApplication(_data=version_str, - _subtype='pgp-encrypted', - _encoder=encode_7or8bit) - encryption_mime.set_charset('us-ascii') + encrypted_mime = MIMEPart(policy = to_encrypt.policy) + encrypted_mime.set_content(encrypted_blob, maintype = 'application', + subtype = 'octet-stream') - encrypted_mime = MIMEApplication( - _data=encrypted_str.decode('ascii'), - _subtype='octet-stream', - _encoder=encode_7or8bit) - encrypted_mime.set_charset('us-ascii') - outer_msg.attach(encryption_mime) - outer_msg.attach(encrypted_mime) - - else: - outer_msg = unencrypted_msg + mail = MIMEPart(policy = to_encrypt.policy) + mail._make_multipart('encrypted', (), None) + mail.set_param('protocol', 'application/pgp-encrypted') + mail.attach(encryption_mime) + mail.attach(encrypted_mime) headers = self.headers.copy() @@ -406,9 +401,14 @@ class Envelope: # copy headers from envelope to mail for k, vlist in headers.items(): for v in vlist: - outer_msg.add_header(k, v) + mail.add_header(k, v) + + # as we are using MIMEPart instead of EmailMessage, set the + # MIME version manually + del mail['MIME-Version'] + mail['MIME-Version'] = '1.0' - return outer_msg + return mail def parse_template(self, raw, reset=False, only_body=False): """parses a template or user edited string to fills this envelope. diff --git a/tests/db/test_envelope.py b/tests/db/test_envelope.py index 86e481bf..0d46ba43 100644 --- a/tests/db/test_envelope.py +++ b/tests/db/test_envelope.py @@ -14,7 +14,7 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . -import email.parser +import email import email.policy import os import tempfile @@ -27,21 +27,25 @@ SETTINGS = { 'user_agent': 'agent', } - class TestEnvelope(unittest.TestCase): + def _compare_content(self, first, second): + c1 = first.get_content().replace('\r\n', '\n') + c2 = second.get_content().replace('\r\n', '\n') + self.assertEqual(c1, c2) + def assertEmailEqual(self, first, second): with self.subTest('body'): self.assertEqual(first.is_multipart(), second.is_multipart()) if not first.is_multipart(): - self.assertEqual(first.get_payload(), second.get_payload()) + self._compare_content(first, second) else: for f, s in zip(first.walk(), second.walk()): if f.is_multipart() or s.is_multipart(): self.assertEqual(first.is_multipart(), second.is_multipart()) else: - self.assertEqual(f.get_payload(), s.get_payload()) + self._compare_content(f, s) with self.subTest('headers'): self.assertListEqual(first.values(), second.values()) @@ -52,9 +56,9 @@ class TestEnvelope(unittest.TestCase): self.assertEqual(e['Subject'], 'sm\xf8rebr\xf8d') def _test_mail(self, envelope): - mail = envelope.construct_mail() - raw = mail.as_string(policy=email.policy.SMTP) - actual = email.parser.Parser().parsestr(raw) + mail = envelope.construct_mail() + raw = mail.as_bytes() + actual = email.message_from_bytes(raw, policy = mail.policy) self.assertEmailEqual(mail, actual) @mock.patch('alot.db.envelope.settings', SETTINGS) -- cgit v1.2.3