Merge lp:~futatuki/mailman/2.1-add-smtp-timeout into lp:mailman/2.1

[Yasuhito FUTATSUKI at POEM]

This add a feature to specify timeout for SMTP response to avoid waiting response forever, to SMTPDirect Handler. To specify timeout, set SMTP_TIMEOUT in mm_cfg.py.

By default, this is disabled(waiting response until respond the MTA).

To test this feature, set mm_cfg.SMTP_TIMEOUT to small value and setup MTA to wait responding (by using greet pause feature, etc) and run smtp test or post message to mailing list to deliver it.

[Mark Sapiro (msapiro)]

Thank you for this contribution. I'm still considering how to handle this. I'd like to make Python 2.7 or at least Python 2.6 a minimum requirement, but that's not really an issue as your code handles older versions.

But there is an issue in how socket.timeout is handled in OutgoingRunner. Currently, OutgoingRunner treats all socket exceptions as a failure to connect. I think I'd want to catch socket.timeout in SMTPDirect, log the fact and then raise SomeRecipientsFailed instead.

Also I think it would be appropriate to set the default other than None. Maybe something like one minute or five minutes, but I'm not sure what a good value would be.

[Yasuhito FUTATSUKI at POEM (futatuki)]

Thank you for your consideration for this issue.

> But there is an issue in how socket.timeout is handled in OutgoingRunner.
> Currently, OutgoingRunner treats all socket exceptions as a failure to
> connect. I think I'd want to catch socket.timeout in SMTPDirect, log the fact
> and then raise SomeRecipientsFailed instead.

To accomplish it, there seems to be some different ways.
(1) inspect SMTPDirect exception to find what causes it.
(2) overriding methods of smtplib.SMTP catches socket.error (send() and getreply(), on current implementation of smtplib module (of CPython 2.x). socket.timeout can be caused in connect(), but it is not caught in connect()).
(3) write our own implementation for SMTP, instead of smtplib
(4) other...

(1) and (2) depend on undocumented part of smtplib (of current implementation in CPython 2) as Python standard library, though.

What kind of way to take do you think? (I'd like to help you for this issue if I can do something for it.)

> Also I think it would be appropriate to set the default other than None. Maybe
> something like one minute or five minutes, but I'm not sure what a good value
> would be.

I agree with None is not appropriate for default, but I also have no idea what is good. It is why I choose None for default value :-)
I think one minute is short even if the MTA trust QutgoingRunner, if checking message body, etc, but of course, this is only my personal opinion.

Unmerged revisions

1745. By Yasuhito FUTATSUKI at POEM on 2018-02-08

Add SMTP timeout feature to SMTPDirect handler

=== modified file 'Mailman/Defaults.py.in'
--- Mailman/Defaults.py.in	2018-01-30 04:06:24 +0000
+++ Mailman/Defaults.py.in	2018-02-08 12:35:39 +0000
@@ -591,6 +591,12 @@
 # uses DEFAULT_URL_HOST. Normally, you should not change this.
 SMTP_HELO_HOST = DEFAULT_URL_HOST
 
+# The time to wait for a response from MTA for DELIVERY_MODULE = 'SMTPDirect',
+# in positive float seconds or None. Setting a timeout of None disables 
+# timeouts on socket operations.
+# Note: too short timeout will often cause duplicate message delivery.
+SMTP_TIMEOUT = None
+
 # Set these variables if you need to authenticate to your NNTP server for
 # Usenet posting or reading.  If no authentication is necessary, specify None
 # for both variables.
 
=== modified file 'Mailman/Handlers/SMTPDirect.py'
--- Mailman/Handlers/SMTPDirect.py	2017-05-23 19:45:06 +0000
+++ Mailman/Handlers/SMTPDirect.py	2018-02-08 12:35:39 +0000
@@ -55,13 +55,24 @@
 
 
 
 
+if mm_cfg.SMTP_TIMEOUT:
+    # wrapper SMTP class to specify timeout (for Python < 2.6 compatibility)
+    class SMTP(smtplib.SMTP):
+        def connect(self, host='localhost', port=0):
+            sv_to = socket.getdefaulttimeout()
+            socket.setdefaulttimeout(mm_cfg.SMTP_TIMEOUT)
+            smtplib.SMTP.connect(self, host, port)
+            socket.setdefaulttimeout(sv_to)
+else:
+    SMTP = smtplib.SMTP
+
 # Manage a connection to the SMTP server
 class Connection:
     def __init__(self):
         self.__conn = None
 
     def __connect(self):
-        self.__conn = smtplib.SMTP()
+        self.__conn = SMTP()
         self.__conn.set_debuglevel(mm_cfg.SMTPLIB_DEBUG_LEVEL)
         self.__conn.connect(mm_cfg.SMTPHOST, mm_cfg.SMTPPORT)
         if mm_cfg.SMTP_AUTH: