classification
Title: smtplib SMTP_SSL not working.
Type: behavior Stage: patch review
Components: Library (Lib) Versions: Python 3.4
process
Status: open Resolution:
Dependencies: 4066 Superseder:
Assigned To: Nosy List: bronger, catalin.iacob, giampaolo.rodola, lcatucci, pitrou, r.david.murray
Priority: normal Keywords: patch

Created on 2008-11-30 16:23 by lcatucci, last changed 2013-05-19 19:07 by lcatucci.

Files
File name Uploaded Description Edit
smtplib_01_default_port.diff lcatucci, 2008-12-02 16:32
smtplib_02_fix_ssl.diff lcatucci, 2008-12-02 16:33
smtplib_03_use_makefile.diff lcatucci, 2008-12-02 16:33
smtplib_04_remove_fakefile.diff lcatucci, 2008-12-02 16:33
smtplib_05_shutdown_socket.diff lcatucci, 2008-12-02 16:34
test_smtpnet.py lcatucci, 2009-02-07 00:24
smtplib_05_shutdown_socket_v2.patch catalin.iacob, 2011-06-17 21:32 Updated smtplib_05_shutdown_socket.diff review
v2_01_fix_lmtp_init lcatucci, 2011-06-18 08:55 Use class attribute for default connection port review
Messages (19)
msg76640 - (view) Author: Lorenzo M. Catucci (lcatucci) * Date: 2008-11-30 16:23
The enclosed patch does three things:
1. enables SMTP_SSL working: the _get_socket method was setting
   self.sock instead of returning the socket to the caller, which
   did reset self.sock to None
2. replace home-grown SSLFakeFile() with calls to ssl.socket's makefile()
   calls both in the starttls and in the SMTP_SSL cases
3. shutdown sockets before closing them, to avoid server-side piling and
   connection refused on connection-limited servers
The last change is just a cosmetical refactoring, but it really helps
the SMTP_SSL case: default_port should really be a class attribute,
instead of being set at __init__ time.
msg76759 - (view) Author: Lorenzo M. Catucci (lcatucci) * Date: 2008-12-02 16:34
I've reworked the patch into a series, like haypo requested for
poplib and imaplib.
msg88235 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2009-05-23 14:14
With the closure of 4066 all the tests in the test patch pass, so I'm
lowering the piority of this ticket.

I haven't reviewed the other patches, but the tests in the test patch
appear to be doing tests in the setup method, which doesn't seem like a
good idea.
msg138235 - (view) Author: Torsten Bronger (bronger) Date: 2011-06-13 12:29
I still have to apply Catucci's patch (or a modification of) after every Ubuntu installation or upgrade.  Otherwise, I get

  ...
  File "/usr/lib/python2.7/smtplib.py", line 752, in __init__
    SMTP.__init__(self, host, port, local_hostname, timeout)
  File "/usr/lib/python2.7/smtplib.py", line 239, in __init__
    (code, msg) = self.connect(host, port)
  File "/usr/lib/python2.7/smtplib.py", line 295, in connect
    self.sock = self._get_socket(host, port, self.timeout)
  File "/usr/lib/python2.7/smtplib.py", line 757, in _get_socket
    new_socket = socket.create_connection((host, port), timeout)
  File "/usr/lib/python2.7/socket.py", line 571, in create_connection
    raise err
socket.error: [Errno 111] Connection refused

But isn't it that #4066 should have solved the critical part this issue pair 4066/4470?
msg138519 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2011-06-17 15:56
Torsten, can you provide a clear, failing unittest for this?
msg138520 - (view) Author: Torsten Bronger (bronger) Date: 2011-06-17 16:08
No, I don't know how to do that.  All I can provide is a minimal version of my code that triggers the above mentioned traceback.  It is:

import smtplib

s = smtplib.SMTP_SSL("relay-auth.rwth-aachen.de")
s.login("***", "***")
s.sendmail("bronger@physik.rwth-aachen.de", "bronger.randys@googlemail.com"], "Hello")

I hope it helps to understand what I mean.
msg138521 - (view) Author: Torsten Bronger (bronger) Date: 2011-06-17 16:11
Sorry, it must be:

import smtplib

s = smtplib.SMTP_SSL("relay-auth.rwth-aachen.de")
s.login("***", "***")
s.sendmail("bronger@physik.rwth-aachen.de", ["bronger.randys@googlemail.com"], "Hello")

(A bracket was missing.)
msg138523 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2011-06-17 16:47
According to your traceback you should be seeing the error in the first line (the creation of the SMTP_SSL object).  If I run that line at the python prompt of python2.7.1, I get your connection failure.  If I run it using 2.7 tip (or 3.3 tip), the connection succeeds.  I know there have been other fixes in this area, but I don't know which one makes the difference.

Torsten, can you test with 2.7.2 and/or 2.7 tip?

I'm not sure what is left to do in this issue.  Do you have an opinion, Lorenzo?
msg138552 - (view) Author: Lorenzo M. Catucci (lcatucci) * Date: 2011-06-17 21:09
On Fri, 17 Jun 2011, R. David Murray wrote:

RDM> 
RDM> R. David Murray <rdmurray@bitdance.com> added the comment:
RDM> 
RDM> According to your traceback you should be seeing the error in the 
RDM> first line (the creation of the SMTP_SSL object).  If I run that line 
RDM> at the python prompt of python2.7.1, I get your connection failure.  
RDM> If I run it using 2.7 tip (or 3.3 tip), the connection succeeds.  I 
RDM> know there have been other fixes in this area, but I don't know which 
RDM> one makes the difference.
RDM> 
RDM> Torsten, can you test with 2.7.2 and/or 2.7 tip?
RDM> 
RDM> I'm not sure what is left to do in this issue.  Do you have an 
RDM> opinion, Lorenzo?
RDM> 

Torsten, would you mind letting us know both the python and the .deb 
version by running 

$ python2.7 --version

and

$ dpkg -l python2.7

At least in debian's python2.7_2.7.1-8, default_port is still an instance 
attribute instead of a class attribute. 

If the ubuntu situation is the same, the missing patch is 
smtplib_01_default_port.diff , and you could work-around the problem by 
explicitly calling s = smtplib.SMTP_SSL("relay-auth.rwth-aachen.de", 465).

The patch got in with:

changeset:   69931:bcf04ced5ef1
branch:      2.7
parent:      69915:7c3a20b5943a
user:        Antoine Pitrou <solipsis@pitrou.net>
date:        Sat May 07 19:59:33 2011 +0200
summary:     Issue #11927: SMTP_SSL now uses port 465 by default as 
documented.  Patch by Kasun Herath.

The last hunk, which fixes LMTP is still missing.

@@ -776,8 +777,9 @@
     authentication, but your mileage might vary."""

     ehlo_msg = "lhlo"
+    default_port = LMTP_PORT

-    def __init__(self, host = '', port = LMTP_PORT, local_hostname = 
None):
+    def __init__(self, host = '', port = 0, local_hostname = None):
         """Initialize a new instance."""
         SMTP.__init__(self, host, port, local_hostname)

Have a nice week-end, yours

	lorenzo

+-------------------------+----------------------------------------------+
|   Lorenzo M.  Catucci   | Centro di Calcolo e Documentazione           |
| catucci@ccd.uniroma2.it | Università degli Studi di Roma "Tor Vergata" |
|                         | Via O. Raimondo 18 ** I-00173 ROMA  ** ITALY |
|  Tel. +39 06 7259 2255  | Fax. +39 06 7259 2125                        |
+-------------------------+----------------------------------------------+
msg138555 - (view) Author: Catalin Iacob (catalin.iacob) * Date: 2011-06-17 21:32
Most of the problems in this issue were solved already so it could almost be closed:
* patch 1 was addressed in #11927
* patch 2 was addressed in #4066
* patches 3 and 4 were addressed in #11893

Torsten's problem was addressed by bcf04ced5ef1.

> I'm not sure what is left to do in this issue.

The only patch remaining is patch 5. I attached an updated version against tip of default branch. My patch mimics shutdown in imaplib.py in that it silences ENOTCONN. However I don't have a test that fails without the shutdown and I don't know if checking ENOTCONN is really needed. I tried to get shutdown to produce ENOTCONN by using Postfix as a server with smtpd_timeout=5s, connecting to it and waiting idle for more than 5 seconds before doing close(). In the Postfix logs I see that Postfix disconnects after 5 seconds of inactivity but doing shutdown afterwards doesn't trigger any exception so the ENOTCONN part remains unexercised.

My patch also adds shutdown method and SHUT_RDWR constant to mock_socket.py since otherwise test_smtplib fails.

(Added Antoine to nosy because he reviewed the patches for #11927 and #11893)
msg138561 - (view) Author: Torsten Bronger (bronger) Date: 2011-06-17 23:30
My Python version is "Python 2.7.1+" and the package is called "python2.7 2.7.1-5ubuntu2" (Ubuntu Natty).
msg138581 - (view) Author: Lorenzo M. Catucci (lcatucci) * Date: 2011-06-18 13:02
Just finished testing both 2.7 and default branches' socket close behaviour, and it seems 05 is not strictly needed.

I'd still prefer if smtplib_05_shutdown_socket_v2.patch since, this way the REMOTE socket close will be unconditionally correct, instead of being dependent on GC artifacts.
msg138688 - (view) Author: Lorenzo M. Catucci (lcatucci) * Date: 2011-06-20 05:07
I'd still prefer if smtplib_05_shutdown_socket_v2.patch could get in,
                                                       ^^^^^^^^^^^^^^
since, this way the REMOTE socket close will be unconditionally correct,
instead of being dependent on GC artifacts.
msg189591 - (view) Author: Torsten Bronger (bronger) Date: 2013-05-19 14:04
For five Ubuntu releases now, I apply this patch.  In contrast to Catalin's statement, it is not solved for me unless the upstream changes of two years ago haven't yet found their way into Ubuntu.  I'm currently using Python 2.7.4 on Ubuntu 13.04.  That said, the patch solves my issue every time.
msg189593 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-05-19 14:11
When you say "I apply this patch", you mean smtplib_05_shutdown_socket_v2.patch, right?
msg189594 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-05-19 14:12
In any case, I'm growing wary of bugfix regressions right now, so I would only apply the patch on the default branch.
msg189597 - (view) Author: Torsten Bronger (bronger) Date: 2013-05-19 14:49
Sorry, after having had another look at it, I realised that I have a different SSMTP issue now, non-Python-related.  So for me, Ubuntu 13.04 indeed solves my old issue.
msg189617 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2013-05-19 18:52
Lorenzo, any chance you could supply a unit test that fails without smtplib_05_shutdown_socket.diff applied?
msg189619 - (view) Author: Lorenzo M. Catucci (lcatucci) * Date: 2013-05-19 19:07
On Sun, 19 May 2013, R. David Murray wrote:

RDM> 
RDM> R. David Murray added the comment:
RDM> 
RDM> Lorenzo, any chance you could supply a unit test that fails without 
RDM> smtplib_05_shutdown_socket.diff applied?
RDM> 

It would really be a functional test, since the problem with half-open 
connection pile-up stems from smtp server's access control rules.

To test we should setup a fake smtp server, which forbids having multiple
connections from the same IP address, and connect twice in a row to the 
fake server. I'm not sure I'm able to implement both an smtpd.py server
serving more than one connection and the client code in a race-free way in 
the same "unit" test. Will try in the next week.

Thank you very much,

	lorenzo
History
Date User Action Args
2013-05-19 19:07:39lcatuccisetmessages: + msg189619
2013-05-19 18:52:24r.david.murraysetmessages: + msg189617
2013-05-19 14:49:26brongersetmessages: + msg189597
2013-05-19 14:12:19pitrousetmessages: + msg189594
versions: + Python 3.4, - Python 2.6, Python 3.1, Python 2.7, Python 3.2
2013-05-19 14:11:21pitrousetmessages: + msg189593
2013-05-19 14:04:28brongersetmessages: + msg189591
2011-06-20 05:07:42lcatuccisetmessages: + msg138688
2011-06-18 13:02:33lcatuccisetmessages: + msg138581
2011-06-18 10:11:49lcatuccisetfiles: - v2_02_mock_socket_shutdown
2011-06-18 09:01:51lcatuccisetfiles: - v2_03_shutdown_socket_on_close
2011-06-18 08:57:53lcatuccisetfiles: + v2_03_shutdown_socket_on_close
2011-06-18 08:56:57lcatuccisetfiles: + v2_02_mock_socket_shutdown
2011-06-18 08:55:59lcatuccisetfiles: + v2_01_fix_lmtp_init
2011-06-17 23:30:56brongersetmessages: + msg138561
2011-06-17 21:32:10catalin.iacobsetfiles: + smtplib_05_shutdown_socket_v2.patch
nosy: + pitrou, catalin.iacob
messages: + msg138555

2011-06-17 21:09:36lcatuccisetmessages: + msg138552
2011-06-17 16:47:53r.david.murraysetmessages: + msg138523
2011-06-17 16:11:20brongersetmessages: + msg138521
2011-06-17 16:08:58brongersetmessages: + msg138520
2011-06-17 15:56:07r.david.murraysetmessages: + msg138519
2011-06-13 12:29:09brongersetnosy: + bronger
messages: + msg138235
2010-04-20 20:54:48pitrousetversions: + Python 3.1, Python 2.7, Python 3.2
2009-05-23 14:14:58r.david.murraysetpriority: high -> normal
nosy: + r.david.murray
messages: + msg88235

2009-05-10 21:02:57ajaksu2setpriority: high
dependencies: + smtplib SMTP_SSL._get_socket doesn't return a value
type: behavior
stage: patch review
2009-02-07 00:24:37lcatuccisetfiles: + test_smtpnet.py
2008-12-02 16:34:33lcatuccisetfiles: + smtplib_05_shutdown_socket.diff
messages: + msg76759
2008-12-02 16:33:22lcatuccisetfiles: + smtplib_04_remove_fakefile.diff
2008-12-02 16:33:13lcatuccisetfiles: + smtplib_03_use_makefile.diff
2008-12-02 16:33:04lcatuccisetfiles: + smtplib_02_fix_ssl.diff
2008-12-02 16:32:54lcatuccisetfiles: + smtplib_01_default_port.diff
2008-12-02 16:32:40lcatuccisetfiles: - smtplib.py.patch
2008-12-01 21:44:46giampaolo.rodolasetnosy: + giampaolo.rodola
2008-11-30 16:23:33lcatuccicreate