Repository: aio-libs/aiosmtpd
Pull request #33 (2017-01-27 00:52:51)

Enforce a stricter coding style
flake8 `hang-closing = True` to be exact.


Patch:
diff --git a/aiosmtpd/docs/NEWS.rst b/aiosmtpd/docs/NEWS.rst
index 6d51586..3783097 100644
--- a/aiosmtpd/docs/NEWS.rst
+++ b/aiosmtpd/docs/NEWS.rst
@@ -8,6 +8,8 @@
 * Minor changes to the way the ``Debugging`` handler prints ``mail_options``
   and ``rcpt_options`` (although the latter is still not support in ``SMTP``).
 * Use ``flufl.testing`` package for nose2 and flake8 plugins.
+* ``DATA`` method now respects original line endings, and passing size limits
+  is now handled better.  Given by Konstantin Volkov.
 * Better Python 3.4 compatibility.
 
 1.0a4 (2016-11-29)
diff --git a/aiosmtpd/handlers.py b/aiosmtpd/handlers.py
index acb123a..2c405e7 100644
--- a/aiosmtpd/handlers.py
+++ b/aiosmtpd/handlers.py
@@ -13,11 +13,13 @@ import mailbox
 import smtplib
 
 from email import message_from_bytes, message_from_string
+from email.feedparser import NLCRE
 from public import public
 
 
+EMPTYSTRING = ''
 COMMASPACE = ', '
-NEWLINE = '\n'
+CRLF = '\r\n'
 log = logging.getLogger('mail.debug')
 
 
@@ -89,15 +91,17 @@ class Proxy:
         self._port = remote_port
 
     def process_message(self, peer, mailfrom, rcpttos, data, **kws):
-        lines = data.split('\n')
+        lines = data.splitlines(keepends=True)
         # Look for the last header
         i = 0
+        ending = CRLF
         for line in lines:                          # pragma: nobranch
-            if not line:
+            if NLCRE.match(line):
+                ending = line
                 break
             i += 1
-        lines.insert(i, 'X-Peer: %s' % peer[0])
-        data = NEWLINE.join(lines)
+        lines.insert(i, 'X-Peer: %s%s' % (peer[0], ending))
+        data = EMPTYSTRING.join(lines)
         refused = self._deliver(mailfrom, rcpttos, data)
         # TBD: what to do with refused addresses?
         log.info('we got some refusals: %s', refused)
diff --git a/aiosmtpd/main.py b/aiosmtpd/main.py
index bb1c432..e623bb2 100644
--- a/aiosmtpd/main.py
+++ b/aiosmtpd/main.py
@@ -152,7 +152,7 @@ def setup_sock(host, port):
             socket.SOCK_STREAM,
             0,
             socket.AI_PASSIVE,
-        )
+            )
     except socket.gaierror:
         # Infer the type from the host.
         addr = host, port
diff --git a/aiosmtpd/smtp.py b/aiosmtpd/smtp.py
index f722570..56443b8 100644
--- a/aiosmtpd/smtp.py
+++ b/aiosmtpd/smtp.py
@@ -23,6 +23,7 @@ log = logging.getLogger('mail.log')
 
 NEWLINE = '\n'
 DATA_SIZE_DEFAULT = 33554432
+EMPTYBYTES = b''
 
 
 @public
@@ -62,7 +63,7 @@ class SMTP(asyncio.StreamReaderProtocol):
             self._dotsep = '.'
             self._newline = NEWLINE
         else:
-            self._emptystring = b''
+            self._emptystring = EMPTYBYTES
             self._linesep = b'\r\n'
             self._dotsep = ord(b'.')
             self._newline = b'\n'
@@ -144,8 +145,6 @@ class SMTP(asyncio.StreamReaderProtocol):
     def _set_rset_state(self):
         """Reset all state variables except the greeting."""
         self._set_post_data_state()
-        self.received_data = ''
-        self.received_lines = []
 
     @asyncio.coroutine
     def push(self, msg):
@@ -505,35 +504,41 @@ class SMTP(asyncio.StreamReaderProtocol):
             return
         yield from self.push('354 End data with <CR><LF>.<CR><LF>')
         data = []
-        self.num_bytes = 0
+        num_bytes = 0
+        size_exceeded = False
         while not self.connection_closed:
             line = yield from self._reader.readline()
             if line == b'.\r\n':
+                if data:
+                    data[-1] = data[-1].rstrip(b'\r\n')
                 break
-            self.num_bytes += len(line)
-            if self.data_size_limit and self.num_bytes > self.data_size_limit:
+            num_bytes += len(line)
+            if (not size_exceeded and
+                    self.data_size_limit and
+                    num_bytes > self.data_size_limit):
+                size_exceeded = True
                 yield from self.push('552 Error: Too much mail data')
-            # XXX this rstrip may not exactly preserve the old behavior
-            line = line.rstrip(b'\r\n')
-            if self._decode_data:
-                data.append(line.decode('utf-8'))
-            else:
+            if not size_exceeded:
                 data.append(line)
+        if size_exceeded:
+            self._set_post_data_state()
+            return
         # Remove extraneous carriage returns and de-transparency
         # according to RFC 5321, Section 4.5.2.
         for i in range(len(data)):
             text = data[i]
-            if text and text[0] == self._dotsep:
+            if text and text[:1] == b'.':
                 data[i] = text[1:]
-        self.received_data = self._newline.join(data)
-        args = (self.peer, self.mailfrom, self.rcpttos,
-                self.received_data)
+        received_data = EMPTYBYTES.join(data)
+        if self._decode_data:
+            received_data = received_data.decode('utf-8')
+        args = (self.peer, self.mailfrom, self.rcpttos, received_data)
         kwargs = {}
         if not self._decode_data:
             kwargs = {
                 'mail_options': self.mail_options,
                 'rcpt_options': self.rcpt_options,
-            }
+                }
         kwargs.update({'loop': self.loop})
         if asyncio.iscoroutinefunction(self.event_handler.process_message):
             status = yield from self.event_handler.process_message(
diff --git a/tox.ini b/tox.ini
index 55fd34b..60d1dac 100644
--- a/tox.ini
+++ b/tox.ini
@@ -46,7 +46,8 @@ deps:
     sphinx
 
 [flake8]
-max-line-length = 79
-jobs = 1
-exclude = conf.py
 enable-extensions = U4
+exclude = conf.py
+hang-closing = True
+jobs = 1
+max-line-length = 79


Test patch:
diff --git a/aiosmtpd/tests/test_handlers.py b/aiosmtpd/tests/test_handlers.py
index ede90c9..ac517e0 100644
--- a/aiosmtpd/tests/test_handlers.py
+++ b/aiosmtpd/tests/test_handlers.py
@@ -15,6 +15,8 @@ from smtplib import SMTP, SMTPRecipientsRefused
 from tempfile import TemporaryDirectory
 from unittest.mock import call, patch
 
+CRLF = '\r\n'
+
 
 class UTF8Controller(Controller):
     def factory(self):
@@ -407,14 +409,16 @@ Testing
                 'anne@example.com', ['bart@example.com'], self.message)
             client.quit()
             mock().connect.assert_called_once_with('localhost', 9025)
+            # SMTP always fixes eols, so it must be always CRLF as delimiter
+            msg = CRLF.join([
+                'From: Anne Person <anne@example.com>',
+                'To: Bart Person <bart@example.com>',
+                'Subject: A test',
+                'X-Peer: ::1',
+                '',
+                'Testing'])
             mock().sendmail.assert_called_once_with(
-                'anne@example.com', ['bart@example.com'], """\
-From: Anne Person <anne@example.com>
-To: Bart Person <bart@example.com>
-Subject: A test
-X-Peer: ::1
-
-Testing""")
+                'anne@example.com', ['bart@example.com'], msg)
             mock().quit.assert_called_once_with()
 
     def test_recipients_refused(self):
diff --git a/aiosmtpd/tests/test_smtp.py b/aiosmtpd/tests/test_smtp.py
index 753cc4e..635176c 100644
--- a/aiosmtpd/tests/test_smtp.py
+++ b/aiosmtpd/tests/test_smtp.py
@@ -1,12 +1,17 @@
 """Test the SMTP protocol."""
 
 import socket
+import asyncio
 import unittest
 
 from aiosmtpd.controller import Controller
 from aiosmtpd.handlers import Sink
 from aiosmtpd.smtp import SMTP as Server, __ident__ as GREETING
-from smtplib import SMTP, SMTPDataError
+from smtplib import SMTP, SMTPDataError, SMTPResponseException
+from unittest.mock import Mock
+
+CRLF = '\r\n'
+BCRLF = b'\r\n'
 
 
 class UTF8Controller(Controller):
@@ -19,6 +24,16 @@ class NoDecodeController(Controller):
         return Server(self.handler, decode_data=False)
 
 
+class ReceivingHandler:
+    box = None
+
+    def __init__(self):
+        self.box = []
+
+    def process_message(self, *args, **kws):
+        self.box.append(args)
+
+
 class SizedController(Controller):
     def __init__(self, handler, size, loop=None, hostname='::0', port=8025):
         self.size = size
@@ -50,6 +65,42 @@ class ErroringHandler:
         return '499 Could not accept the message'
 
 
+class TestProtocol(unittest.TestCase):
+    def setUp(self):
+        self.transport = Mock()
+        self._old_loop = asyncio.get_event_loop()
+        self.loop = asyncio.new_event_loop()
+        asyncio.set_event_loop(self.loop)
+
+    def tearDown(self):
+        self.loop.close()
+        asyncio.set_event_loop(self._old_loop)
+
+    def _get_protocol(self, *args, **kwargs):
+        protocol = Server(*args, loop=self.loop, **kwargs)
+        protocol.connection_made(self.transport)
+        return protocol
+
+    def test_honors_mail_delimeters(self):
+        handler = ReceivingHandler()
+        data = b'test\r\nmail\rdelimeters\nsaved'
+        protocol = self._get_protocol(handler)
+        protocol.data_received(BCRLF.join([
+            b'HELO example.org',
+            b'MAIL FROM: <anne@example.com>',
+            b'RCPT TO: <anne@example.com>',
+            b'DATA',
+            data + b'\r\n.',
+            b'QUIT\r\n'
+        ]))
+        try:
+            self.loop.run_until_complete(protocol._handler_coroutine)
+        except asyncio.CancelledError:
+            pass
+        assert len(handler.box) == 1
+        assert handler.box[0][3] == data
+
+
 class TestSMTP(unittest.TestCase):
     def setUp(self):
         controller = UTF8Controller(Sink)
@@ -590,6 +641,32 @@ Testing
                 self.assertEqual(cm.exception.response,
                                  b'Could not accept the message')
 
+    def test_too_long_message_body(self):
+        controller = SizedController(Sink(), size=100)
+        controller.start()
+        self.addCleanup(controller.stop)
+        with SMTP(controller.hostname, controller.port) as client:
+            client.helo('example.com')
+            mail = '\r\n'.join(['z' * 20] * 10)
+            with self.assertRaises(SMTPResponseException) as cm:
+                client.sendmail('anne@example.com', ['bart@example.com'], mail)
+            self.assertEqual(cm.exception.smtp_code, 552)
+            self.assertEqual(cm.exception.smtp_error,
+                             b'Error: Too much mail data')
+
+    def test_dots_escaped(self):
+        handler = ReceivingHandler()
+        controller = UTF8Controller(handler)
+        controller.start()
+        self.addCleanup(controller.stop)
+        with SMTP(controller.hostname, controller.port) as client:
+            client.helo('example.com')
+            mail = CRLF.join(['Test', '.', 'mail'])
+            client.sendmail('anne@example.com', ['bart@example.com'], mail)
+            self.assertEqual(len(handler.box), 1)
+            mail = handler.box[0]
+            self.assertEqual(mail[3], 'Test\r\n.\r\nmail')
+
 
 class TestCustomizations(unittest.TestCase):
     def test_custom_hostname(self):