From be285e522ddc2afe1342065098d83070eef34479 Mon Sep 17 00:00:00 2001 From: Nicolas Sebrecht Date: Thu, 28 Jul 2016 07:18:23 +0200 Subject: [PATCH] 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 Signed-off-by: Nicolas Sebrecht --- offlineimap/folder/IMAP.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/offlineimap/folder/IMAP.py b/offlineimap/folder/IMAP.py index e3a7a55..f9a03d0 100644 --- a/offlineimap/folder/IMAP.py +++ b/offlineimap/folder/IMAP.py @@ -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]