Issue 4142: smtplib doesn't clear helo/ehlo flags on quit (original) (raw)

Created on 2008-10-17 20:02 by dwig, last changed 2022-04-11 14:56 by admin.

Files
File name Uploaded Description Edit
helo.patch Felix Schwarz,2009-02-15 15:41 Patch against /trunk, can probably be applied for 2.4.x, 2.5.x and 2.6.x also
flag_reset_4142.patch varun,2014-03-09 21:01 review
Messages (10)
msg74935 - (view) Author: Don Dwiggins (dwig) Date: 2008-10-17 20:02
Found on Windows, running Python 2.4.3. SMTP.login() and SMTP.sendmail set one of the attributes ehlo_resp or helo_resp to whatever the server responded (only if they're not already set). SMTP.quit() doesn't clear these attributes, so on the next connection, the HELO/EHLO commands won't be sent; this will cause problems with some servers (MDaemon in my case). The fix is obvious, and a workaround would be to clear the attributes in the instance before calling login (or sendmail where authentication isn't needed).
msg82158 - (view) Author: Felix Schwarz (Felix Schwarz) Date: 2009-02-15 15:17
I can confirm that this issue is still present in Python 2.5.2, 2.6.1 and 3.0.1. The current behavior of smtplib is a clear violation of the SMTP specification. The problem can be reproduced with code like that (stub, pseudo code-like): smtp = smtplib.SMTP() smtp.sendmail('from@example.com', 'foo@example.com', msg) smtp.quit() smtp.connect() # This command does not send EHLO/HELO again, violating the spec! smtp.sendmail('from@example.com', 'foo@example.com', msg) Real world bug reports due to this behavior were mostly visible with Exim because this MTA rejects the 'MAIL FROM' if SMTP extension verbs are used without EHLO: * http://groups.google.com/group/turbomail-devel/browse_thread/thread/a25c89a94b42724f * http://www.google.com/search?q=exim+python+%22malformed+address%22+size
msg82165 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2009-02-15 18:46
IIUC, the bug only occurs if you use the same SMTP object for multiple connections. I would claim that this is a bug in the application: SMTP objects are not designed to be used for multiple connections. You need to create a new one for each connection. If you want to extend the SMTP class to work for multiple connections, the provided patch is incorrect. Resetting the attributes should not be done in .connect() (i.e. for the new connection), but in .close() (i.e. when shutting down the previous connection). Furthermore, it should not only reset the two response attributes, but all per-connection attributes (which includes e.g. esmtp_features), and a test should be provided that, after close, only known "good" attributes remain set on the object (e.g. debuglevel).
msg82287 - (view) Author: Don Dwiggins (dwig) Date: 2009-02-17 00:17
Re: "SMTP objects are not designed to be used for multiple connections" ... "If you want to extend the SMTP class to work for multiple connections". Carefully parsing the library reference documentation, I can see that it could be interpreted that way: "A SMTP instance encapsulates _an_ SMTP connection" (emphasis added). I'd recommend making the documentation clearer in this regard, whether the implementation is extended or not. (And if non-reusability is intended, it'd be even more helpful if the connect() method were to throw a clear exception if called again.)
msg82290 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2009-02-17 00:27
It's neither deliberately designed to work for a single connection only, nor designed to support multiple of them. I guess (and only guess) that the authors of the module never considered your usage. So if you want to see something done, please contribute patches. I'm personally neutral which way they should go (and don't feel motivated to write patches myself - I can live with the status quo). Of course, any patch would set a precedent, and httplib and ftplib would then probably need to follow (requiring contributors then also).
msg101631 - (view) Author: Jesús Cea Avión (jcea) * (Python committer) Date: 2010-03-24 12:58
Support for smtpd too? (a stdlib addition in 2.7/3.2)
msg128157 - (view) Author: Sandro Tosi (sandro.tosi) * (Python committer) Date: 2011-02-07 22:09
Hello Felix/Don, are you still interested in seeing this fixed in smtplib? If so, what about incorporating Martin's suggestions and propose a new patch for review?
msg128181 - (view) Author: Don Dwiggins (dwig) Date: 2011-02-08 18:13
I have no pressing need for it. I'm happy enough with Martin's idea of single-use instances, as long as they're clearly documented as such, and don't "pretend" to work for a subsequent connect. I'd also be happy with instances that could be used for multiple connections, as long as that didn't increase the complexity of the code too much. Martin's also right that the similar clients for http and ftp, as well as smtp, should all work the same.
msg212985 - (view) Author: Varun Sharma (varun) * Date: 2014-03-09 21:01
I have added a patch and it's test case as per martin's suggestions. Now function close() resets the attributes :sock, default_port, ehlo_msg, ehlo_resp, helo_resp, source_address But it does *not* reset : debuglevel, does_esmtp,esmtp_features, file, local_hostname I think the attributes does_esmtp, esmtp_features and local_hostname are most probably not going to be changed even if someone uses the same object more than once.
msg213050 - (view) Author: Don Dwiggins (dwig) Date: 2014-03-10 16:51
Varun, thanks for the patch; sounds like the right way to go, in that it should work whether the usage is single-connection or multiple-connection. Ideally, the documentation would be expanded a bit to clarify just which attributes get reset on close().
History
Date User Action Args
2022-04-11 14:56:40 admin set github: 48392
2014-03-16 09:35:13 berker.peksag set stage: test needed -> patch reviewversions: + Python 3.4
2014-03-10 16:51:59 dwig set messages: +
2014-03-09 21:01:01 varun set files: + flag_reset_4142.patchnosy: + varunmessages: +
2011-03-14 12:12:40 jcea set nosy:loewis, jcea, giampaolo.rodola, dwig, r.david.murray, sandro.tosi, alfmelversions: + Python 3.3, - Python 2.6, Python 3.1, Python 3.2
2011-02-08 18:13:30 dwig set nosy:loewis, jcea, giampaolo.rodola, dwig, r.david.murray, sandro.tosi, alfmelmessages: +
2011-02-07 22:09:14 sandro.tosi set nosy: + sandro.tosi, - Felix Schwarzmessages: + stage: test needed
2011-01-04 01:39:48 pitrou set nosy: + r.david.murray
2010-08-17 14:57:35 giampaolo.rodola set nosy: + alfmel
2010-04-20 21:14:50 giampaolo.rodola set nosy: + giampaolo.rodola
2010-03-24 12:58:01 jcea set nosy: + jceamessages: + versions: + Python 3.1, Python 3.2, - Python 2.5, Python 2.4, Python 3.0
2009-08-05 20:58:40 amaury.forgeotdarc link issue6605 superseder
2009-02-17 00:27:47 loewis set messages: +
2009-02-17 00:17:46 dwig set messages: +
2009-02-15 18:46:28 loewis set nosy: + loewismessages: +
2009-02-15 15:45:25 Felix Schwarz set versions: + Python 2.6, Python 2.5, Python 3.0, Python 2.7
2009-02-15 15:41:42 Felix Schwarz set files: + helo.patchkeywords: + patch
2009-02-15 15:17:54 Felix Schwarz set nosy: + Felix Schwarzmessages: +
2008-10-17 20:02:42 dwig create