Merge lp:~qwertyface/localmail/localmail into lp:localmail

[Peter Russell]

[lp:~qwertyface/localmail/localmail]

54. By Peter Russell on 2018-04-13

Linter fixes.

[Simon Davy (bloodearnest)]

Thanks! Sorry for the delay, I was on holiday!

I'll merge this and do pypi release on monday.

Thanks again!

[Simon Davy (bloodearnest)]

review: Approve

[Peter Russell (qwertyface)]

Actually there are done encoding bugs I've seen since making the merge
request. Best hold of from making a release for now.

Peter

On Sun, 15 Apr 2018, 19:48 Simon Davy, <email address hidden> wrote:

> Thanks! Sorry for the delay, I was on holiday!
>
> I'll merge this and do pypi release on monday.
>
> Thanks again!
> --
> https://code.launchpad.net/~qwertyface/localmail/localmail/+merge/343177
> You are the owner of lp:~qwertyface/localmail/localmail.
>

[lp:~qwertyface/localmail/localmail]

55. By Peter Russell on 2018-05-01

Fix a few errors relating to non-ascii mails

* Correctly convert message payloads to bytes in inbox.py
* Correctly decode message headers in inbox.py
* Convert a few strings including '\' characts to raw strings
* Add tests which round-trip non-ascii mail with differing encodings
  (these could probably do with a refactor)

[Peter Russell (qwertyface)]

I've updated the branch with fixes for the encoding related bugs. Could you
review again and merge?

Peter

On 15 April 2018 at 19:51, Peter Russell <email address hidden> wrote:

> Actually there are done encoding bugs I've seen since making the merge
> request. Best hold of from making a release for now.
>
> Peter
>
> On Sun, 15 Apr 2018, 19:48 Simon Davy, <email address hidden> wrote:
>
> > Thanks! Sorry for the delay, I was on holiday!
> >
> > I'll merge this and do pypi release on monday.
> >
> > Thanks again!
> > --
> > https://code.launchpad.net/~qwertyface/localmail/localmail/+merge/343177
> > You are the owner of lp:~qwertyface/localmail/localmail.
> >
>
> --
> https://code.launchpad.net/~qwertyface/localmail/localmail/+merge/343177
> You are the owner of lp:~qwertyface/localmail/localmail.
>

=== modified file '.bzrignore'
--- .bzrignore	2014-09-09 11:49:58 +0000
+++ .bzrignore	2018-05-01 09:16:46 +0000
@@ -6,3 +6,4 @@
 build
 .tox
 *.egg
+.eggs/
 
=== modified file 'localmail/cred.py'
--- localmail/cred.py	2012-11-13 14:25:36 +0000
+++ localmail/cred.py	2018-05-01 09:16:46 +0000
@@ -13,17 +13,17 @@
 # You should have received a copy of the GNU General Public License
 # along with this program; if not, write to the Free Software
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
-from zope.interface import implements
+from zope.interface import implementer
 from twisted.internet import defer
 from twisted.cred import portal, checkers, credentials
 from twisted.mail import smtp, imap4
 
-from imap import IMAPUserAccount
-from smtp import MemoryDelivery
-
-
+from .imap import IMAPUserAccount
+from .smtp import MemoryDelivery
+
+
+@implementer(portal.IRealm)
 class TestServerRealm(object):
-    implements(portal.IRealm)
     avatarInterfaces = {
         imap4.IAccount: IMAPUserAccount,
         smtp.IMessageDelivery: MemoryDelivery,
@@ -34,16 +34,19 @@
             if requestedInterface in self.avatarInterfaces:
                 avatarClass = self.avatarInterfaces[requestedInterface]
                 avatar = avatarClass()
+
                 # null logout function: take no arguments and do nothing
-                logout = lambda: None
+                def logout():
+                    pass
+
                 return defer.succeed((requestedInterface, avatar, logout))
 
         # none of the requested interfaces was supported
         raise KeyError("None of the requested interfaces is supported")
 
 
+@implementer(checkers.ICredentialsChecker)
 class CredentialsNonChecker(object):
-    implements(checkers.ICredentialsChecker)
     credentialInterfaces = (credentials.IUsernamePassword,
                             credentials.IUsernameHashedPassword)
 
 
=== modified file 'localmail/imap.py'
--- localmail/imap.py	2014-09-09 16:41:59 +0000
+++ localmail/imap.py	2018-05-01 09:16:46 +0000
@@ -15,13 +15,13 @@
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
 from twisted.internet import protocol
 from twisted.mail import imap4
-from zope.interface import implements
-
-from inbox import INBOX
-
-
+from zope.interface import implementer
+
+from .inbox import INBOX
+
+
+@implementer(imap4.IAccount)
 class IMAPUserAccount(object):
-    implements(imap4.IAccount)
 
     def listMailboxes(self, ref, wildcard):
         "only support one folder"
 
=== modified file 'localmail/inbox.py'
--- localmail/inbox.py	2015-10-14 13:24:14 +0000
+++ localmail/inbox.py	2018-05-01 09:16:46 +0000
@@ -1,3 +1,4 @@
+# -*- coding: utf-8 -*-
 # Copyright (C) 2012- Canonical Ltd
 #
 # This program is free software; you can redistribute it and/or modify
@@ -20,15 +21,18 @@
 import mailbox
 import email.utils
 from itertools import count
-from cStringIO import StringIO
+try:
+    from cStringIO import StringIO
+except ImportError:
+    from io import BytesIO as StringIO
 
-from zope.interface import implements
+from zope.interface import implementer
 
 from twisted.mail import imap4
 from twisted.python import log
 
 UID_GENERATOR = count()
-LAST_UID = UID_GENERATOR.next()
+LAST_UID = next(UID_GENERATOR)
 
 SEEN = r'\Seen'
 UNSEEN = r'\Unseen'
@@ -40,12 +44,12 @@
 
 def get_counter():
     global LAST_UID
-    LAST_UID = UID_GENERATOR.next()
+    LAST_UID = next(UID_GENERATOR)
     return LAST_UID
 
 
+@implementer(imap4.IMailbox)
 class MemoryIMAPMailbox(object):
-    implements(imap4.IMailbox)
 
     mbox = None
 
@@ -117,7 +121,7 @@
 
     def fetch(self, msg_set, uid):
         messages = self._get_msgs(msg_set, uid)
-        return messages.items()
+        return list(messages.items())
 
     def addListener(self, listener):
         self.listeners.append(listener)
@@ -165,8 +169,8 @@
 INBOX = MemoryIMAPMailbox()
 
 
+@implementer(imap4.IMessagePart)
 class MessagePart(object):
-    implements(imap4.IMessagePart)
 
     def __init__(self, msg):
         self.msg = msg
@@ -185,7 +189,16 @@
     def getBodyFile(self):
         if self.msg.is_multipart():
             raise TypeError("Requested body file of a multipart message")
-        return StringIO(self.msg.get_payload())
+        # On Python 3, the payload may be a string created using
+        # surrogate-escape encoding.
+        # We can't get at this through the public API, without also undoing
+        # any Content-Transfer-Encoding, which would be tedious to recreate
+        # so we access the private field. This may cause issues in future.
+        # ¯\_(ツ)_/¯
+        payload = self.msg._payload
+        if not isinstance(payload, bytes):
+            payload = payload.encode('ascii', 'surrogateescape')
+        return StringIO(payload)
 
     def getSize(self):
         return len(self.msg.as_string())
@@ -211,17 +224,24 @@
     def unicode(self, header):
         """Converts a header to unicode"""
         value = self.msg[header]
-        orig, enc = decode_header(value)[0]
-        if enc is None:
-            enc = self.parse_charset()
-        return orig.decode(enc)
-
-
+        parts = decode_header(value)
+        return ''.join(
+            decoded_part.decode(codec)
+            if codec is not None else decoded_part.decode('ascii')
+            for decoded_part, codec in parts)
+
+
+@implementer(imap4.IMessage)
 class Message(MessagePart):
-    implements(imap4.IMessage)
 
     def __init__(self, fp, flags, date):
-        super(Message, self).__init__(email.message_from_file(fp))
+        # email.message_from_binary_file is new in Python 3.3,
+        # and we need to use it if we are on Python3.
+        if hasattr(email, 'message_from_binary_file'):
+            parsed_message = email.message_from_binary_file(fp)
+        else:
+            parsed_message = email.message_from_file(fp)
+        super(Message, self).__init__(parsed_message)
         self.data = str(self.msg)
         self.uid = get_counter()
         self.flags = set(flags)
 
=== modified file 'localmail/smtp.py'
--- localmail/smtp.py	2014-09-09 16:41:59 +0000
+++ localmail/smtp.py	2018-05-01 09:16:46 +0000
@@ -14,25 +14,31 @@
 # along with this program; if not, write to the Free Software
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
 
-from cStringIO import StringIO
-
+try:
+    from cStringIO import StringIO
+except ImportError:
+    from io import BytesIO as StringIO
 from twisted.internet import defer
 from twisted.mail import smtp
 from twisted.mail.imap4 import LOGINCredentials, PLAINCredentials
-from zope.interface import implements
-
-from inbox import INBOX
-
-
+from zope.interface import implementer
+
+from .inbox import INBOX
+
+
+@implementer(smtp.IMessage)
 class MemoryMessage(object):
     """Reads a message into a StringIO, and passes on to global inbox"""
-    implements(smtp.IMessage)
 
     def __init__(self):
         self.file = StringIO()
 
     def lineReceived(self, line):
-        self.file.write(line + '\n')
+        # Sometimes line is bytes, and sometimes it is a str.
+        # Let's assume that if it's a string, it's ASCII compatible.
+        if not isinstance(line, bytes):
+            line = line.encode('ascii')
+        self.file.write(line + b'\n')
 
     def eomReceived(self):
         self.file.seek(0)
@@ -44,9 +50,9 @@
         self.file.close()
 
 
+@implementer(smtp.IMessageDelivery)
 class MemoryDelivery(object):
     """Null-validator for email address - always delivers succesfully"""
-    implements(smtp.IMessageDelivery)
 
     def validateTo(self, user):
         return MemoryMessage
@@ -62,8 +68,8 @@
     """Factort for SMTP connections that authenticates any user"""
     protocol = smtp.ESMTP
     challengers = {
-        "LOGIN": LOGINCredentials,
-        "PLAIN": PLAINCredentials
+        b"LOGIN": LOGINCredentials,
+        b"PLAIN": PLAINCredentials
     }
     noisy = False
 
 
=== modified file 'tests/helpers.py'
--- tests/helpers.py	2014-09-09 20:50:23 +0000
+++ tests/helpers.py	2018-05-01 09:16:46 +0000
@@ -1,6 +1,9 @@
 import smtplib
 import imaplib
-from email import message_from_string
+try:
+    from email import message_from_bytes as message_from_string
+except ImportError:
+    from email import message_from_string
 
 
 class ContextHelper(object):
@@ -34,7 +37,7 @@
 
     def start(self):
         self.client = smtplib.SMTP(self.host, self.port)
-        #self.client.set_debuglevel(1)
+        # self.client.set_debuglevel(1)
         self.client.login(self.user, self.password)
         return self
 
@@ -99,5 +102,5 @@
                 return None
             msg_set = msgs[seq - 1]
         else:
-            msg_set = str(seq)
+            msg_set = str(seq).encode('ascii')
         return msg_set
 
=== modified file 'tests/spam/check.py'
--- tests/spam/check.py	2014-09-09 11:49:58 +0000
+++ tests/spam/check.py	2018-05-01 09:16:46 +0000
@@ -1,19 +1,20 @@
+from __future__ import print_function
 from email.parser import Parser
 import glob
 
 for file in glob.glob('*.txt'):
-    print file
+    print(file)
     msg = Parser().parse(open(file, 'rb'))
-    print "Type: ", msg.get_content_type()
+    print("Type: ", msg.get_content_type())
     for k, v in msg.items():
         print("%s: %s" % (k, v))
     for part in msg.walk():
         if part.get_content_maintype() == 'multipart':
             continue
-        print "PART:"
-        print part.get_content_type()
-        print part.get_payload()[:150]
-        print
-    print
-    print "-------------------------------------------------"
-    print
+        print("PART:")
+        print(part.get_content_type())
+        print(part.get_payload()[:150])
+        print()
+    print()
+    print("-------------------------------------------------")
+    print()
 
=== modified file 'tests/test_localmail.py'
--- tests/test_localmail.py	2014-09-09 16:41:59 +0000
+++ tests/test_localmail.py	2018-05-01 09:16:46 +0000
@@ -1,11 +1,20 @@
+# coding: utf-8
+
 import os
 import time
 import threading
 import imaplib
 import smtplib
+from io import BytesIO
+from email.charset import Charset, BASE64, QP
+from email.message import Message
 from email.mime.text import MIMEText
 from email.mime.multipart import MIMEMultipart
-
+from email.header import decode_header
+try:
+    from email.generator import BytesGenerator as Generator
+except ImportError:
+    from email.generator import Generator
 try:
     import unittest2 as unittest
 except ImportError:
@@ -13,7 +22,7 @@
 
 import localmail
 
-from helpers import (
+from .helpers import (
     SMTPClient,
     IMAPClient,
     clean_inbox,
@@ -150,21 +159,21 @@
 
     def test_delete_single_message(self):
         self.smtp.send(self._testmsg(1))
-        self.imap.store(1, '(\Deleted)')
+        self.imap.store(1, r'(\Deleted)')
         self.imap.client.expunge()
         self.assertEqual(self.imap.search('ALL'), [])
 
     def test_delete_with_multiple(self):
         self.smtp.send(self._testmsg(1))
         self.smtp.send(self._testmsg(2))
-        self.imap.store(1, '(\Deleted)')
+        self.imap.store(1, r'(\Deleted)')
         self.imap.client.expunge()
         self.assertEqual(self.imap.search('ALL'), [self.imap.msgid(1)])
 
     def test_search_deleted(self):
         self.smtp.send(self._testmsg(1))
         self.smtp.send(self._testmsg(2))
-        self.imap.store(1, '(\Deleted)')
+        self.imap.store(1, r'(\Deleted)')
         self.assertEqual(
             self.imap.search('(DELETED)'),
             [self.imap.msgid(1)]
@@ -191,3 +200,118 @@
         msg.attach(html)
         msg.attach(text)
         return msg
+
+
+class EncodingTestCase(BaseLocalmailTestcase):
+
+    # These characters are one byte in latin-1 but two in utf-8
+    difficult_chars = u"£ë"
+    # These characters are two bytes in either encoding
+    difficult_chars += u"筷子"
+    # Unicode snowman for good measure
+    difficult_chars += u"☃"
+    # These characters might trip up Base64 encoding...
+    difficult_chars += u"=+/"
+    # ... and these characters might trip up quoted printable
+    difficult_chars += u"=3D"  # QP encoded
+
+    difficult_chars_latin1_compatible = difficult_chars\
+        .encode('latin-1', 'ignore')\
+        .decode('latin-1')
+
+    uid = False
+
+    def setUp(self):
+        super(EncodingTestCase, self).setUp()
+        self.smtp = SMTPClient(HOST, SMTP_PORT)
+        self.smtp.start()
+        self.imap = IMAPClient(HOST, IMAP_PORT, uid=self.uid)
+        self.imap.start()
+        msgs = self.imap.search('ALL')
+        self.assertEqual(msgs, [])
+        self.addCleanup(self.smtp.stop)
+        self.addCleanup(self.imap.stop)
+
+    def _encode_message(self, msg):
+        with BytesIO() as fp:
+            generator = Generator(fp)
+            generator.flatten(msg)
+            return fp.getvalue()
+
+    def _make_message(self, text, charset, cte):
+        msg = Message()
+        ctes = {'8bit': None, 'base64': BASE64, 'quoted-printable': QP}
+        cs = Charset(charset)
+        cs.body_encoding = ctes[cte]
+        msg.set_payload(text, charset=cs)
+
+        # Should always be encoded correctly.
+        msg['Subject'] = self.difficult_chars
+        msg['From'] = '[e-mail]'
+        msg['To'] = '[e-mail]'
+        self.assertEqual(msg['Content-Transfer-Encoding'], cte)
+        return msg
+
+    def _fetch_and_delete_sole_message(self):
+        message_number = None
+        for _ in range(5):
+            try:
+                message_number, = self.imap.search('ALL')
+                break
+            except ValueError:
+                time.sleep(0.5)
+        else:
+            raise AssertionError("Single Message not found")
+        msg = self.imap.fetch(message_number)
+        self.imap.store(message_number, r'(\Deleted)')
+        self.imap.client.expunge()
+        return msg
+
+    def _do_test(self, payload, charset, cte):
+        # Arrange
+        msg = self._make_message(payload, charset, cte)
+        encoded = self._encode_message(msg)
+
+        # Act
+        self.smtp.client.sendmail(msg['From'], msg['To'], encoded)
+        received = self._fetch_and_delete_sole_message()
+
+        # Assert
+        payload_bytes = received.get_payload(decode=True)
+        payload_text = payload_bytes.decode(received.get_content_charset())
+        self.assertEqual(received['Content-Transfer-Encoding'], cte)
+        self.assertEqual(received.get_content_charset(), charset.lower())
+        (subject_bytes, subject_encoding), = decode_header(received['Subject'])
+        self.assertEqual(
+            subject_bytes.decode(subject_encoding),
+            self.difficult_chars)
+        self.assertEqual(payload_text.strip(), payload)
+
+    def test_roundtrip_latin_1_mail(self):
+        """
+        Mail with only latin-1 chars can be sent in latin-1
+
+        (8-bit MIME)
+        """
+        self._do_test(self.difficult_chars_latin1_compatible,
+                      'iso-8859-1', '8bit')
+
+    def test_roundtrip_utf8_mail(self):
+        """
+        Mail can be sent in utf-8 without encoding
+
+        (8-bit MIME)
+        """
+        self._do_test(self.difficult_chars, 'utf-8', '8bit')
+
+    def test_roundtrip_utf8_qp_mail(self):
+        """
+        Mail can be sent in utf-8 in quoted printable format
+        """
+        self._do_test(self.difficult_chars, 'utf-8', 'quoted-printable')
+
+    def test_roundtrip_utf8_base64_mail(self):
+        """
+        Mail can be sent in utf-8 in quoted printable format
+        """
+        self._do_test(self.difficult_chars, 'utf-8', 'base64')
 
=== modified file 'tox.ini'
--- tox.ini	2013-05-24 17:03:36 +0000
+++ tox.ini	2018-05-01 09:16:46 +0000
@@ -1,4 +1,5 @@
 [tox]
-envlist = py26,py27,pypy
+envlist = py26,py27,pypy,py35
 [testenv]
 commands = python setup.py test
+passenv = LOCALMAIL
 
=== modified file 'twisted/plugins/localmail_tap.py'
--- twisted/plugins/localmail_tap.py	2015-10-14 09:00:55 +0000
+++ twisted/plugins/localmail_tap.py	2018-05-01 09:16:46 +0000
@@ -13,7 +13,7 @@
 # You should have received a copy of the GNU General Public License
 # along with this program; if not, write to the Free Software
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
-from zope.interface import implements
+from zope.interface import implementer
 
 from twisted.application import service
 from twisted import plugin
@@ -34,8 +34,8 @@
     ]
 
 
+@implementer(service.IServiceMaker, plugin.IPlugin)
 class LocalmailServiceMaker(object):
-    implements(service.IServiceMaker, plugin.IPlugin)
     tapname = "localmail"
     description = "A test SMTP/IMAP server"
     options = Options