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

Proposed by Yasuhito FUTATSUKI at POEM on 2018-02-08
Status: Needs review
Proposed branch: lp:~futatuki/mailman/2.1-add-smtp-timeout
Merge into: lp:mailman/2.1
Diff against target: 46 lines (+18/-1)
2 files modified
Mailman/Defaults.py.in (+6/-0)
Mailman/Handlers/SMTPDirect.py (+12/-1)
To merge this branch: bzr merge lp:~futatuki/mailman/2.1-add-smtp-timeout
Reviewer Review Type Date Requested Status
Mailman Coders 2018-02-08 Pending
Review via email: mp+337353@code.launchpad.net

Description of the change

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.

To post a comment you must log in.
Mark Sapiro (msapiro) wrote :

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.

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'Mailman/Defaults.py.in'
2--- Mailman/Defaults.py.in 2018-01-30 04:06:24 +0000
3+++ Mailman/Defaults.py.in 2018-02-08 12:35:39 +0000
4@@ -591,6 +591,12 @@
5 # uses DEFAULT_URL_HOST. Normally, you should not change this.
6 SMTP_HELO_HOST = DEFAULT_URL_HOST
7
8+# The time to wait for a response from MTA for DELIVERY_MODULE = 'SMTPDirect',
9+# in positive float seconds or None. Setting a timeout of None disables
10+# timeouts on socket operations.
11+# Note: too short timeout will often cause duplicate message delivery.
12+SMTP_TIMEOUT = None
13+
14 # Set these variables if you need to authenticate to your NNTP server for
15 # Usenet posting or reading. If no authentication is necessary, specify None
16 # for both variables.
17
18=== modified file 'Mailman/Handlers/SMTPDirect.py'
19--- Mailman/Handlers/SMTPDirect.py 2017-05-23 19:45:06 +0000
20+++ Mailman/Handlers/SMTPDirect.py 2018-02-08 12:35:39 +0000
21@@ -55,13 +55,24 @@
22
23
24
25
26+if mm_cfg.SMTP_TIMEOUT:
27+ # wrapper SMTP class to specify timeout (for Python < 2.6 compatibility)
28+ class SMTP(smtplib.SMTP):
29+ def connect(self, host='localhost', port=0):
30+ sv_to = socket.getdefaulttimeout()
31+ socket.setdefaulttimeout(mm_cfg.SMTP_TIMEOUT)
32+ smtplib.SMTP.connect(self, host, port)
33+ socket.setdefaulttimeout(sv_to)
34+else:
35+ SMTP = smtplib.SMTP
36+
37 # Manage a connection to the SMTP server
38 class Connection:
39 def __init__(self):
40 self.__conn = None
41
42 def __connect(self):
43- self.__conn = smtplib.SMTP()
44+ self.__conn = SMTP()
45 self.__conn.set_debuglevel(mm_cfg.SMTPLIB_DEBUG_LEVEL)
46 self.__conn.connect(mm_cfg.SMTPHOST, mm_cfg.SMTPPORT)
47 if mm_cfg.SMTP_AUTH:

Subscribers

People subscribed via source and target branches

to status/vote changes: