IMAP: don't take junk data for valid mail content

OfflineIMAP frequently delivers mail files to the Maildir consisting exclusively
of a single ASCII digit in IDLE mode. IMAPFolder.getmessage expects 'data' to be
of the form

  [(fetch-info, message-body)]

However, the imapobj.uid call in getmessage returns a list of *all* pending
untagged FETCH responses.  If any message flags were changed in the selected
IMAP folder since the last command (by another client or another thread in
OfflineIMAP itself), the IMAP server will issue unsolicited FETCH responses
indicating these flag changes (RFC3501, section 7).  When this happens, 'data'
will look like, for example

  ['1231 (FLAGS (\\Seen) UID 5300)',
   '1238 (FLAGS (\\Seen) UID 5318)',
   ('1242 (UID 5325 BODY[] {7976}', message-body)]

Unfortunately, getmessage retrieves the message body as data[0][1], which in
this example is just the string "2", and this is what gets stored in the mail
file.

Multi-threaded OfflineIMAP with IDLE or holdconnectionopen is particularly
susceptible to this problem because flag changes synced back to the IMAP server
on one thread will appear as unsolicited FETCH responses on another thread if it
happens to have the same folder selected.  This can also happen without IDLE or
holdconnectionopen or even in single-threaded OfflineIMAP with concurrent access
from other IMAP clients (webmail clients, etc.), though the window for the bug
is much smaller.

Ideally, either imaplib2 or getmessage would parse the fetch responses to find
the response for the requested UID.  However, since IMAP only specifies
unilateral FETCH responses for flag changes, it's almost certainly safe to
simply find the element of 'data' that is a tuple (perhaps aborting if there is
more than one tuple) and use that.

Github-fix: https://github.com/OfflineIMAP/offlineimap/issues/162
Based-on-patch-by: Austin Clements <amdragon@MIT.EDU>
Signed-off-by: Nicolas Sebrecht <nicolas.s-dev@laposte.net>
This commit is contained in:
Nicolas Sebrecht 2016-07-28 07:18:23 +02:00
parent 0ef11a8392
commit be285e522d

View File

@ -734,8 +734,14 @@ class IMAPFolder(BaseFolder):
# ``with`` without taking this into account.
self.imapserver.releaseconnection(imapobj)
if data == [None] or res_type != 'OK':
# IMAP server says bad request or UID does not exist.
# Ensure to not consider unsolicited FETCH responses caused by flag
# changes from concurrent connections. These appear as strings in
# 'data' (the BODY response appears as a tuple). This should leave
# exactly one response.
if res_type == 'OK':
data = [res for res in data if not isinstance(res, str)]
# Could not fetch message.
if data == [None] or res_type != 'OK' or len(data) != 1:
severity = OfflineImapError.ERROR.MESSAGE
reason = "IMAP server '%s' failed to fetch messages UID '%s'."\
"Server responded: %s %s"% (self.getrepository(), uids,
@ -764,7 +770,7 @@ class IMAPFolder(BaseFolder):
severity = OfflineImapError.ERROR.MESSAGE
reason = "IMAP server '%s' failed to store %s for message UID '%d'."\
"Server responded: %s %s"% (
self.getrepository(), field, uid, res_type, retdata)
self.getrepository(), field, uid, res_type, retdata)
raise OfflineImapError(reason, severity)
return retdata[0]