patch smtplib:when SMTPDataError, rset crashes with sslerror

[kxroberto]

BPO	1481032
Nosy	@warsaw, @terryjreedy, @tiran, @bitdancer, @zvyn
Files	smtplib_nosideeffecterror.patch issue1481032.patch

^{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:

assignee = None
closed_at = None
created_at = <Date 2006-05-03.11:59:13.000>
labels = ['3.7', 'type-bug', 'library']
title = 'patch smtplib:when SMTPDataError, rset crashes with sslerror'
updated_at = <Date 2016-09-08.15:03:02.164>
user = 'https://bugs.python.org/kxroberto'

bugs.python.org fields:

activity = <Date 2016-09-08.15:03:02.164>
actor = 'christian.heimes'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2006-05-03.11:59:13.000>
creator = 'kxroberto'
dependencies = []
files = ['18613', '36057']
hgrepos = []
issue_num = 1481032
keywords = ['patch']
message_count = 10.0
messages = ['50175', '50176', '50177', '114657', '114734', '147683', '223800', '226087', '226101', '275036']
nosy_count = 8.0
nosy_names = ['barry', 'terry.reedy', 'kxroberto', 'christian.heimes', 'hwundram', 'r.david.murray', 'jesstess', 'zvyn']
pr_nums = []
priority = 'normal'
resolution = None
stage = 'test needed'
status = 'pending'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue1481032'
versions = ['Python 3.5', 'Python 3.6', 'Python 3.7']

Linked PRs

• gh-43311: Add regression test for SSLError masking SMTPDataError in rset() #154866

[kxroberto]

File "smtplib.pyc", line 690, in sendmail
File "smtplib.pyc", line 445, in rset
File "smtplib.pyc", line 370, in docmd
File "smtplib.pyc", line 344, in getreply
File "smtplib.pyc", line 159, in readline
sslerror: (8, 'EOF occurred in violation of protocol')

traced from a py2.3 - yet unchanged.

=> hides original error SMTPDataError. such
SMTPDataError may have forced a disconnect of server.

patch for py2.3,py2.4,..

( I have this patch in my MUST-DO-PATCHes after any
Python installation. )

--

the patch passes on any error in this rset() location.
it also patches a error in PLAIN authentication

could cleanly catch socket.sslerror / socket.error /
EnvironmentError, yet ssl not always there ...
rset() is tested otherwise, so the nacked except is ok?

(there should maybe be special a common base Exception
class "IOBaseError" for : EnvironmentError(IOError,
OSError), EOFError, socket.error, socket.sslerror,
ftplib.all_errors, etc. Nice as it and not all IO
sublibs have to be imported to catch such errors.)

---

the same problem is with smtp.quit() on many SSL'ed
connections (without any other errors occuring): a
final socket.sslerror is raised during quit().
There may be a problem in the termination code of
ssl-FakeSockets or ssl.c . Or the same type of error
catch (on IOBaseError) should be applied. I am not sure

• in my apps I (must) catch on smtp.quit() generally.

(Compare also bug bpo-978833 / shutdown(2)-remedy in
httplib's SSL FakeSocket - this shutdown(2) remedy
patch of bpo-978833 I still have it on my MUST list
(py2.3/py2.4 installations), otherwise this FakeSocket
doesn't close fully in a FTPS application (where
termination on data channel is crucial for getting
response on the control channel) - and most likely puts
tremendous connection load on HTTPS servers because of
stale unterminated HTTPS connections while the bug may
not be obvious in casual usage. I'm not completely
clear about the nature of this error. Thus, what I say
is based on trial-and-error.

-robert

[hwundram]

Logged In: YES
user_id=791932

Where is the patch? :-)

[kxroberto]

Logged In: YES
user_id=972995

here (i forgot the upload checkbox?)

[BreamoreBoy]

The change to the encode_plain method in the patch was done via bpo-1075928 on 2004-12-06. The try/except change around a call to self.rset() was never implemented.

[kxroberto]

still I think all 3 self.rset()'s in SMTP.sendmail, which are followed by a raise <someerror> statement have to be bracketed with an except clause or so - nowadays better like """
try: self.res()
except (EnvironmentError, SMTPException):
pass
""", as all socket.error, sslerror seem meanwhile correctly inherited (via IOError).

Because the original error must pop up, not a side effect of state cleaning!

[kxroberto]

ping!
perhaps I forgot to write that I uploaded the cleaned patch also on 2010-08-23. I really think this simple patch is necessary. Just seen the same problem again - as I forgot the patch in one of my recent Python update installations.

When SMTPDataError, SMTPRecipientsRefused, SMTPSenderRefused should be raised, very likely a subsequent error (like closed connection etc.) overlappes the original Exception during self.rset(), and one will not be able to locate the problem on user level in eons ..

This patch is still in my vital-patches collection which I have to apply after every Python update since years.

[zvyn]

This bug was resolved for SMTPServerDisconnected exceptions but not for SSLError. Is it still reproducible? If so, the attached patch should fix it for 3.x (couldn't test if it does because I don't know how to reproduce the error).

[terryjreedy]

David, do you agree with either of the proposed changes?

[bitdancer]

Milan's patch should be correct. It would be nice to have a reproducer.

[tiran]

Is this still an issue? The bug is 10 years old and the patch two years.