From 08a579657a465c97feb4601c7e9d4e815289591d Mon Sep 17 00:00:00 2001 From: John Goerzen Date: Sun, 2 Mar 2008 22:25:05 -0600 Subject: [PATCH] Fix handling of servers that return UIDs in some FETCH responses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit closes #22 from pistore in OfflineIMAP #22: When an IMAP flag update is performed for multiple messages, some IMAP servers (e.g. Exchange) return the UID attribute only for some of the FETCH untagged responses, as shown in the following log: 21:19.04 > DCKF8 UID STORE 66050,50613,52164,40043,40055,25874 +FLAGS (\Deleted) 21:19.36 < * 35 FETCH (FLAGS (\Seen \Deleted) UID 25874) 21:19.36 < * 321 FETCH (FLAGS (\Seen \Deleted)) 21:19.57 < * 322 FETCH (FLAGS (\Seen \Deleted)) 21:19.57 < * 560 FETCH (FLAGS (\Seen \Deleted)) 21:19.57 < * 581 FETCH (FLAGS (\Seen \Deleted) UID 52164) 21:19.62 < * 1022 FETCH (FLAGS (\Seen \Deleted)) 21:19.62 < DCKF8 OK STORE completed. Function IMAPFolder.processmessagesflags is able to manage the servers which return the UID and the servers which do not return it, but is not able to deal with the mixed behavior shown above. The problem is that the fragment of function IMAPFolder.processmessagesflags that handles the responses with UID attribute uses variable flags to store the list of flags of the message in the IMAP format ("flags = attributehashFLAGS?"), while the fragment that handles the responses without UID expects variable "flags" to contain the list of modified flags passed to the function in Maildir format ("self.messagelist[uid]flags?.append(flag)"). As a consequence, the wrong list of flags is used for the messages without UID, leading to the addition of "strange" flags to the Maildir messages: Syncing messages IMAP[INBOX] -> Maildir[.] Adding flags to 4 messages on Maildir[.] Adding flags e to 4 messages on Maildir[.] Adding flags d to 4 messages on Maildir[.] Adding flags ) to 4 messages on Maildir[.] Adding flags ( to 4 messages on Maildir[.] Adding flags l to 4 messages on Maildir[.] Adding flags n to 4 messages on Maildir[.] Adding flags t to 4 messages on Maildir[.] Adding flags \ to 4 messages on Maildir[.] Adding flags D to 4 messages on Maildir[.] Deleting flags T to 4 messages on Maildir[.] Adding flags to 4 messages on LocalStatus[.] Adding flags e to 4 messages on LocalStatus[.] Adding flags d to 4 messages on LocalStatus[.] Adding flags ) to 4 messages on LocalStatus[.] Adding flags ( to 4 messages on LocalStatus[.] Adding flags l to 4 messages on LocalStatus[.] Adding flags n to 4 messages on LocalStatus[.] Adding flags t to 4 messages on LocalStatus[.] Adding flags \ to 4 messages on LocalStatus[.] Adding flags D to 4 messages on LocalStatus[.] Deleting flags T to 4 messages on LocalStatus[.] Fix: use a different variable to store IMAP flags when managing messages corresponding to responses with UID attribute, e.g.: *** IMAP.py.orig Wed Aug 22 18:23:17 2007 --- IMAP.py Wed Aug 22 18:22:38 2007 *************** class IMAPFolder(BaseFolder): *** 340,348 **** if not ('UID' in attributehash and 'FLAGS' in attributehash): # Compensate for servers that don't return a UID attribute. continue ! flags = attributehash['FLAGS'] uid = long(attributehash['UID']) ! self.messagelist[uid]['flags'] = imaputil.flagsimap2maildir(flags) try: needupdate.remove(uid) except ValueError: # Let it slide if it's not in the list --- 340,348 ---- if not ('UID' in attributehash and 'FLAGS' in attributehash): # Compensate for servers that don't return a UID attribute. continue ! lflags = attributehash['FLAGS'] uid = long(attributehash['UID']) ! self.messagelist[uid]['flags'] = imaputil.flagsimap2maildir(lflags) try: needupdate.remove(uid) except ValueError: # Let it slide if it's not in the list 02/03/08 14:04:35 changed by js * attachment flags-fix.patch added. Delete 02/03/08 14:05:24 changed by js ¶ Unfortunately I have to fetch some of my mail from an Exchange server (Microsoft Exchange Server 2003 IMAP4rev1 server version 6.5.7638.1) and I can confirm that the analysis of the problem is correct, and the patch given here fixes the problem. Looking at the code of the processmessagesflags() method I think it generally is a bug that the "flags" parameter is reused as a local variable, since the final "for uid in needupdate:" loop needs the original value of "flags". This only worked by accident. I'm attaching a unidiff version of the patch which applies cleanly against Debian unstable's offlineimap 5.99.4. --- offlineimap/folder/IMAP.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/offlineimap/folder/IMAP.py b/offlineimap/folder/IMAP.py index 9ed75ef..6b0e805 100644 --- a/offlineimap/folder/IMAP.py +++ b/offlineimap/folder/IMAP.py @@ -410,9 +410,9 @@ class IMAPFolder(BaseFolder): if not ('UID' in attributehash and 'FLAGS' in attributehash): # Compensate for servers that don't return a UID attribute. continue - flags = attributehash['FLAGS'] + lflags = attributehash['FLAGS'] uid = long(attributehash['UID']) - self.messagelist[uid]['flags'] = imaputil.flagsimap2maildir(flags) + self.messagelist[uid]['flags'] = imaputil.flagsimap2maildir(lflags) try: needupdate.remove(uid) except ValueError: # Let it slide if it's not in the list