Repository: aio-libs/aiosmtpd
Pull request #4 (2016-08-03 16:12:15)

SMTPUTF-8 Support & Handle exceptions
While SMTPUTF-8 enabled or not, you can receive 8bit encoded messages (not utf-8) because remote client is not fully RFC compatible or supports not UTF, but only 8bit encoding. Also, it wouldnt get encoding fail on cases, when command, not expecting paramets will get bad encoded argument.

Also I added finally/except clauses to connection handle coroutine. If exception is raised it hangs up and connection is keeped alive. Remote client will wait answer until timeout is reached and will not receive any answer. So we need not answer with exception code and close connection when handling is finished.

Good idea will be to extract params before decoding sender/recipient and keep it as byte string if decoding fails. This will make next smtp relay or handler deside what to do with it - may be it is not fatal error and may be ignored (for example we save mails on disk or send to /dev/null) or next relay server will deal with it (for example Postfix do so).

And some other fixes and tests



Patch:
diff --git a/aiosmtpd/smtp.py b/aiosmtpd/smtp.py
index 51c4e69..cc3118d 100644
--- a/aiosmtpd/smtp.py
+++ b/aiosmtpd/smtp.py
@@ -44,6 +44,7 @@ class Envelope:
     def __init__(self):
         self.mail_from = None
         self.mail_options = []
+        self.smtp_utf8 = False
         self.content = None
         self.rcpt_tos = []
         self.rcpt_options = []
@@ -80,12 +81,6 @@ class SMTP(asyncio.StreamReaderProtocol):
         self.event_handler = handler
         self.data_size_limit = data_size_limit
         self.enable_SMTPUTF8 = enable_SMTPUTF8
-        if enable_SMTPUTF8:
-            if decode_data:
-                raise ValueError(
-                    "decode_data and enable_SMTPUTF8 cannot be set to "
-                    "True at the same time")
-            decode_data = False
         self._decode_data = decode_data
         self.command_size_limits.clear()
         if hostname:
@@ -177,7 +172,6 @@ class SMTP(asyncio.StreamReaderProtocol):
     def _set_post_data_state(self):
         """Reset state variables to their post-DATA state."""
         self.envelope = self._create_envelope()
-        self.require_SMTPUTF8 = False
 
     def _set_rset_state(self):
         """Reset all state variables except the greeting."""
@@ -186,7 +180,7 @@ class SMTP(asyncio.StreamReaderProtocol):
     @asyncio.coroutine
     def push(self, status):
         response = bytes(
-            status + '\r\n', 'utf-8' if self.require_SMTPUTF8 else 'ascii')
+            status + '\r\n', 'utf-8' if self.enable_SMTPUTF8 else 'ascii')
         self._writer.write(response)
         log.debug(response)
         yield from self._writer.drain()
@@ -213,18 +207,33 @@ class SMTP(asyncio.StreamReaderProtocol):
                 return
             try:
                 # XXX this rstrip may not completely preserve old behavior.
-                line = line.decode('utf-8').rstrip('\r\n')
+                line = line.rstrip(b'\r\n')
                 log.info('%r Data: %s', self.session.peer, line)
                 if not line:
                     yield from self.push('500 Error: bad syntax')
                     continue
-                i = line.find(' ')
+                i = line.find(b' ')
+                # Decode to string only the command name part, which must be
+                # ASCII as per RFC.  If there is an argument, it is decoded to
+                # UTF-8/surrogateescape so that non-UTF-8 data can be
+                # re-encoded back to the original bytes when the SMTP command
+                # is handled.
                 if i < 0:
-                    command = line.upper()
+                    command = line.upper().decode(encoding='ascii')
                     arg = None
                 else:
-                    command = line[:i].upper()
+                    command = line[:i].upper().decode(encoding='ascii')
                     arg = line[i+1:].strip()
+                    # Remote SMTP servers can send us UTF-8 content despite
+                    # whether they've declared to do so or not.  Some old
+                    # servers can send 8-bit data.  Use surrogateescape so
+                    # that the fidelity of the decoding is preserved, and the
+                    # original bytes can be retrieved.
+                    if self.enable_SMTPUTF8:
+                        arg = str(
+                            arg, encoding='utf-8', errors='surrogateescape')
+                    else:
+                        arg = str(arg, encoding='ascii', errors='strict')
                 max_sz = (self.command_size_limits[command]
                           if self.session.extended_smtp
                           else self.command_size_limit)
@@ -468,13 +477,14 @@ class SMTP(asyncio.StreamReaderProtocol):
                 yield from self.push(
                     '501 Error: BODY can only be one of 7BIT, 8BITMIME')
                 return
-        if self.enable_SMTPUTF8:
-            smtputf8 = params.pop('SMTPUTF8', False)
-            if smtputf8 is True:
-                self.require_SMTPUTF8 = True
-            elif smtputf8 is not False:
-                yield from self.push('501 Error: SMTPUTF8 takes no arguments')
-                return
+        smtputf8 = params.pop('SMTPUTF8', False)
+        if not isinstance(smtputf8, bool):
+            yield from self.push('501 Error: SMTPUTF8 takes no arguments')
+            return
+        if smtputf8 and not self.enable_SMTPUTF8:
+            yield from self.push('501 Error: SMTPUTF8 disabled')
+            return
+        self.envelope.smtp_utf8 = smtputf8
         size = params.pop('SIZE', None)
         if size:
             if isinstance(size, bool) or not size.isdigit():
@@ -600,7 +610,11 @@ class SMTP(asyncio.StreamReaderProtocol):
                 data[i] = text[1:]
         content = original_content = EMPTYBYTES.join(data)
         if self._decode_data:
-            content = original_content.decode('utf-8')
+            if self.enable_SMTPUTF8:
+                content = original_content.decode(
+                    'utf-8', errors='surrogateescape')
+            else:
+                content = original_content.decode('ascii', errors='strict')
         self.envelope.content = content
         self.envelope.original_content = original_content
         # Call the new API first if it's implemented.


Test patch:
diff --git a/aiosmtpd/tests/test_server.py b/aiosmtpd/tests/test_server.py
index 2f3741d..c24467e 100644
--- a/aiosmtpd/tests/test_server.py
+++ b/aiosmtpd/tests/test_server.py
@@ -11,12 +11,6 @@ from smtplib import SMTP
 
 
 class TestServer(unittest.TestCase):
-    def test_constructor_contraints(self):
-        # These two arguments cannot both be set.
-        self.assertRaises(ValueError, Server, Sink(),
-                          enable_SMTPUTF8=True,
-                          decode_data=True)
-
     def test_smtp_utf8(self):
         controller = Controller(Sink())
         controller.start()
diff --git a/aiosmtpd/tests/test_smtp.py b/aiosmtpd/tests/test_smtp.py
index d37d0d8..ec2b66c 100644
--- a/aiosmtpd/tests/test_smtp.py
+++ b/aiosmtpd/tests/test_smtp.py
@@ -19,7 +19,7 @@ BCRLF = b'\r\n'
 
 class DecodingController(Controller):
     def factory(self):
-        return Server(self.handler, decode_data=True)
+        return Server(self.handler, decode_data=True, enable_SMTPUTF8=True)
 
 
 class NoDecodeController(Controller):
@@ -35,7 +35,7 @@ class ReceivingHandler:
 
     @asyncio.coroutine
     def handle_DATA(self, server, session, envelope):
-        self.box.append(envelope.content)
+        self.box.append(envelope)
         return '250 OK'
 
 
@@ -48,6 +48,11 @@ class SizedController(Controller):
         return Server(self.handler, data_size_limit=self.size)
 
 
+class StrictASCIIController(Controller):
+    def factory(self):
+        return Server(self.handler, enable_SMTPUTF8=False, decode_data=True)
+
+
 class CustomHostnameController(Controller):
     def factory(self):
         return Server(self.handler, hostname='custom.localhost')
@@ -121,7 +126,7 @@ class TestProtocol(unittest.TestCase):
         except asyncio.CancelledError:
             pass
         self.assertEqual(len(handler.box), 1)
-        self.assertEqual(handler.box[0], data)
+        self.assertEqual(handler.box[0].content, data)
 
     def test_empty_email(self):
         handler = ReceivingHandler()
@@ -140,7 +145,7 @@ class TestProtocol(unittest.TestCase):
             pass
         self.assertEqual(self.responses[5], b'250 OK\r\n')
         self.assertEqual(len(handler.box), 1)
-        self.assertEqual(handler.box[0], b'')
+        self.assertEqual(handler.box[0].content, b'')
 
 
 class TestSMTP(unittest.TestCase):
@@ -179,7 +184,8 @@ class TestSMTP(unittest.TestCase):
             lines = response.splitlines()
             self.assertEqual(lines[0], bytes(socket.getfqdn(), 'utf-8'))
             self.assertEqual(lines[1], b'SIZE 33554432')
-            self.assertEqual(lines[2], b'HELP')
+            self.assertEqual(lines[2], b'SMTPUTF8')
+            self.assertEqual(lines[3], b'HELP')
 
     def test_ehlo_duplicate(self):
         with SMTP(*self.address) as client:
@@ -611,15 +617,31 @@ class TestSMTPWithController(unittest.TestCase):
                 b'Error: message size exceeds fixed maximum message size')
 
     def test_mail_with_compatible_smtputf8(self):
-        controller = Controller(Sink())
+        handler = ReceivingHandler()
+        controller = Controller(handler)
         controller.start()
         self.addCleanup(controller.stop)
+        recipient = 'bart\xCB@example.com'
+        sender = 'anne\xCB@example.com'
         with SMTP(controller.hostname, controller.port) as client:
             client.ehlo('example.com')
-            code, response = client.docmd(
-                'MAIL FROM: <anne@example.com> SMTPUTF8')
+            client.send(bytes(
+                'MAIL FROM: <' + sender + '> SMTPUTF8\r\n',
+                encoding='utf-8'))
+            code, response = client.getreply()
             self.assertEqual(code, 250)
             self.assertEqual(response, b'OK')
+            client.send(bytes(
+                'RCPT TO: <' + recipient + '>\r\n',
+                encoding='utf-8'))
+            code, response = client.getreply()
+            self.assertEqual(code, 250)
+            self.assertEqual(response, b'OK')
+            code, response = client.data('')
+            self.assertEqual(code, 250)
+            self.assertEqual(response, b'OK')
+        self.assertEqual(handler.box[0].rcpt_tos[0], recipient)
+        self.assertEqual(handler.box[0].mail_from, sender)
 
     def test_mail_with_unrequited_smtputf8(self):
         controller = Controller(Sink())
@@ -706,7 +728,7 @@ Testing
             mail = CRLF.join(['Test', '.', 'mail'])
             client.sendmail('anne@example.com', ['bart@example.com'], mail)
             self.assertEqual(len(handler.box), 1)
-            self.assertEqual(handler.box[0], 'Test\r\n.\r\nmail')
+            self.assertEqual(handler.box[0].content, 'Test\r\n.\r\nmail')
 
     def test_unexpected_errors(self):
         handler = ErroringHandler()
@@ -743,6 +765,32 @@ Testing
         # handle_exception() method.
         self.assertIsNone(handler.error)
 
+    def test_bad_encodings(self):
+        handler = ReceivingHandler()
+        controller = DecodingController(handler)
+        controller.start()
+        self.addCleanup(controller.stop)
+        with SMTP(controller.hostname, controller.port) as client:
+            client.helo('example.com')
+            mail_from = b'anne\xFF@example.com'
+            mail_to = b'bart\xFF@example.com'
+            client.ehlo('test')
+            client.send(b'MAIL FROM:' + mail_from + b'\r\n')
+            code, response = client.getreply()
+            self.assertEqual(code, 250)
+            client.send(b'RCPT TO:' + mail_to + b'\r\n')
+            code, response = client.getreply()
+            self.assertEqual(code, 250)
+            client.data('Test mail')
+            self.assertEqual(len(handler.box), 1)
+            envelope = handler.box[0]
+            mail_from2 = envelope.mail_from.encode(
+                'utf-8', errors='surrogateescape')
+            self.assertEqual(mail_from2, mail_from)
+            mail_to2 = envelope.rcpt_tos[0].encode(
+                'utf-8', errors='surrogateescape')
+            self.assertEqual(mail_to2, mail_to)
+
 
 class TestCustomizations(unittest.TestCase):
     def test_custom_hostname(self):
@@ -844,3 +892,50 @@ class TestClientCrash(unittest.TestCase):
             # Don't include the CRLF.
             client.send('FOO')
             client.close()
+
+
+class TestStrictASCII(unittest.TestCase):
+    def setUp(self):
+        controller = StrictASCIIController(Sink())
+        controller.start()
+        self.addCleanup(controller.stop)
+        self.address = (controller.hostname, controller.port)
+
+    def test_ehlo(self):
+        with SMTP(*self.address) as client:
+            code, response = client.ehlo('example.com')
+            self.assertEqual(code, 250)
+            lines = response.splitlines()
+            self.assertNotIn(b'SMTPUTF8', lines)
+
+    def test_bad_encoded_param(self):
+        with SMTP(*self.address) as client:
+            client.ehlo('example.com')
+            client.send(b'MAIL FROM: <anne\xFF@example.com>\r\n')
+            code, response = client.getreply()
+            self.assertEqual(code, 500)
+            self.assertIn(b'Error: (UnicodeDecodeError)', response)
+
+    def test_mail_param(self):
+        with SMTP(*self.address) as client:
+            client.ehlo('example.com')
+            code, response = client.docmd(
+                'MAIL FROM: <anne@example.com> SMTPUTF8')
+            self.assertEqual(code, 501)
+            self.assertEqual(response, b'Error: SMTPUTF8 disabled')
+
+    def test_data(self):
+        with SMTP(*self.address) as client:
+            code, response = client.ehlo('example.com')
+            self.assertEqual(code, 250)
+            with self.assertRaises(SMTPDataError) as cm:
+                client.sendmail('anne@example.com', ['bart@example.com'], b"""\
+From: anne@example.com
+To: bart@example.com
+Subject: A test
+
+Testing\xFF
+""")
+            self.assertEqual(cm.exception.smtp_code, 500)
+            self.assertIn(b'Error: (UnicodeDecodeError)',
+                          cm.exception.smtp_error)