[Python-checkins] cpython (merge 3.2 -> 3.3): Merge: #5713: Handle 421 error codes during sendmail by closing the socket.

r.david.murray python-checkins at python.org
Thu Mar 21 02:15:13 CET 2013


http://hg.python.org/cpython/rev/cf5c40025af2
changeset:   82850:cf5c40025af2
branch:      3.3
parent:      82845:f2cc3d8b88bb
parent:      82849:bc538bc8e65d
user:        R David Murray <rdmurray at bitdance.com>
date:        Wed Mar 20 21:12:17 2013 -0400
summary:
  Merge: #5713: Handle 421 error codes during sendmail by closing the socket.

This is a partial fix to the issue of servers disconnecting unexpectedly; in
this case the 421 says they are disconnecting, so we close the socket and
return the 421 in the appropriate error context.

Original patch by Mark Sapiro, updated by Kushal Das, with additional
tests by me.

files:
  Lib/smtplib.py           |  13 ++++-
  Lib/test/test_smtplib.py |  74 ++++++++++++++++++++++++---
  Misc/NEWS                |   4 +
  3 files changed, 80 insertions(+), 11 deletions(-)


diff --git a/Lib/smtplib.py b/Lib/smtplib.py
--- a/Lib/smtplib.py
+++ b/Lib/smtplib.py
@@ -751,7 +751,10 @@
                 esmtp_opts.append(option)
         (code, resp) = self.mail(from_addr, esmtp_opts)
         if code != 250:
-            self.rset()
+            if code == 421:
+                self.close()
+            else:
+                self.rset()
             raise SMTPSenderRefused(code, resp, from_addr)
         senderrs = {}
         if isinstance(to_addrs, str):
@@ -760,13 +763,19 @@
             (code, resp) = self.rcpt(each, rcpt_options)
             if (code != 250) and (code != 251):
                 senderrs[each] = (code, resp)
+            if code == 421:
+                self.close()
+                raise SMTPRecipientsRefused(senderrs)
         if len(senderrs) == len(to_addrs):
             # the server refused all our recipients
             self.rset()
             raise SMTPRecipientsRefused(senderrs)
         (code, resp) = self.data(msg)
         if code != 250:
-            self.rset()
+            if code == 421:
+                self.close()
+            else:
+                self.rset()
             raise SMTPDataError(code, resp)
         #if we got here then somebody got our mail
         return senderrs
diff --git a/Lib/test/test_smtplib.py b/Lib/test/test_smtplib.py
--- a/Lib/test/test_smtplib.py
+++ b/Lib/test/test_smtplib.py
@@ -592,8 +592,12 @@
 # Simulated SMTP channel & server
 class SimSMTPChannel(smtpd.SMTPChannel):
 
-    # For testing failures in QUIT when using the context manager API.
     quit_response = None
+    mail_response = None
+    rcpt_response = None
+    data_response = None
+    rcpt_count = 0
+    rset_count = 0
 
     def __init__(self, extra_features, *args, **kw):
         self._extrafeatures = ''.join(
@@ -608,6 +612,8 @@
                 '250-DELIVERBY\r\n')
         resp = resp + self._extrafeatures + '250 HELP'
         self.push(resp)
+        self.seen_greeting = arg
+        self.extended_smtp = True
 
     def smtp_VRFY(self, arg):
         # For max compatibility smtplib should be sending the raw address.
@@ -646,30 +652,50 @@
             self.push('550 No access for you!')
 
     def smtp_QUIT(self, arg):
-        # args is ignored
         if self.quit_response is None:
             super(SimSMTPChannel, self).smtp_QUIT(arg)
         else:
             self.push(self.quit_response)
             self.close_when_done()
 
+    def smtp_MAIL(self, arg):
+        if self.mail_response is None:
+            super().smtp_MAIL(arg)
+        else:
+            self.push(self.mail_response)
+
+    def smtp_RCPT(self, arg):
+        if self.rcpt_response is None:
+            super().smtp_RCPT(arg)
+            return
+        self.push(self.rcpt_response[self.rcpt_count])
+        self.rcpt_count += 1
+
+    def smtp_RSET(self, arg):
+        super().smtp_RSET(arg)
+        self.rset_count += 1
+
+    def smtp_DATA(self, arg):
+        if self.data_response is None:
+            super().smtp_DATA(arg)
+        else:
+            self.push(self.data_response)
+
     def handle_error(self):
         raise
 
 
 class SimSMTPServer(smtpd.SMTPServer):
 
-    # For testing failures in QUIT when using the context manager API.
-    quit_response = None
+    channel_class = SimSMTPChannel
 
     def __init__(self, *args, **kw):
         self._extra_features = []
         smtpd.SMTPServer.__init__(self, *args, **kw)
 
     def handle_accepted(self, conn, addr):
-        self._SMTPchannel = SimSMTPChannel(
+        self._SMTPchannel = self.channel_class(
             self._extra_features, self, conn, addr)
-        self._SMTPchannel.quit_response = self.quit_response
 
     def process_message(self, peer, mailfrom, rcpttos, data):
         pass
@@ -809,18 +835,48 @@
         self.assertRaises(smtplib.SMTPServerDisconnected, smtp.send, b'foo')
 
     def test_with_statement_QUIT_failure(self):
-        self.serv.quit_response = '421 QUIT FAILED'
         with self.assertRaises(smtplib.SMTPResponseException) as error:
             with smtplib.SMTP(HOST, self.port) as smtp:
+                self.serv._SMTPchannel.quit_response = '421 QUIT FAILED'
                 smtp.noop()
         self.assertEqual(error.exception.smtp_code, 421)
         self.assertEqual(error.exception.smtp_error, b'QUIT FAILED')
-        # We don't need to clean up self.serv.quit_response because a new
-        # server is always instantiated in the setUp().
 
     #TODO: add tests for correct AUTH method fallback now that the
     #test infrastructure can support it.
 
+    # Issue 5713: make sure close, not rset, is called if we get a 421 error
+    def test_421_from_mail_cmd(self):
+        smtp = smtplib.SMTP(HOST, self.port, local_hostname='localhost', timeout=15)
+        self.serv._SMTPchannel.mail_response = '421 closing connection'
+        with self.assertRaises(smtplib.SMTPSenderRefused):
+            smtp.sendmail('John', 'Sally', 'test message')
+        self.assertIsNone(smtp.sock)
+        self.assertEqual(self.serv._SMTPchannel.rcpt_count, 0)
+
+    def test_421_from_rcpt_cmd(self):
+        smtp = smtplib.SMTP(HOST, self.port, local_hostname='localhost', timeout=15)
+        self.serv._SMTPchannel.rcpt_response = ['250 accepted', '421 closing']
+        with self.assertRaises(smtplib.SMTPRecipientsRefused) as r:
+            smtp.sendmail('John', ['Sally', 'Frank', 'George'], 'test message')
+        self.assertIsNone(smtp.sock)
+        self.assertEqual(self.serv._SMTPchannel.rset_count, 0)
+        self.assertDictEqual(r.exception.args[0], {'Frank': (421, b'closing')})
+
+    def test_421_from_data_cmd(self):
+        class MySimSMTPChannel(SimSMTPChannel):
+            def found_terminator(self):
+                if self.smtp_state == self.DATA:
+                    self.push('421 closing')
+                else:
+                    super().found_terminator()
+        self.serv.channel_class = MySimSMTPChannel
+        smtp = smtplib.SMTP(HOST, self.port, local_hostname='localhost', timeout=15)
+        with self.assertRaises(smtplib.SMTPDataError):
+            smtp.sendmail('John at foo.org', ['Sally at foo.org'], 'test message')
+        self.assertIsNone(smtp.sock)
+        self.assertEqual(self.serv._SMTPchannel.rcpt_count, 0)
+
 
 @support.reap_threads
 def test_main(verbose=None):
diff --git a/Misc/NEWS b/Misc/NEWS
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -196,6 +196,10 @@
 Library
 -------
 
+- Issue #5713: smtplib now handles 421 (closing connection) error codes when
+  sending mail by closing the socket and reporting the 421 error code via the
+  exception appropriate to the command that received the error response.
+
 - Issue #17192: Update the ctypes module's libffi to v3.0.13.  This
   specifically addresses a stack misalignment issue on x86 and issues on
   some more recent platforms.

-- 
Repository URL: http://hg.python.org/cpython


More information about the Python-checkins mailing list