IMAP4_SSL spin because of SSLSocket.suppress_ragged_eofs

[kevinwatters]

BPO	5949
Nosy	@warsaw, @rhettinger, @ncoghlan, @benjaminp, @bitdancer
Files	imaplib-r76683.patch: corrects the handling of EOF in readline() and _get_line() test_imaplib_issue5949-3.patch: PEP8ified tests test_imaplib_issue5949-py26.patch: tests backported for 2.6 socketserver_issue_6267.patch: backport for py26 of SocketServer patch from r73638 test_imaplib_issue5949_py3k.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 = 'https://github.com/bitdancer'
closed_at = <Date 2009-12-12.18:47:24.325>
created_at = <Date 2009-05-06.15:45:39.779>
labels = ['type-bug', 'library']
title = 'IMAP4_SSL spin because of SSLSocket.suppress_ragged_eofs'
updated_at = <Date 2009-12-12.18:47:24.324>
user = 'https://bugs.python.org/kevinwatters'

bugs.python.org fields:

activity = <Date 2009-12-12.18:47:24.324>
actor = 'r.david.murray'
assignee = 'r.david.murray'
closed = True
closed_date = <Date 2009-12-12.18:47:24.325>
closer = 'r.david.murray'
components = ['Library (Lib)']
creation = <Date 2009-05-06.15:45:39.779>
creator = 'kevinwatters'
dependencies = []
files = ['15456', '15474', '15510', '15511', '15512']
hgrepos = []
issue_num = 5949
keywords = ['patch']
message_count = 38.0
messages = ['87337', '87359', '87410', '91663', '93392', '93401', '93402', '93621', '93708', '95424', '96010', '96017', '96018', '96019', '96020', '96021', '96025', '96027', '96028', '96029', '96030', '96031', '96038', '96039', '96050', '96052', '96061', '96170', '96172', '96174', '96176', '96177', '96178', '96179', '96180', '96186', '96187', '96300']
nosy_count = 10.0
nosy_names = ['barry', 'rhettinger', 'ncoghlan', 'janssen', 'scott.dial', 'kevinwatters', 'benjamin.peterson', 'r.david.murray', 'rmore', 'rtucker']
pr_nums = []
priority = 'high'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue5949'
versions = ['Python 2.6', 'Python 3.1', 'Python 2.7', 'Python 3.2']

[kevinwatters]

I cannot reproduce this locally yet, but I have several bug reports from users showing
that IMAP4_SSL connections can get stuck in an infinite loop during logout()

File "imaplib.pyo", line 529, in logout
File "imaplib.pyo", line 1059, in _simple_command
File "imaplib.pyo", line 889, in _command_complete
File "imaplib.pyo", line 990, in _get_tagged_response
File "imaplib.pyo", line 907, in _get_response
File "imaplib.pyo", line 1000, in _get_line
File "imaplib.pyo", line 1170, in readline
File "ssl.pyo", line 136, in read

IMAP4_SSL uses ssl.wrap_socket and should probably be passing
suppress_ragged_eofs=False. Without this, self.sslobj.read() may return '' forever, and
both IMAP4_SSL.read() and IMAP4_SSL.readline() will spin. With
suppress_ragged_eofs=False, calling logout() may raise an SSLError instead.

Thoughts?

[rmore]

Isn't the call to read(1) blocking in this case?

If so the only reason it should spin would be if it returned an empty
string representing EOF.

Couldn't we just add a check for an empty string and, if found, raise an
EOF exception?

[kevinwatters]

I wasn't sure--the docs for SSLSocket.read
(http://docs.python.org/library/ssl.html#ssl.SSLSocket.read) say

"Reads up to nbytes bytes from the SSL-encrypted channel and returns
them."

With that "up to" I wasn't sure that an empty string would necessarily
mean EOF--I'm not familiar enough with _ssl.c to say either way.

[rtucker]

I can reproduce this problem with Python 2.6. It manifests itself as a
lengthy iteration through an IMAP SSL mailbox locking up after a long
while and the interpreter consuming all available system memory.

I suspect this to be the combination of doom:

imaplib.py:
1166 def readline(self):
1167 """Read line from remote."""
1168 line = []
1169 while 1:
1170 char = self.sslobj.read(1)
1171 line.append(char)
1172 if char == "\n": return ''.join(line)

ssl.py:
130 def read(self, len=1024):
131
132 """Read up to LEN bytes and return them.
133 Return zero-length string on EOF."""
134
135 try:
136 return self._sslobj.read(len)
137 except SSLError, x:
138 if x.args[0] == SSL_ERROR_EOF and
self.suppress_ragged_eofs:
139 return ''
140 else:
141 raise

After setting suppress_ragged_eofs=False, I now get:

ssl.SSLError: [Errno 8] _ssl.c:1325: EOF occurred in violation of protocol

... instead of an explosion. This I can trap and handle much more
easily than an infinite loop appending '' to a list on each iteration :-)

I can reliably reproduce this against my gmail mailbox, although it does
take some number of hours. I am not sure if this would be an imaplib
bug or a ssl bug; I'd think ssl, because it is Not the Python Way to
bury an exception like this.

[scottdial]

It would seem the logical patch would be to return the line when the
empty string is returned. This would fall in line with the behavior of
other objects with a readline() in python. In following with that, the
patch I have attached returns a truncated line. Therein, the _get_line()
implementation needs to actually check for the correct termination of
the line (it's arguably a bug as it is, just chucking away the last two
characters blindly).

[ncoghlan]

Probably too late for 2.6.3 - assigning to Barry to check anyway.

[warsaw]

I'm afraid so. Please consider this for landing after 2.6.3 is released.

[rhettinger]

Can this go into 2.6.4?

[warsaw]

Sorry, 2.6.4 will be solely to remove the brown paper bag of 2.6.3 from
our heads. This can go in 2.6.5 though.

[ncoghlan]

Unassigned from Barry since this isn't an RM review issue anymore