From 1f2e8af8aaf08db8df03e45962aa67e47297f3f5 Mon Sep 17 00:00:00 2001 From: Eygene Ryabinkin Date: Tue, 8 Jul 2014 12:23:05 +0400 Subject: [PATCH 1/5] Trade recursion by plain old cycle We can do a simpler and more stack-friendly hack for IMAP servers with limited line lenghts. Reported-by: Josh Berry, https://github.com/josh-berry GH: https://github.com/OfflineIMAP/offlineimap/pull/100 Signed-off-by: Eygene Ryabinkin --- offlineimap/folder/IMAP.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/offlineimap/folder/IMAP.py b/offlineimap/folder/IMAP.py index 1029cf7..05e0a36 100644 --- a/offlineimap/folder/IMAP.py +++ b/offlineimap/folder/IMAP.py @@ -755,14 +755,7 @@ class IMAPFolder(BaseFolder): def deletemessagesflags(self, uidlist, flags): self.__processmessagesflags('-', uidlist, flags) - def __processmessagesflags(self, operation, uidlist, flags): - # XXX: should really iterate over batches of 100 UIDs - if len(uidlist) > 101: - # Hack for those IMAP servers with a limited line length - self.__processmessagesflags(operation, uidlist[:100], flags) - self.__processmessagesflags(operation, uidlist[100:], flags) - return - + def __processmessagesflags_real(self, operation, uidlist, flags): imapobj = self.imapserver.acquireconnection() try: try: @@ -804,6 +797,16 @@ class IMAPFolder(BaseFolder): elif operation == '-': self.messagelist[uid]['flags'] -= flags + + def __processmessagesflags(self, operation, uidlist, flags): + # Hack for those IMAP servers with a limited line length + batch_size = 100 + while len(uidlist): + self.__processmessagesflags_real(operation, uidlist[:batch_size], flags) + uidlist = uidlist[batch_size:] + return + + # Interface from BaseFolder def change_message_uid(self, uid, new_uid): """Change the message from existing uid to new_uid From aa55b38e26e0ccb904638b4b99f563a45ab3ec0e Mon Sep 17 00:00:00 2001 From: Eygene Ryabinkin Date: Thu, 10 Jul 2014 10:24:11 +0400 Subject: [PATCH 2/5] Avoid copying array every time, just slice it Suggested-by: Josh Berry Signed-off-by: Eygene Ryabinkin --- Changelog.rst | 2 ++ offlineimap/folder/IMAP.py | 6 +++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/Changelog.rst b/Changelog.rst index 781e252..0b485ee 100644 --- a/Changelog.rst +++ b/Changelog.rst @@ -17,6 +17,8 @@ OfflineIMAP v6.5.6.1 (YYYY-MM-DD) * Various fixes in documentation. +* Fix unbounded recursion during flag update (Josh Berry). + OfflineIMAP v6.5.6 (2014-05-14) =============================== diff --git a/offlineimap/folder/IMAP.py b/offlineimap/folder/IMAP.py index 05e0a36..e317baf 100644 --- a/offlineimap/folder/IMAP.py +++ b/offlineimap/folder/IMAP.py @@ -801,9 +801,9 @@ class IMAPFolder(BaseFolder): def __processmessagesflags(self, operation, uidlist, flags): # Hack for those IMAP servers with a limited line length batch_size = 100 - while len(uidlist): - self.__processmessagesflags_real(operation, uidlist[:batch_size], flags) - uidlist = uidlist[batch_size:] + for i in xrange(0, len(uidlist), batch_size): + self.__processmessagesflags_real(operation, + uidlist[i:i + batch_size], flags) return From 73e2c95acd829282b537df4144a3145b9452c0e7 Mon Sep 17 00:00:00 2001 From: Eygene Ryabinkin Date: Sat, 2 Aug 2014 22:23:31 +0400 Subject: [PATCH 3/5] Create SQLite DB directory if it doesn't exist yet Also check if DB path doesn't point to a directory. By: Nick Farrell GitHub: https://github.com/OfflineIMAP/offlineimap/pull/102 Signed-off-by: Eygene Ryabinkin --- Changelog.rst | 4 ++++ offlineimap/folder/LocalStatusSQLite.py | 8 +++++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/Changelog.rst b/Changelog.rst index 0b485ee..082950f 100644 --- a/Changelog.rst +++ b/Changelog.rst @@ -8,6 +8,10 @@ ChangeLog OfflineIMAP v6.5.6.1 (YYYY-MM-DD) ================================= +* Create SQLite database directory if it doesn't exist + yet; warn if path is not a directory (Nick Farrell, + GutHub pull #102) + * Fix mangled message headers for servers without UIDPLUS: X-OfflineIMAP was added with preceeding '\n' instead of '\r\n' just before message was uploaded to the IMAP server. diff --git a/offlineimap/folder/LocalStatusSQLite.py b/offlineimap/folder/LocalStatusSQLite.py index f8921f1..e48545d 100644 --- a/offlineimap/folder/LocalStatusSQLite.py +++ b/offlineimap/folder/LocalStatusSQLite.py @@ -14,7 +14,7 @@ # You should have received a copy of the GNU General Public License # along with this program; if not, write to the Free Software # Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA -import os.path +import os import re from threading import Lock from .Base import BaseFolder @@ -51,6 +51,12 @@ class LocalStatusSQLiteFolder(BaseFolder): self._newfolder = False # flag if the folder is new + dirname = os.path.dirname(self.filename) + if not os.path.exists(dirname): + os.makedirs(dirname) + if not os.path.isdir(dirname): + raise UserWarning("SQLite database path '%s' is not a directory." % dirname) + # dblock protects against concurrent writes in same connection self._dblock = Lock() From 7df765cfdb4096a33f35bf709db6820a6c2c79fd Mon Sep 17 00:00:00 2001 From: Eygene Ryabinkin Date: Sun, 3 Aug 2014 16:47:26 +0400 Subject: [PATCH 4/5] Properly manipulate contents of messagelist for folder Create initializer function that puts default values to all fields of message list item. Fix all code that directly assigns some hash to the elements of messagelist: for direct assignments only initializer is now permitted, all other modification are done in-place. Signed-off-by: Eygene Ryabinkin --- offlineimap/folder/Base.py | 11 +++++++++++ offlineimap/folder/Gmail.py | 6 ++++++ offlineimap/folder/GmailMaildir.py | 6 ++++++ offlineimap/folder/IMAP.py | 8 +++++++- offlineimap/folder/LocalStatus.py | 19 ++++++++++++++++--- offlineimap/folder/LocalStatusSQLite.py | 16 ++++++++++++++-- offlineimap/folder/Maildir.py | 17 +++++++++++++++-- 7 files changed, 75 insertions(+), 8 deletions(-) diff --git a/offlineimap/folder/Base.py b/offlineimap/folder/Base.py index 41fd627..b56cd1b 100644 --- a/offlineimap/folder/Base.py +++ b/offlineimap/folder/Base.py @@ -236,6 +236,17 @@ class BaseFolder(object): You must call cachemessagelist() before calling this function!""" raise NotImplementedError + def msglist_item_initializer(self, uid): + """ + Returns value for empty messagelist element with given UID. + + This function must initialize all fields of messagelist item + and must be called every time when one creates new messagelist + entry to ensure that all fields that must be present are present. + + """ + raise NotImplementedError + def uidexists(self, uid): """Returns True if uid exists""" return uid in self.getmessagelist() diff --git a/offlineimap/folder/Gmail.py b/offlineimap/folder/Gmail.py index 2a598de..e3ef92a 100644 --- a/offlineimap/folder/Gmail.py +++ b/offlineimap/folder/Gmail.py @@ -110,6 +110,11 @@ class GmailFolder(IMAPFolder): else: return set() + # Interface from BaseFolder + def msglist_item_initializer(self, uid): + return {'uid': uid, 'flags': set(), 'labels': set(), 'time': 0} + + # TODO: merge this code with the parent's cachemessagelist: # TODO: they have too much common logics. def cachemessagelist(self): @@ -152,6 +157,7 @@ class GmailFolder(IMAPFolder): minor = 1) else: uid = long(options['UID']) + self.messagelist[uid] = self.msglist_item_initializer(uid) flags = imaputil.flagsimap2maildir(options['FLAGS']) m = re.search('\(([^\)]*)\)', options['X-GM-LABELS']) if m: diff --git a/offlineimap/folder/GmailMaildir.py b/offlineimap/folder/GmailMaildir.py index be0d20d..b62b701 100644 --- a/offlineimap/folder/GmailMaildir.py +++ b/offlineimap/folder/GmailMaildir.py @@ -56,6 +56,12 @@ class GmailMaildirFolder(MaildirFolder): return True return False #Nope, nothing changed + + # Interface from BaseFolder + def msglist_item_initializer(self, uid): + return {'flags': set(), 'labels': set(), 'filename': '/no-dir/no-such-file/', 'mtime': 0} + + def cachemessagelist(self): if self.messagelist is None: self.messagelist = self._scanfolder() diff --git a/offlineimap/folder/IMAP.py b/offlineimap/folder/IMAP.py index e317baf..a698888 100644 --- a/offlineimap/folder/IMAP.py +++ b/offlineimap/folder/IMAP.py @@ -202,6 +202,10 @@ class IMAPFolder(BaseFolder): return msgsToFetch + # Interface from BaseFolder + def msglist_item_initializer(self, uid): + return {'uid': uid, 'flags': set(), 'time': 0} + # Interface from BaseFolder def cachemessagelist(self): @@ -239,6 +243,7 @@ class IMAPFolder(BaseFolder): minor = 1) else: uid = long(options['UID']) + self.messagelist[uid] = self.msglist_item_initializer(uid) flags = imaputil.flagsimap2maildir(options['FLAGS']) rtime = imaplibutil.Internaldate2epoch(messagestr) self.messagelist[uid] = {'uid': uid, 'flags': flags, 'time': rtime} @@ -641,7 +646,8 @@ class IMAPFolder(BaseFolder): if imapobj: self.imapserver.releaseconnection(imapobj) if uid: # avoid UID FETCH 0 crash happening later on - self.messagelist[uid] = {'uid': uid, 'flags': flags} + self.messagelist[uid] = self.msglist_item_initializer(uid) + self.messagelist[uid]['flags'] = flags self.ui.debug('imap', 'savemessage: returning new UID %d' % uid) return uid diff --git a/offlineimap/folder/LocalStatus.py b/offlineimap/folder/LocalStatus.py index 7e5928e..1dccf90 100644 --- a/offlineimap/folder/LocalStatus.py +++ b/offlineimap/folder/LocalStatus.py @@ -57,6 +57,11 @@ class LocalStatusFolder(BaseFolder): os.unlink(self.filename) + # Interface from BaseFolder + def msglist_item_initializer(self, uid): + return {'uid': uid, 'flags': set(), 'labels': set(), 'time': 0, 'mtime': 0} + + def readstatus_v1(self, fp): """ Read status folder in format version 1. @@ -76,7 +81,8 @@ class LocalStatusFolder(BaseFolder): (line, self.filename) self.ui.warn(errstr) raise ValueError(errstr) - self.messagelist[uid] = {'uid': uid, 'flags': flags, 'mtime': 0, 'labels': set()} + self.messagelist[uid] = self.msglist_item_initializer(uid) + self.messagelist[uid]['flags'] = flags def readstatus(self, fp): @@ -100,7 +106,10 @@ class LocalStatusFolder(BaseFolder): (line, self.filename) self.ui.warn(errstr) raise ValueError(errstr) - self.messagelist[uid] = {'uid': uid, 'flags': flags, 'mtime': mtime, 'labels': labels} + self.messagelist[uid] = self.msglist_item_initializer(uid) + self.messagelist[uid]['flags'] = flags + self.messagelist[uid]['mtime'] = mtime + self.messagelist[uid]['labels'] = labels # Interface from BaseFolder @@ -197,7 +206,11 @@ class LocalStatusFolder(BaseFolder): self.savemessageflags(uid, flags) return uid - self.messagelist[uid] = {'uid': uid, 'flags': flags, 'time': rtime, 'mtime': mtime, 'labels': labels} + self.messagelist[uid] = self.msglist_item_initializer(uid) + self.messagelist[uid]['flags'] = flags + self.messagelist[uid]['time'] = rtime + self.messagelist[uid]['mtime'] = mtime + self.messagelist[uid]['labels'] = labels self.save() return uid diff --git a/offlineimap/folder/LocalStatusSQLite.py b/offlineimap/folder/LocalStatusSQLite.py index e48545d..a9ef6c2 100644 --- a/offlineimap/folder/LocalStatusSQLite.py +++ b/offlineimap/folder/LocalStatusSQLite.py @@ -184,14 +184,23 @@ class LocalStatusSQLiteFolder(BaseFolder): self._newfolder = True + # Interface from BaseFolder + def msglist_item_initializer(self, uid): + return {'uid': uid, 'flags': set(), 'labels': set(), 'time': 0, 'mtime': 0} + + # Interface from BaseFolder def cachemessagelist(self): self.messagelist = {} cursor = self.connection.execute('SELECT id,flags,mtime,labels from status') for row in cursor: + uid = row[0] + self.messagelist[uid] = self.msglist_item_initializer(uid) flags = set(row[1]) labels = set([lb.strip() for lb in row[3].split(',') if len(lb.strip()) > 0]) - self.messagelist[row[0]] = {'uid': row[0], 'flags': flags, 'mtime': row[2], 'labels': labels} + self.messagelist[uid]['flags'] = flags + self.messagelist[uid]['labels'] = labels + self.messagelist[uid]['mtime'] = row[2] # Interface from LocalStatusFolder def save(self): @@ -271,6 +280,7 @@ class LocalStatusSQLiteFolder(BaseFolder): self.savemessageflags(uid, flags) return uid + self.messagelist[uid] = self.msglist_item_initializer(uid) self.messagelist[uid] = {'uid': uid, 'flags': flags, 'time': rtime, 'mtime': mtime, 'labels': labels} flags = ''.join(sorted(flags)) labels = ', '.join(sorted(labels)) @@ -278,9 +288,11 @@ class LocalStatusSQLiteFolder(BaseFolder): (uid,flags,mtime,labels)) return uid + # Interface from BaseFolder def savemessageflags(self, uid, flags): - self.messagelist[uid] = {'uid': uid, 'flags': flags} + assert self.uidexists(uid) + self.messagelist[uid]['flags'] = flags flags = ''.join(sorted(flags)) self.__sql_write('UPDATE status SET flags=? WHERE id=?',(flags,uid)) diff --git a/offlineimap/folder/Maildir.py b/offlineimap/folder/Maildir.py index d067194..156b39e 100644 --- a/offlineimap/folder/Maildir.py +++ b/offlineimap/folder/Maildir.py @@ -191,7 +191,9 @@ class MaildirFolder(BaseFolder): else: uid = long(uidmatch.group(1)) # 'filename' is 'dirannex/filename', e.g. cur/123,U=1,FMD5=1:2,S - retval[uid] = {'flags': flags, 'filename': filepath} + retval[uid] = self.msglist_item_initializer(uid) + retval[uid]['flags'] = flags + retval[uid]['filename'] = filepath return retval # Interface from BaseFolder @@ -208,6 +210,12 @@ class MaildirFolder(BaseFolder): return True return False #Nope, nothing changed + + # Interface from BaseFolder + def msglist_item_initializer(self, uid): + return {'flags': set(), 'filename': '/no-dir/no-such-file/'} + + # Interface from BaseFolder def cachemessagelist(self): if self.messagelist is None: @@ -319,7 +327,9 @@ class MaildirFolder(BaseFolder): if rtime != None: os.utime(os.path.join(self.getfullname(), tmpname), (rtime, rtime)) - self.messagelist[uid] = {'flags': flags, 'filename': tmpname} + self.messagelist[uid] = self.msglist_item_initializer(uid) + self.messagelist[uid]['flags'] = flags + self.messagelist[uid]['filename'] = tmpname # savemessageflags moves msg to 'cur' or 'new' as appropriate self.savemessageflags(uid, flags) self.ui.debug('maildir', 'savemessage: returning uid %d' % uid) @@ -339,6 +349,9 @@ class MaildirFolder(BaseFolder): Note that this function does not check against dryrun settings, so you need to ensure that it is never called in a dryrun mode.""" + + assert uid in self.messagelist + oldfilename = self.messagelist[uid]['filename'] dir_prefix, filename = os.path.split(oldfilename) # If a message has been seen, it goes into 'cur' From aef88cc1f85ca021935b1ac09fd12652500a9ddf Mon Sep 17 00:00:00 2001 From: Eygene Ryabinkin Date: Fri, 22 Aug 2014 17:43:36 +0400 Subject: [PATCH 5/5] Fix label processing in GmailMaildir Commit 7df765cfdb4096a33f35bf709db6820a6c2c79fd introduced regression: GmailMaildir caches labels in its own function and it was testing the presence of the 'labels' key in message descriptor. But 7df765cf changed descriptor initialization and this key is always present. So now we have 'labels_cached' flag that tells us if labels were already cached or not. Signed-off-by: Eygene Ryabinkin --- offlineimap/folder/GmailMaildir.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/offlineimap/folder/GmailMaildir.py b/offlineimap/folder/GmailMaildir.py index b62b701..2cab213 100644 --- a/offlineimap/folder/GmailMaildir.py +++ b/offlineimap/folder/GmailMaildir.py @@ -59,7 +59,8 @@ class GmailMaildirFolder(MaildirFolder): # Interface from BaseFolder def msglist_item_initializer(self, uid): - return {'flags': set(), 'labels': set(), 'filename': '/no-dir/no-such-file/', 'mtime': 0} + return {'flags': set(), 'labels': set(), 'labels_cached': False, + 'filename': '/no-dir/no-such-file/', 'mtime': 0} def cachemessagelist(self): @@ -72,9 +73,10 @@ class GmailMaildirFolder(MaildirFolder): filepath = os.path.join(self.getfullname(), msg['filename']) msg['mtime'] = long(os.stat(filepath).st_mtime) + def getmessagelabels(self, uid): # Labels are not cached in cachemessagelist because it is too slow. - if not 'labels' in self.messagelist[uid]: + if not self.messagelist[uid]['labels_cached']: filename = self.messagelist[uid]['filename'] filepath = os.path.join(self.getfullname(), filename) @@ -88,10 +90,11 @@ class GmailMaildirFolder(MaildirFolder): self.messagelist[uid]['labels'] = \ imaputil.labels_from_header(self.labelsheader, self.getmessageheader(content, self.labelsheader)) - + self.messagelist[uid]['labels_cached'] = True return self.messagelist[uid]['labels'] + def getmessagemtime(self, uid): if not 'mtime' in self.messagelist[uid]: return 0