Repository: aio-libs/aiosmtpd
Pull request #109 (2017-06-04 22:57:22)

Fixes NOOP not accepting argument.
As already said in #107 NOOP *SHOULD* accept arguments as stated in RFC 5321 $4.1.1.9.


Patch:
diff --git a/aiosmtpd/docs/NEWS.rst b/aiosmtpd/docs/NEWS.rst
index 63f686d..5471410 100644
--- a/aiosmtpd/docs/NEWS.rst
+++ b/aiosmtpd/docs/NEWS.rst
@@ -5,8 +5,6 @@
 1.1 (201X-XX-XX)
 ================
 * Drop support for Python 3.4.
-* As per RFC 5321, §4.1.4, multiple ``HELO`` / ``EHLO`` commands in the same
-  session are semantically equivalent to ``RSET``.  (Closes #78)
 * General improvements in the ``Controller`` class.  (Closes #104)
 * When aiosmtpd handles a ``STARTTLS`` it must arrange for the original
   transport to be closed when the wrapped transport is closed.  This fixes a
diff --git a/aiosmtpd/smtp.py b/aiosmtpd/smtp.py
index 287875b..75c2908 100644
--- a/aiosmtpd/smtp.py
+++ b/aiosmtpd/smtp.py
@@ -284,6 +284,10 @@ class SMTP(asyncio.StreamReaderProtocol):
         if not hostname:
             await self.push('501 Syntax: HELO hostname')
             return
+        # See issue #21783 for a discussion of this behavior.
+        if self.session.host_name:
+            await self.push('503 Duplicate HELO/EHLO')
+            return
         self._set_rset_state()
         self.session.extended_smtp = False
         status = await self._call_handler_hook('HELO', hostname)
@@ -296,6 +300,11 @@ class SMTP(asyncio.StreamReaderProtocol):
         if not hostname:
             await self.push('501 Syntax: EHLO hostname')
             return
+        # See https://bugs.python.org/issue21783 for a discussion of this
+        # behavior.
+        if self.session.host_name:
+            await self.push('503 Duplicate HELO/EHLO')
+            return
         self._set_rset_state()
         self.session.extended_smtp = True
         await self.push('250-%s' % self.hostname)
@@ -320,11 +329,8 @@ class SMTP(asyncio.StreamReaderProtocol):
         await self.push(status)
 
     async def smtp_NOOP(self, arg):
-        if arg:
-            await self.push('501 Syntax: NOOP')
-        else:
-            status = await self._call_handler_hook('NOOP')
-            await self.push('250 OK' if status is MISSING else status)
+        status = await self._call_handler_hook('NOOP', arg)
+        await self.push('250 OK' if status is MISSING else status)
 
     async def smtp_QUIT(self, arg):
         if arg:


Test patch:
diff --git a/aiosmtpd/tests/test_smtp.py b/aiosmtpd/tests/test_smtp.py
index 4e4c518..6c190ff 100644
--- a/aiosmtpd/tests/test_smtp.py
+++ b/aiosmtpd/tests/test_smtp.py
@@ -38,15 +38,6 @@ class ReceivingHandler:
         return '250 OK'
 
 
-class StoreEnvelopeOnVRFYHandler:
-    """Saves envelope for later inspection when handling VRFY."""
-    envelope = None
-
-    async def handle_VRFY(self, server, session, envelope, addr):
-        self.envelope = envelope
-        return '250 OK'
-
-
 class SizedController(Controller):
     def __init__(self, handler, size):
         self.size = size
@@ -210,7 +201,8 @@ class TestSMTP(unittest.TestCase):
             code, response = client.helo('example.com')
             self.assertEqual(code, 250)
             code, response = client.helo('example.org')
-            self.assertEqual(code, 250)
+            self.assertEqual(code, 503)
+            self.assertEqual(response, b'Duplicate HELO/EHLO')
 
     def test_ehlo(self):
         with SMTP(*self.address) as client:
@@ -227,7 +219,8 @@ class TestSMTP(unittest.TestCase):
             code, response = client.ehlo('example.com')
             self.assertEqual(code, 250)
             code, response = client.ehlo('example.org')
-            self.assertEqual(code, 250)
+            self.assertEqual(code, 503)
+            self.assertEqual(response, b'Duplicate HELO/EHLO')
 
     def test_ehlo_no_hostname(self):
         with SMTP(*self.address) as client:
@@ -242,14 +235,16 @@ class TestSMTP(unittest.TestCase):
             code, response = client.helo('example.com')
             self.assertEqual(code, 250)
             code, response = client.ehlo('example.org')
-            self.assertEqual(code, 250)
+            self.assertEqual(code, 503)
+            self.assertEqual(response, b'Duplicate HELO/EHLO')
 
     def test_ehlo_then_helo(self):
         with SMTP(*self.address) as client:
             code, response = client.ehlo('example.com')
             self.assertEqual(code, 250)
             code, response = client.helo('example.org')
-            self.assertEqual(code, 250)
+            self.assertEqual(code, 503)
+            self.assertEqual(response, b'Duplicate HELO/EHLO')
 
     def test_noop(self):
         with SMTP(*self.address) as client:
@@ -259,9 +254,8 @@ class TestSMTP(unittest.TestCase):
     def test_noop_with_arg(self):
         with SMTP(*self.address) as client:
             # .noop() doesn't accept arguments.
-            code, response = client.docmd('NOOP', 'oops')
-            self.assertEqual(code, 501)
-            self.assertEqual(response, b'Syntax: NOOP')
+            code, response = client.docmd('NOOP', 'ok')
+            self.assertEqual(code, 250)
 
     def test_quit(self):
         client = SMTP(*self.address)
@@ -670,87 +664,6 @@ class TestSMTP(unittest.TestCase):
                 b'Error: command "FOOBAR" not recognized')
 
 
-class TestResetCommands(unittest.TestCase):
-    """Test that sender and recipients are reset on RSET, HELO, and EHLO.
-
-    The tests below issue each command twice with different addresses and
-    verify that mail_from and rcpt_tos have been replacecd.
-    """
-    expected_envelope_data = [
-        # Pre-RSET/HELO/EHLO envelope data.
-        dict(
-            mail_from='anne@example.com',
-            rcpt_tos=['bart@example.com', 'cate@example.com'],
-            ),
-        dict(
-            mail_from='dave@example.com',
-            rcpt_tos=['elle@example.com', 'fred@example.com'],
-            ),
-        ]
-
-    def setUp(self):
-        self._handler = StoreEnvelopeOnVRFYHandler()
-        self._controller = DecodingController(self._handler)
-        self._controller.start()
-        self._address = (self._controller.hostname, self._controller.port)
-        self.addCleanup(self._controller.stop)
-
-    def _send_envelope_data(self, client, mail_from, rcpt_tos):
-        client.mail(mail_from)
-        for rcpt in rcpt_tos:
-            client.rcpt(rcpt)
-
-    def test_helo(self):
-        with SMTP(*self._address) as client:
-            # Each time through the loop, the HELO will reset the envelope.
-            for data in self.expected_envelope_data:
-                client.helo('example.com')
-                # Save the envelope in the handler.
-                client.vrfy('zuzu@example.com')
-                self.assertIsNone(self._handler.envelope.mail_from)
-                self.assertEqual(len(self._handler.envelope.rcpt_tos), 0)
-                self._send_envelope_data(client, **data)
-                client.vrfy('zuzu@example.com')
-                self.assertEqual(
-                    self._handler.envelope.mail_from, data['mail_from'])
-                self.assertEqual(
-                    self._handler.envelope.rcpt_tos, data['rcpt_tos'])
-
-    def test_ehlo(self):
-        with SMTP(*self._address) as client:
-            # Each time through the loop, the EHLO will reset the envelope.
-            for data in self.expected_envelope_data:
-                client.ehlo('example.com')
-                # Save the envelope in the handler.
-                client.vrfy('zuzu@example.com')
-                self.assertIsNone(self._handler.envelope.mail_from)
-                self.assertEqual(len(self._handler.envelope.rcpt_tos), 0)
-                self._send_envelope_data(client, **data)
-                client.vrfy('zuzu@example.com')
-                self.assertEqual(
-                    self._handler.envelope.mail_from, data['mail_from'])
-                self.assertEqual(
-                    self._handler.envelope.rcpt_tos, data['rcpt_tos'])
-
-    def test_rset(self):
-        with SMTP(*self._address) as client:
-            client.helo('example.com')
-            # Each time through the loop, the RSET will reset the envelope.
-            for data in self.expected_envelope_data:
-                self._send_envelope_data(client, **data)
-                # Save the envelope in the handler.
-                client.vrfy('zuzu@example.com')
-                self.assertEqual(
-                    self._handler.envelope.mail_from, data['mail_from'])
-                self.assertEqual(
-                    self._handler.envelope.rcpt_tos, data['rcpt_tos'])
-                # Reset the envelope explicitly.
-                client.rset()
-                client.vrfy('zuzu@example.com')
-                self.assertIsNone(self._handler.envelope.mail_from)
-                self.assertEqual(len(self._handler.envelope.rcpt_tos), 0)
-
-
 class TestSMTPWithController(unittest.TestCase):
     def test_mail_with_size_too_large(self):
         controller = SizedController(Sink(), 9999)