python / cpython

The Python programming language
https://www.python.org
Other
63.08k stars 30.21k forks source link

smtplib mishandles SMTP disconnects #35629

Closed 1af41a4c-c5ec-4789-95e6-64c09c7f7f3c closed 22 years ago

1af41a4c-c5ec-4789-95e6-64c09c7f7f3c commented 22 years ago
BPO 487310
Nosy @gvanrossum, @warsaw
Files
  • smtplib.txt: Patch to smtplib
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields: ```python assignee = 'https://github.com/warsaw' closed_at = created_at = labels = ['library'] title = 'smtplib mishandles SMTP disconnects' updated_at = user = 'https://bugs.python.org/majid' ``` bugs.python.org fields: ```python activity = actor = 'barry' assignee = 'barry' closed = True closed_date = None closer = None components = ['Library (Lib)'] creation = creator = 'majid' dependencies = [] files = ['223'] hgrepos = [] issue_num = 487310 keywords = [] message_count = 6.0 messages = ['7886', '7887', '7888', '7889', '7890', '7891'] nosy_count = 3.0 nosy_names = ['gvanrossum', 'barry', 'majid'] pr_nums = [] priority = 'normal' resolution = 'accepted' stage = None status = 'closed' superseder = None type = None url = 'https://bugs.python.org/issue487310' versions = [] ```

    1af41a4c-c5ec-4789-95e6-64c09c7f7f3c commented 22 years ago

    smtplib handles SMTP disconnects (e.g. timeouts) inconsistently. The smtplib.SMTP.send() method does not reset self.file on a socket error, unlike getreply(), and thus a close() is necessary for in one case but not in the other.

    Recommendation: have smtplib.SMTP.send() call close() on socket.error just as smtplib.SMTP.getreply() does on EOF on self.file.

    In the following test case, I have set the SMTP timeout on my local machine to 10 seconds:

    Python 2.1.1 (#1, Nov  7 2001, 16:18:10)
    [GCC 2.95.3 20010315 (release)] on sunos5
    Type "copyright", "credits" or "license" for more
    information.
    >>> import smtplib
    >>> s = smtplib.SMTP('localhost', 25)
    >>> import time
    >>> time.sleep(10)
    >>> s.sendmail('fmajid@kefta.com', 'fmajid@kefta.com',
    '...')
    Traceback (most recent call last):
      File "<stdin>", line 1, in ?
      File "/usr/local/lib/python2.1/smtplib.py", line 469,
    in sendmail
        (code,resp) = self.helo()
      File "/usr/local/lib/python2.1/smtplib.py", line 305,
    in helo
        self.putcmd("helo", socket.getfqdn())
      File "/usr/local/lib/python2.1/smtplib.py", line 249,
    in putcmd
        self.send(str)
      File "/usr/local/lib/python2.1/smtplib.py", line 239,
    in send
        raise SMTPServerDisconnected('Server not connected')
    smtplib.SMTPServerDisconnected: Server not connected
    >>> s.connect('localhost', 25)
    Traceback (most recent call last):
      File "<stdin>", line 1, in ?
      File "/usr/local/lib/python2.1/smtplib.py", line 226,
    in connect
        (code,msg)=self.getreply()
      File "/usr/local/lib/python2.1/smtplib.py", line 271,
    in getreply
        raise SMTPServerDisconnected("Connection
    unexpectedly closed")
    smtplib.SMTPServerDisconnected: Connection unexpectedly
    closed
    >>> s.connect('localhost', 25)
    (220, 'bayazid.kefta.com ESMTP Postfix')
    1af41a4c-c5ec-4789-95e6-64c09c7f7f3c commented 22 years ago

    Logged In: YES user_id=110477

    I forgot to mention a workaround: if you catch SMTPServerDisconnected, call self.close(). The operation is idempotent so it isn't a problem if you do it twice, for instance if the exception occurs in getreply().

    gvanrossum commented 22 years ago

    Logged In: YES user_id=6380

    Can you please supply a patch?

    Upload a context diff, don't forget to check the file upload checkbox.

    1af41a4c-c5ec-4789-95e6-64c09c7f7f3c commented 22 years ago

    Logged In: YES user_id=110477

    I am attaching a patch that calls self.close() before raising the SMTPServerDisconnected exception.

    There does not seem to be a standard unit test for smtplib. I have run this patch through a unit test of my own that exercises smtplib. I don't have a way to test a broken MTA that disconnects on EHLO, however.

    1af41a4c-c5ec-4789-95e6-64c09c7f7f3c commented 22 years ago

    Logged In: YES user_id=110477

    Oops... forgot to check the checkbox for attachments.

    warsaw commented 22 years ago

    Logged In: YES user_id=12800

    Patch seems good to me, so I'm going to apply it to smtplib.py for both 2.2c1 and 2.2 trunk.

    Yes, it would be very nice to have a test suite for smtplib.py. I have a start of one in Mailman that uses smtpd.py. If you think it's worthwhile, why don't you upload your test suite to the patch tracker, assign it to me, and I'll merge it with what I have.

    This would be for Python 2.3.