From 41fad171257bb997b309faa7b51af1a1b6edbdd6 Mon Sep 17 00:00:00 2001 From: Scott Henson Date: Thu, 2 Jun 2011 17:04:52 -0400 Subject: [PATCH 01/19] Fix gssapi with multiple connections Fix a gssapi issue where threads beyond the first would not be able to authenticate against the imap server. This is done by using the connection lock around the gssapi authentication code and resetting (and releasing) the kerberos state after success so that subsequent connections may make use of kerberos. Signed-off-by: Scott Henson Reviewed-by: Sebastian Spaeth Signed-off-by: Nicolas Sebrecht --- offlineimap/imapserver.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/offlineimap/imapserver.py b/offlineimap/imapserver.py index e7892cc..0aaeca6 100644 --- a/offlineimap/imapserver.py +++ b/offlineimap/imapserver.py @@ -219,6 +219,7 @@ class IMAPServer: try: # Try GSSAPI and continue if it fails if 'AUTH=GSSAPI' in imapobj.capabilities and have_gss: + self.connectionlock.acquire() self.ui.debug('imap', 'Attempting GSSAPI authentication') try: @@ -229,8 +230,12 @@ class IMAPServer: 'GSSAPI Authentication failed') else: self.gssapi = True + kerberos.authGSSClientClean(self.gss_vc) + self.gss_vc = None + self.gss_step = self.GSS_STATE_STEP #if we do self.password = None then the next attempt cannot try... #self.password = None + self.connectionlock.release() if not self.gssapi: if 'AUTH=CRAM-MD5' in imapobj.capabilities: From 856982a4e6cbb7d75ddf489ed8fe741922a57de6 Mon Sep 17 00:00:00 2001 From: Sebastian Spaeth Date: Wed, 15 Jun 2011 10:59:00 +0200 Subject: [PATCH 02/19] Throw OfflineImapError when we try to request an inexistant message During a sync run, someone might remove or move IMAP messages. As we only cache the list of UIDs in the beginning, we might be requesting UIDs that don't exist anymore. Protect folder.IMAP.getmessage() against the response that we get when we ask for unknown UIDs. Also, if the server responds with anything else than "OK", (eg. Gmail seems to be saying frequently ['NO', 'Dave I can't let you do that now'] :-) so we should also be throwing OfflineImapErrors here rather than AssertionErrors. Signed-off-by: Sebastian Spaeth Signed-off-by: Nicolas Sebrecht --- Changelog.draft.rst | 3 +++ offlineimap/folder/IMAP.py | 19 +++++++++++++++---- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/Changelog.draft.rst b/Changelog.draft.rst index fd445ac..3d20bcf 100644 --- a/Changelog.draft.rst +++ b/Changelog.draft.rst @@ -20,6 +20,9 @@ Bug Fixes --------- +* We protect more robustly against asking for inexistent messages from the + IMAP server, when someone else deletes or moves messages while we sync. + Pending for the next major release ================================== diff --git a/offlineimap/folder/IMAP.py b/offlineimap/folder/IMAP.py index 8851b5b..62f220f 100644 --- a/offlineimap/folder/IMAP.py +++ b/offlineimap/folder/IMAP.py @@ -23,7 +23,7 @@ import re import time from copy import copy from Base import BaseFolder -from offlineimap import imaputil, imaplibutil +from offlineimap import imaputil, imaplibutil, OfflineImapError class IMAPFolder(BaseFolder): def __init__(self, imapserver, name, visiblename, accountname, repository): @@ -195,13 +195,24 @@ class IMAPFolder(BaseFolder): def getmessage(self, uid): """Retrieve message with UID from the IMAP server (incl body) - :returns: the message body + :returns: the message body or throws and OfflineImapError + (probably severity MESSAGE) if e.g. no message with + this UID could be found. """ imapobj = self.imapserver.acquireconnection() try: imapobj.select(self.getfullname(), readonly = 1) - res_type, data = imapobj.uid('fetch', '%d' % uid, '(BODY.PEEK[])') - assert res_type == 'OK', "Fetching message with UID '%d' failed" % uid + res_type, data = imapobj.uid('fetch', str(uid), '(BODY.PEEK[])') + if data == [None] or res_type != 'OK': + #IMAP server says bad request or UID does not exist + severity = OfflineImapError.ERROR.MESSAGE + reason = "IMAP server '%s' responded with '%s' to fetching "\ + "message UID '%d'" % (self.getrepository(), res_type, uid) + if data == [None]: + #IMAP server did not find a message with this UID + reason = "IMAP server '%s' does not have a message "\ + "with UID '%s'" % (self.getrepository(), uid) + raise OfflineImapError(reason, severity) # data looks now e.g. [('320 (UID 17061 BODY[] # {2565}','msgbody....')] we only asked for one message, # and that msg is in data[0]. msbody is in [0][1] From 2180f5fbf4d49c5f54b18918943bae22c079ae04 Mon Sep 17 00:00:00 2001 From: Sebastian Spaeth Date: Thu, 16 Jun 2011 17:22:29 +0200 Subject: [PATCH 03/19] UIDMappedFolder.savemessage() returned nothing, breaking API conventions Folder.savemessage() is supposed to return the new UID that a backend assigned, and it BaseFolder.copymessageto() fails if we don't return a non-negative number in the savemessage() there. For some reason, the UIDMappedFolder was not returning anything in savemessage, despite clearly stating in the code docs that it is supposed to return a UID. Not sure how long this has already been the case. This patch fixes the UIDMappedFolder to behave as it should. Signed-off-by: Sebastian Spaeth Signed-off-by: Nicolas Sebrecht --- offlineimap/folder/UIDMaps.py | 1 + 1 file changed, 1 insertion(+) diff --git a/offlineimap/folder/UIDMaps.py b/offlineimap/folder/UIDMaps.py index 76c5ee4..5f770ce 100644 --- a/offlineimap/folder/UIDMaps.py +++ b/offlineimap/folder/UIDMaps.py @@ -197,6 +197,7 @@ class MappingFolderMixIn: self._savemaps(dolock = 0) finally: self.maplock.release() + return uid def getmessageflags(self, uid): return self._mb.getmessageflags(self, self.r2l[uid]) From 335b320d9a3cefabc586c69ddfe62454752f3525 Mon Sep 17 00:00:00 2001 From: Sebastian Spaeth Date: Thu, 16 Jun 2011 17:09:29 +0200 Subject: [PATCH 04/19] Fix recursively scanning Maildir folders Commit 1754bf4110da251f4aa1dc0803889899b02bfcff introduced a blunder: - for dirname in os.listdir(toppath) + ['.']: + for dirname in os.listdir(toppath) + [toppath]: ... - if self.getsep() == '/' and dirname != '.': + if self.getsep() == '/' and dirname: This change was plainly wrong and would never have worked, so this commit reverts above bit. While touching the function, some minor code documentation, cleanup and limiting line length to 80 chars. Signed-off-by: Sebastian Spaeth Signed-off-by: Nicolas Sebrecht --- offlineimap/repository/Maildir.py | 36 +++++++++++++++++-------------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/offlineimap/repository/Maildir.py b/offlineimap/repository/Maildir.py index 2a10a93..86716e1 100644 --- a/offlineimap/repository/Maildir.py +++ b/offlineimap/repository/Maildir.py @@ -117,23 +117,23 @@ class MaildirRepository(BaseRepository): self.accountname, self.config) def _getfolders_scandir(self, root, extension = None): + """Recursively scan folder 'root'; return a list of MailDirFolder + + :param root: (absolute) path to Maildir root + :param extension: (relative) subfolder to examine within root""" self.debug("_GETFOLDERS_SCANDIR STARTING. root = %s, extension = %s" \ % (root, extension)) - # extension willl only be non-None when called recursively when - # getsep() returns '/'. retval = [] # Configure the full path to this repository -- "toppath" - - if extension == None: - toppath = root - else: + if extension: toppath = os.path.join(root, extension) - + else: + toppath = root self.debug(" toppath = %s" % toppath) # Iterate over directories in top & top itself. - for dirname in os.listdir(toppath) + [toppath]: + for dirname in os.listdir(toppath) + ['.']: self.debug(" *** top of loop") self.debug(" dirname = %s" % dirname) if dirname in ['cur', 'new', 'tmp']: @@ -153,17 +153,21 @@ class MaildirRepository(BaseRepository): os.path.isdir(os.path.join(fullname, 'new')) and os.path.isdir(os.path.join(fullname, 'tmp'))): # This directory has maildir stuff -- process - self.debug(" This is a maildir folder.") + self.debug(" This is maildir folder '%s'." % foldername) - self.debug(" foldername = %s" % foldername) - - if self.config.has_option('Repository ' + self.name, 'restoreatime') and self.config.getboolean('Repository ' + self.name, 'restoreatime'): + if self.config.has_option('Repository %s' % self, + 'restoreatime') and \ + self.config.getboolean('Repository %s' % self, + 'restoreatime'): self._append_folder_atimes(foldername) - retval.append(folder.Maildir.MaildirFolder(self.root, foldername, - self.getsep(), self, self.accountname, + retval.append(folder.Maildir.MaildirFolder(self.root, + foldername, + self.getsep(), + self, + self.accountname, self.config)) - if self.getsep() == '/' and dirname: - # Check sub-directories for folders. + if self.getsep() == '/' and dirname != '.': + # Recursively check sub-directories for folders too. retval.extend(self._getfolders_scandir(root, foldername)) self.debug("_GETFOLDERS_SCANDIR RETURNING %s" % \ repr([x.getname() for x in retval])) From 8634b0030d7aba5f421d00a17aef9440895b8253 Mon Sep 17 00:00:00 2001 From: Sebastian Spaeth Date: Tue, 14 Jun 2011 11:52:05 +0200 Subject: [PATCH 05/19] Always call ui.terminate() I discovered that we do not run ui.terminate in all circumstances, so make sure that we call with properly at the end of each run (whether in threaded or single-thread mode). Signed-off-by: Sebastian Spaeth Signed-off-by: Nicolas Sebrecht --- offlineimap/init.py | 1 + 1 file changed, 1 insertion(+) diff --git a/offlineimap/init.py b/offlineimap/init.py index 7258365..93b7224 100644 --- a/offlineimap/init.py +++ b/offlineimap/init.py @@ -343,6 +343,7 @@ class OfflineImap: t.start() threadutil.exitnotifymonitorloop(threadutil.threadexited) + ui.terminate() except KeyboardInterrupt: ui.terminate(1, errormsg = 'CTRL-C pressed, aborting...') return From 89cbdc9244689ff05b86aae8bf2fbeedfcd14eed Mon Sep 17 00:00:00 2001 From: Nicolas Sebrecht Date: Thu, 16 Jun 2011 23:38:55 +0200 Subject: [PATCH 06/19] Revert "Throw errors on connection refused and on non-standard SSL ports" This reverts commit 3dc9fc519a6fbbb5aad57744de570bdba2bcdfb4. --- offlineimap/imapserver.py | 38 +++++--------------------------------- 1 file changed, 5 insertions(+), 33 deletions(-) diff --git a/offlineimap/imapserver.py b/offlineimap/imapserver.py index 0aaeca6..10492cd 100644 --- a/offlineimap/imapserver.py +++ b/offlineimap/imapserver.py @@ -24,14 +24,9 @@ import offlineimap.accounts import hmac import socket import base64 -import errno from socket import gaierror -try: - from ssl import SSLError -except ImportError: - # Protect against python<2.6, use dummy and won't get SSL errors. - SSLError = None + try: # do we have a recent pykerberos? have_gss = False @@ -290,37 +285,14 @@ class IMAPServer: if(self.connectionlock.locked()): self.connectionlock.release() - # now, check for known errors and throw OfflineImapErrors - severity = OfflineImapError.ERROR.REPO - if isinstance(e, gaierror): + if type(e) == gaierror: #DNS related errors. Abort Repo sync + severity = OfflineImapError.ERROR.REPO #TODO: special error msg for e.errno == 2 "Name or service not known"? reason = "Could not resolve name '%s' for repository "\ "'%s'. Make sure you have configured the ser"\ - "ver name correctly and that you are online."%\ - (self.hostname, self.reposname) - raise OfflineImapError(reason, severity) - - elif isinstance(e, SSLError) and e.errno == 1: - # SSL unknown protocol error - # happens e.g. when connecting via SSL to a non-SSL service - if self.port != 443: - reason = "Could not connect via SSL to host '%s' and non-s"\ - "tandard ssl port %d configured. Make sure you connect"\ - " to the correct port." % (self.hostname, self.port) - else: - reason = "Unknown SSL protocol connecting to host '%s' for"\ - "repository '%s'. OpenSSL responded:\n%s"\ - % (self.hostname, self.reposname, e) - raise OfflineImapError(reason, severity) - - elif isinstance(e, socket.error) and e.args[0] == errno.ECONNREFUSED: - # "Connection refused", can be a non-existing port, or an unauthorized - # webproxy (open WLAN?) - reason = "Connection to host '%s:%d' for repository '%s' was "\ - "refused. Make sure you have the right host and port "\ - "configured and that you are actually able to access the "\ - "network." % (self.hostname, self.port, self.reposname) + "ver name correctly and that you are online."\ + % (self.hostname, self.reposname) raise OfflineImapError(reason, severity) # Could not acquire connection to the remote; # socket.error(last_error) raised From 5ffff9cf208fe756747a5f167a906541899b250f Mon Sep 17 00:00:00 2001 From: Sebastian Spaeth Date: Mon, 6 Jun 2011 11:37:25 +0200 Subject: [PATCH 07/19] Allow to create the root MailDir directory We currently do not allow nametrans rules such as nametrans = lambda foldername: re.sub('^INBOX$', '', foldername) because we crash with a traceback when running: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=499755 The underlying reason is that we cannot create the "top level" root directory of the Maildir in the function makefolders(), it will bail out. John Goerzen intentionally prevented offlineimap from creating the top-level dir, so that a misconfiguration could not arbitrarily create folders on the file system. I believe that it should be perfectly possible to automatically create the root dirctory of the maildir. We still protect against folder creations at arbitrary places in the file system though. This patch cleans up makefolders(), adds documentation, allows to automatically create rootfolders if needed (using absolute paths) and adds some robustness in case the folders already exist that we want to create (rather than simply crapping out). Signed-off-by: Sebastian Spaeth Tested-by: Mark Foxwell Signed-off-by: Nicolas Sebrecht --- offlineimap/repository/Maildir.py | 78 +++++++++++++++---------------- 1 file changed, 39 insertions(+), 39 deletions(-) diff --git a/offlineimap/repository/Maildir.py b/offlineimap/repository/Maildir.py index 18fa591..2a10a93 100644 --- a/offlineimap/repository/Maildir.py +++ b/offlineimap/repository/Maildir.py @@ -65,46 +65,46 @@ class MaildirRepository(BaseRepository): return self.getconf('sep', '.').strip() def makefolder(self, foldername): - self.debug("makefolder called with arg " + repr(foldername)) - # Do the chdir thing so the call to makedirs does not make the - # self.root directory (we'd prefer to raise an error in that case), - # but will make the (relative) paths underneath it. Need to use - # makedirs to support a / separator. - self.debug("Is dir? " + repr(os.path.isdir(foldername))) - if self.getsep() == '/': - for invalid in ['new', 'cur', 'tmp', 'offlineimap.uidvalidity']: - for component in foldername.split('/'): - assert component != invalid, "When using nested folders (/ as a separator in the account config), your folder names may not contain 'new', 'cur', 'tmp', or 'offlineimap.uidvalidity'." + """Create new Maildir folder if necessary - assert foldername.find('./') == -1, "Folder names may not contain ../" + :param foldername: A relative mailbox name. The maildir will be + created in self.root+'/'+foldername. All intermediate folder + levels will be created if they do not exist yet. 'cur', + 'tmp', and 'new' subfolders will be created in the maildir. + """ + self.debug("makefolder called with arg '%s'" % (foldername)) + full_path = os.path.abspath(os.path.join(self.root, foldername)) + + # sanity tests + if self.getsep() == '/': + for component in foldername.split('/'): + assert not component in ['new', 'cur', 'tmp'],\ + "When using nested folders (/ as a Maildir separator), "\ + "folder names may not contain 'new', 'cur', 'tmp'." + assert foldername.find('../') == -1, "Folder names may not contain ../" assert not foldername.startswith('/'), "Folder names may not begin with /" - oldcwd = os.getcwd() - os.chdir(self.root) - - # If we're using hierarchical folders, it's possible that sub-folders - # may be created before higher-up ones. If this is the case, - # makedirs will fail because the higher-up dir already exists. - # So, check to see if this is indeed the case. - - if (self.getsep() == '/' or self.getconfboolean('existsok', 0) or foldername == '.') \ - and os.path.isdir(foldername): - self.debug("makefolder: %s already is a directory" % foldername) - # Already exists. Sanity-check that it's not a Maildir. - for subdir in ['cur', 'new', 'tmp']: - assert not os.path.isdir(os.path.join(foldername, subdir)), \ - "Tried to create folder %s but it already had dir %s" %\ - (foldername, subdir) - else: - self.debug("makefolder: calling makedirs %s" % foldername) - assert foldername != '', "Can not create root MailDir." - os.makedirs(foldername, 0700) - self.debug("makefolder: creating cur, new, tmp") + # If we're using hierarchical folders, it's possible that + # sub-folders may be created before higher-up ones. + self.debug("makefolder: calling makedirs '%s'" % full_path) + try: + os.makedirs(full_path, 0700) + except OSError, e: + if e.errno == 17 and os.path.isdir(full_path): + self.debug("makefolder: '%s' already a directory" % foldername) + else: + raise for subdir in ['cur', 'new', 'tmp']: - os.mkdir(os.path.join(foldername, subdir), 0700) - # Invalidate the cache + try: + os.mkdir(os.path.join(full_path, subdir), 0700) + except OSError, e: + if e.errno == 17 and os.path.isdir(full_path): + self.debug("makefolder: '%s' already has subdir %s" % + (foldername, sudir)) + else: + raise + # Invalidate the folder cache self.folders = None - os.chdir(oldcwd) def deletefolder(self, foldername): self.ui.warn("NOT YET IMPLEMENTED: DELETE FOLDER %s" % foldername) @@ -132,11 +132,11 @@ class MaildirRepository(BaseRepository): self.debug(" toppath = %s" % toppath) - # Iterate over directories in top. - for dirname in os.listdir(toppath) + ['.']: + # Iterate over directories in top & top itself. + for dirname in os.listdir(toppath) + [toppath]: self.debug(" *** top of loop") self.debug(" dirname = %s" % dirname) - if dirname in ['cur', 'new', 'tmp', 'offlineimap.uidvalidity']: + if dirname in ['cur', 'new', 'tmp']: self.debug(" skipping this dir (Maildir special)") # Bypass special files. continue @@ -162,7 +162,7 @@ class MaildirRepository(BaseRepository): retval.append(folder.Maildir.MaildirFolder(self.root, foldername, self.getsep(), self, self.accountname, self.config)) - if self.getsep() == '/' and dirname != '.': + if self.getsep() == '/' and dirname: # Check sub-directories for folders. retval.extend(self._getfolders_scandir(root, foldername)) self.debug("_GETFOLDERS_SCANDIR RETURNING %s" % \ From 7570f718809b4883b03934837f639bf6c2af6971 Mon Sep 17 00:00:00 2001 From: Sebastian Spaeth Date: Fri, 17 Jun 2011 08:56:30 +0200 Subject: [PATCH 08/19] Throw OfflineImapError when we try to request an inexistant message During a sync run, someone might remove or move IMAP messages. As we only cache the list of UIDs in the beginning, we might be requesting UIDs that don't exist anymore. Protect folder.IMAP.getmessage() against the response that we get when we ask for unknown UIDs. Also, if the server responds with anything else than "OK", (eg. Gmail seems to be saying frequently ['NO', 'Dave I can't let you do that now'] :-) so we should also be throwing OfflineImapErrors here rather than AssertionErrors. Signed-off-by: Sebastian Spaeth Signed-off-by: Nicolas Sebrecht --- Changelog.draft.rst | 3 +++ offlineimap/folder/IMAP.py | 19 +++++++++++++++---- offlineimap/imapserver.py | 2 +- 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/Changelog.draft.rst b/Changelog.draft.rst index fd445ac..3d20bcf 100644 --- a/Changelog.draft.rst +++ b/Changelog.draft.rst @@ -20,6 +20,9 @@ Bug Fixes --------- +* We protect more robustly against asking for inexistent messages from the + IMAP server, when someone else deletes or moves messages while we sync. + Pending for the next major release ================================== diff --git a/offlineimap/folder/IMAP.py b/offlineimap/folder/IMAP.py index 8851b5b..62f220f 100644 --- a/offlineimap/folder/IMAP.py +++ b/offlineimap/folder/IMAP.py @@ -23,7 +23,7 @@ import re import time from copy import copy from Base import BaseFolder -from offlineimap import imaputil, imaplibutil +from offlineimap import imaputil, imaplibutil, OfflineImapError class IMAPFolder(BaseFolder): def __init__(self, imapserver, name, visiblename, accountname, repository): @@ -195,13 +195,24 @@ class IMAPFolder(BaseFolder): def getmessage(self, uid): """Retrieve message with UID from the IMAP server (incl body) - :returns: the message body + :returns: the message body or throws and OfflineImapError + (probably severity MESSAGE) if e.g. no message with + this UID could be found. """ imapobj = self.imapserver.acquireconnection() try: imapobj.select(self.getfullname(), readonly = 1) - res_type, data = imapobj.uid('fetch', '%d' % uid, '(BODY.PEEK[])') - assert res_type == 'OK', "Fetching message with UID '%d' failed" % uid + res_type, data = imapobj.uid('fetch', str(uid), '(BODY.PEEK[])') + if data == [None] or res_type != 'OK': + #IMAP server says bad request or UID does not exist + severity = OfflineImapError.ERROR.MESSAGE + reason = "IMAP server '%s' responded with '%s' to fetching "\ + "message UID '%d'" % (self.getrepository(), res_type, uid) + if data == [None]: + #IMAP server did not find a message with this UID + reason = "IMAP server '%s' does not have a message "\ + "with UID '%s'" % (self.getrepository(), uid) + raise OfflineImapError(reason, severity) # data looks now e.g. [('320 (UID 17061 BODY[] # {2565}','msgbody....')] we only asked for one message, # and that msg is in data[0]. msbody is in [0][1] diff --git a/offlineimap/imapserver.py b/offlineimap/imapserver.py index 0aaeca6..1eb8f6b 100644 --- a/offlineimap/imapserver.py +++ b/offlineimap/imapserver.py @@ -301,7 +301,7 @@ class IMAPServer: (self.hostname, self.reposname) raise OfflineImapError(reason, severity) - elif isinstance(e, SSLError) and e.errno == 1: + elif SSLError and isinstance(e, SSLError) and e.errno == 1: # SSL unknown protocol error # happens e.g. when connecting via SSL to a non-SSL service if self.port != 443: From b40d02e8012f4b86113c80033594fe9e1be0e7dd Mon Sep 17 00:00:00 2001 From: Sebastian Spaeth Date: Wed, 22 Jun 2011 21:38:19 +0200 Subject: [PATCH 09/19] repository/Maildir.py: Fix typo 'sudir' sudir->subdir in a debug statement. Thanks ccxCZ on IRC for the heads up. Signed-off-by: Sebastian Spaeth Signed-off-by: Nicolas Sebrecht --- offlineimap/repository/Maildir.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/offlineimap/repository/Maildir.py b/offlineimap/repository/Maildir.py index 86716e1..069748a 100644 --- a/offlineimap/repository/Maildir.py +++ b/offlineimap/repository/Maildir.py @@ -100,7 +100,7 @@ class MaildirRepository(BaseRepository): except OSError, e: if e.errno == 17 and os.path.isdir(full_path): self.debug("makefolder: '%s' already has subdir %s" % - (foldername, sudir)) + (foldername, subdir)) else: raise # Invalidate the folder cache From 3ce514e92ba7801d43bef1ee3911902975661cde Mon Sep 17 00:00:00 2001 From: Sebastian Spaeth Date: Fri, 24 Jun 2011 11:28:30 +0200 Subject: [PATCH 10/19] Simplify MappedIMAPFolder, fixing bugs Previously, we instanciated an MappedImapFolder, and would cleverly (too cleverly?) invoke methods on it casting it to an IMAPFolder by calling methods such as: self._mb.cachemessages(self) where self._MB is the class IMAPFolder and self and instance of MappedImapFolder. If e.g. cachemessages() invokes a method uidexists() which exists for MappedImapFolder, but not directly in IMAPFolder, I am not sure if Python would at some point attempt to use the method of the wrong class. Also, this leads to some twisted thinking as our class would in same cases act as an IMAPFolder and in some cases as an MappedImapFOlder and it is not always clear if we mean REMOTE UID or LOCAL UID. This commit simplifies the class, by a)doing away with the complex Mixin construct and directly inheriting from IMAPFOlder (so we get all the IMAPFOlder methods that we can inherit). We instantiate self._mb as a new instance of IMAPFolder which represents the local IMAP using local UIDs, separating the MappedIMAPFolder construct logically from the IMAPFolder somewhat. In the long run, I would like to remove self._mb completely and simply override any method that needs overriding, but let us take small and understandable baby steps here. Reported-and-tested-by: Vincent Beffara Signed-off-by: Sebastian Spaeth Signed-off-by: Nicolas Sebrecht --- offlineimap/folder/UIDMaps.py | 46 +++++++++++++++++------------------ 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/offlineimap/folder/UIDMaps.py b/offlineimap/folder/UIDMaps.py index 5f770ce..9233662 100644 --- a/offlineimap/folder/UIDMaps.py +++ b/offlineimap/folder/UIDMaps.py @@ -20,8 +20,11 @@ from threading import * from IMAP import IMAPFolder import os.path -class MappingFolderMixIn: - """Helper class to map between Folder() instances where both side assign a uid +class MappedIMAPFolder(IMAPFolder): + """IMAP class to map between Folder() instances where both side assign a uid + + This Folder is used on the local side, while the remote side should + be an IMAPFolder. Instance variables (self.): r2l: dict mapping message uids: self.r2l[remoteuid]=localuid @@ -29,10 +32,13 @@ class MappingFolderMixIn: #TODO: what is the difference, how are they used? diskr2l: dict mapping message uids: self.r2l[remoteuid]=localuid diskl2r: dict mapping message uids: self.r2l[localuid]=remoteuid""" - def _initmapping(self): + + def __init__(self, *args, **kwargs): + IMAPFolder.__init__(self, *args, **kwargs) self.maplock = Lock() (self.diskr2l, self.diskl2r) = self._loadmaps() - self._mb = self.__class__.__bases__[1] + self._mb = IMAPFolder(*args, **kwargs) + """Representing the local IMAP Folder using local UIDs""" def _getmapfilename(self): return os.path.join(self.repository.getmapdir(), @@ -81,8 +87,8 @@ class MappingFolderMixIn: return [mapping[x] for x in items] def cachemessagelist(self): - self._mb.cachemessagelist(self) - reallist = self._mb.getmessagelist(self) + self._mb.cachemessagelist() + reallist = self._mb.getmessagelist() self.maplock.acquire() try: @@ -137,7 +143,7 @@ class MappingFolderMixIn: cachemessagelist() before calling this function!""" retval = {} - localhash = self._mb.getmessagelist(self) + localhash = self._mb.getmessagelist() self.maplock.acquire() try: for key, value in localhash.items(): @@ -158,7 +164,7 @@ class MappingFolderMixIn: def getmessage(self, uid): """Returns the content of the specified message.""" - return self._mb.getmessage(self, self.r2l[uid]) + return self._mb.getmessage(self.r2l[uid]) def savemessage(self, uid, content, flags, rtime): """Writes a new message, with the specified uid. @@ -185,7 +191,7 @@ class MappingFolderMixIn: self.savemessageflags(uid, flags) return uid - newluid = self._mb.savemessage(self, -1, content, flags, rtime) + newluid = self._mb.savemessage(-1, content, flags, rtime) if newluid < 1: raise ValueError("Backend could not find uid for message") self.maplock.acquire() @@ -200,19 +206,19 @@ class MappingFolderMixIn: return uid def getmessageflags(self, uid): - return self._mb.getmessageflags(self, self.r2l[uid]) + return self._mb.getmessageflags(self.r2l[uid]) def getmessagetime(self, uid): return None def savemessageflags(self, uid, flags): - self._mb.savemessageflags(self, self.r2l[uid], flags) + self._mb.savemessageflags(self.r2l[uid], flags) def addmessageflags(self, uid, flags): - self._mb.addmessageflags(self, self.r2l[uid], flags) + self._mb.addmessageflags(self.r2l[uid], flags) def addmessagesflags(self, uidlist, flags): - self._mb.addmessagesflags(self, self._uidlist(self.r2l, uidlist), + self._mb.addmessagesflags(self._uidlist(self.r2l, uidlist), flags) def _mapped_delete(self, uidlist): @@ -233,22 +239,16 @@ class MappingFolderMixIn: self.maplock.release() def deletemessageflags(self, uid, flags): - self._mb.deletemessageflags(self, self.r2l[uid], flags) + self._mb.deletemessageflags(self.r2l[uid], flags) def deletemessagesflags(self, uidlist, flags): - self._mb.deletemessagesflags(self, self._uidlist(self.r2l, uidlist), + self._mb.deletemessagesflags(self._uidlist(self.r2l, uidlist), flags) def deletemessage(self, uid): - self._mb.deletemessage(self, self.r2l[uid]) + self._mb.deletemessage(self.r2l[uid]) self._mapped_delete([uid]) def deletemessages(self, uidlist): - self._mb.deletemessages(self, self._uidlist(self.r2l, uidlist)) + self._mb.deletemessages(self._uidlist(self.r2l, uidlist)) self._mapped_delete(uidlist) - -# Define a class for local part of IMAP. -class MappedIMAPFolder(MappingFolderMixIn, IMAPFolder): - def __init__(self, *args, **kwargs): - IMAPFolder.__init__(self, *args, **kwargs) - self._initmapping() From 416df0fc44749b9667d574e7f0ee093f53e12f28 Mon Sep 17 00:00:00 2001 From: Sebastian Spaeth Date: Fri, 24 Jun 2011 15:38:55 +0200 Subject: [PATCH 11/19] imaputil.imapsplit: Remove overzealous debug statement In an IMAP run where we did not have to sync anything, I spend nearly a fulls second in imaputil.debug() without even having debug output enabled. imapsplit is mainly called by flagsplit() which will also do debug output, so we get TONS of nearly duplicate debug output in the log which makes it really hard to analyze. Cut down the debug logging in imapsplit, we should be debug logging stuff at a slightly higher level than here anyway. This one-line change sped up my folder sync (without having to sync anything) by 0.5 seconds even when debugging is disabled. Signed-off-by: Sebastian Spaeth Signed-off-by: Nicolas Sebrecht --- offlineimap/imaputil.py | 1 - 1 file changed, 1 deletion(-) diff --git a/offlineimap/imaputil.py b/offlineimap/imaputil.py index 101f9a6..3905851 100644 --- a/offlineimap/imaputil.py +++ b/offlineimap/imaputil.py @@ -143,7 +143,6 @@ def imapsplit(imapstring): elif splitslen == 0: # There was not even an unquoted word. break - debug("imapsplit() returning:", retval) return retval flagmap = [('\\Seen', 'S'), From 6311716edbf05e730790151bb5c3b83e0f2cf9b7 Mon Sep 17 00:00:00 2001 From: Sebastian Spaeth Date: Fri, 24 Jun 2011 16:06:07 +0200 Subject: [PATCH 12/19] imapserver.py: Implement STARTTLS If we do not use a SSL connection anyway and if the server supports it, authenticate automatically with STARTTLS. Signed-off-by: Sebastian Spaeth Signed-off-by: Nicolas Sebrecht --- Changelog.draft.rst | 3 +++ offlineimap/imapserver.py | 11 +++++++++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/Changelog.draft.rst b/Changelog.draft.rst index 3d20bcf..fecce26 100644 --- a/Changelog.draft.rst +++ b/Changelog.draft.rst @@ -13,6 +13,9 @@ others. New Features ------------ +* Added StartTLS support, it will automatically be used if the server + supports it. + Changes ------- diff --git a/offlineimap/imapserver.py b/offlineimap/imapserver.py index 1aabcab..77a3c38 100644 --- a/offlineimap/imapserver.py +++ b/offlineimap/imapserver.py @@ -233,11 +233,18 @@ class IMAPServer: self.connectionlock.release() if not self.gssapi: + if 'STARTTLS' in imapobj.capabilities and not\ + self.usessl: + self.ui.debug('imap', + 'Using STARTTLS connection') + imapobj.starttls() + if 'AUTH=CRAM-MD5' in imapobj.capabilities: self.ui.debug('imap', - 'Attempting CRAM-MD5 authentication') + 'Attempting CRAM-MD5 authentication') try: - imapobj.authenticate('CRAM-MD5', self.md5handler) + imapobj.authenticate('CRAM-MD5', + self.md5handler) except imapobj.error, val: self.plainauth(imapobj) else: From 9d95d7bc62bc56af0f1d5c39a5489d26fdbb85f6 Mon Sep 17 00:00:00 2001 From: Haojun Bao Date: Mon, 27 Jun 2011 09:13:35 +0800 Subject: [PATCH 13/19] folder/IMAP: fix typo with maxsize and maxage. Signed-off-by: Bao Haojun Signed-off-by: Nicolas Sebrecht --- offlineimap/folder/IMAP.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/offlineimap/folder/IMAP.py b/offlineimap/folder/IMAP.py index 62f220f..9297a7b 100644 --- a/offlineimap/folder/IMAP.py +++ b/offlineimap/folder/IMAP.py @@ -137,7 +137,7 @@ class IMAPFolder(BaseFolder): search_condition += date_search_str if(maxsize != -1): - if(maxage != 1): #There are two conditions - add a space + if(maxage != -1): #There are two conditions - add a space search_condition += " " search_condition += "SMALLER " + self.config.getdefault("Account " + self.accountname, "maxsize", -1) From 928c363044510edfb5520e14975a065f9dc20165 Mon Sep 17 00:00:00 2001 From: Sebastian Spaeth Date: Mon, 27 Jun 2011 11:08:54 +0200 Subject: [PATCH 14/19] imapserver.py: Make severity var available where it is needed We we using the variable 'severity' in a few places to throw OfflineImapErrorrs of severity REPO. Somehow, that variable is now not accessible in all places that refer to it, so we move where it is defined to before all the 'if' checks which might make use of it. Signed-off-by: Sebastian Spaeth Signed-off-by: Nicolas Sebrecht --- offlineimap/imapserver.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/offlineimap/imapserver.py b/offlineimap/imapserver.py index 77a3c38..0713eb8 100644 --- a/offlineimap/imapserver.py +++ b/offlineimap/imapserver.py @@ -292,9 +292,9 @@ class IMAPServer: if(self.connectionlock.locked()): self.connectionlock.release() + severity = OfflineImapError.ERROR.REPO if type(e) == gaierror: #DNS related errors. Abort Repo sync - severity = OfflineImapError.ERROR.REPO #TODO: special error msg for e.errno == 2 "Name or service not known"? reason = "Could not resolve name '%s' for repository "\ "'%s'. Make sure you have configured the ser"\ @@ -328,8 +328,7 @@ class IMAPServer: if str(e)[:24] == "can't open socket; error": raise OfflineImapError("Could not connect to remote server '%s' "\ "for repository '%s'. Remote does not answer." - % (self.hostname, self.reposname), - OfflineImapError.ERROR.REPO) + % (self.hostname, self.reposname), severity) else: # re-raise all other errors raise From 8e2589982c9f2300fa15af7c387194cd4dd3bf9c Mon Sep 17 00:00:00 2001 From: Sebastian Spaeth Date: Tue, 14 Jun 2011 10:23:39 +0200 Subject: [PATCH 15/19] Don't use CStringIO to format a traceback The traceback module has format_exc() for this purpose so let's use that. Signed-off-by: Sebastian Spaeth Signed-off-by: Nicolas Sebrecht --- offlineimap/ui/UIBase.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/offlineimap/ui/UIBase.py b/offlineimap/ui/UIBase.py index 4c60b2a..5524579 100644 --- a/offlineimap/ui/UIBase.py +++ b/offlineimap/ui/UIBase.py @@ -21,7 +21,6 @@ import time import sys import traceback import threading -from StringIO import StringIO import offlineimap debugtypes = {'':'Other offlineimap related sync messages', @@ -309,10 +308,8 @@ class UIBase: s.terminate(100) def getMainExceptionString(s): - sbuf = StringIO() - traceback.print_exc(file = sbuf) - return "Main program terminated with exception:\n" + \ - sbuf.getvalue() + "\n" + \ + return "Main program terminated with exception:\n%s\n" %\ + traceback.format_exc() + \ s.getThreadDebugLog(threading.currentThread()) def mainException(s): From aec35eebd37c4ea608bfbe9ec1bfc164d9b566c6 Mon Sep 17 00:00:00 2001 From: Sebastian Spaeth Date: Fri, 1 Jul 2011 13:15:06 +0200 Subject: [PATCH 16/19] Reduce initial license blurb Rather than the extremly verbose NO WARRANTY blurb, we output a somewhat smaller initial text which should still make the GPL happy. Signed-off-by: Sebastian Spaeth Signed-off-by: Nicolas Sebrecht --- offlineimap/__init__.py | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/offlineimap/__init__.py b/offlineimap/__init__.py index f64f93d..688e50a 100644 --- a/offlineimap/__init__.py +++ b/offlineimap/__init__.py @@ -2,21 +2,20 @@ __all__ = ['OfflineImap'] __productname__ = 'OfflineIMAP' __version__ = "6.3.4-rc2" -__copyright__ = "Copyright (C) 2002 - 2010 John Goerzen" +__copyright__ = "Copyright 2002-2011 John Goerzen & contributors" __author__ = "John Goerzen" __author_email__= "john@complete.org" __description__ = "Disconnected Universal IMAP Mail Synchronization/Reader Support" +__license__ = "Licensed under the GNU GPL v2+ (v2 or any later version)" __bigcopyright__ = """%(__productname__)s %(__version__)s -%(__copyright__)s <%(__author_email__)s>""" % locals() - -banner = __bigcopyright__ + """ - -This software comes with ABSOLUTELY NO WARRANTY; see the file -COPYING for details. This is free software, and you are welcome -to distribute it under the conditions laid out in COPYING.""" - +%(__copyright__)s. +%(__license__)s. +""" % locals() __homepage__ = "http://github.com/nicolas33/offlineimap" -__license__ = "Licensed under the GNU GPL v2+ (v2 or any later version)." + + +banner = __bigcopyright__ + from offlineimap.error import OfflineImapError # put this last, so we don't run into circular dependencies using From 09ba269576ddb969e4d294211324fd4ae69daa12 Mon Sep 17 00:00:00 2001 From: Arnaud Fontaine Date: Wed, 6 Jul 2011 23:08:07 +0200 Subject: [PATCH 17/19] Add missing import of SSLError exception. In commit 89cbdc9, usage of SSLError was dropped but later reintroduced without importing SSLError exception. Signed-off-by: Arnaud Fontaine Signed-off-by: Sebastian Spaeth Signed-off-by: Nicolas Sebrecht --- offlineimap/imapserver.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/offlineimap/imapserver.py b/offlineimap/imapserver.py index 0713eb8..a235fd5 100644 --- a/offlineimap/imapserver.py +++ b/offlineimap/imapserver.py @@ -26,6 +26,11 @@ import socket import base64 from socket import gaierror +try: + from ssl import SSLError +except ImportError: + # Protect against python<2.6, use dummy and won't get SSL errors. + SSLError = None try: # do we have a recent pykerberos? From c293e64119c3217347b48a1c0564af6dbf9b3a7c Mon Sep 17 00:00:00 2001 From: Sebastian Spaeth Date: Wed, 6 Jul 2011 23:15:33 +0200 Subject: [PATCH 18/19] Fix caching inconsistencies in getselectedfolder() getselectedfolder was using a cached variable that we were setting in select(), but sometimes the IMAP4 instance got into the SELECTED state without explicitely select()ing, it seems, and our variable was unset. Let us just use the self.mailbox variable that imaplib2 is setting when select()ing rather than doing our own caching. Also remove the part where we were setting the cache. Just access self.state rather than looking up self.state via self.getstate() every time, it is just an unnecessary layer of redirection. Original-patch-by: Arnaud Fontaine Signed-off-by: Sebastian Spaeth Signed-off-by: Nicolas Sebrecht --- offlineimap/imaplibutil.py | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/offlineimap/imaplibutil.py b/offlineimap/imaplibutil.py index 20ab336..6492086 100644 --- a/offlineimap/imaplibutil.py +++ b/offlineimap/imaplibutil.py @@ -33,11 +33,9 @@ except ImportError: pass class UsefulIMAPMixIn: - def getstate(self): - return self.state def getselectedfolder(self): - if self.getstate() == 'SELECTED': - return self.selectedfolder + if self.state == 'SELECTED': + return self.mailbox return None def select(self, mailbox='INBOX', readonly=None, force = 0): @@ -58,10 +56,6 @@ class UsefulIMAPMixIn: (mailbox, result) severity = OfflineImapError.ERROR.FOLDER raise OfflineImapError(errstr, severity) - if self.getstate() == 'SELECTED': - self.selectedfolder = mailbox - else: - self.selectedfolder = None return result def _mesg(self, s, tn=None, secs=None): From 129b703bf20a154e3a16f7aa4c19877236336e32 Mon Sep 17 00:00:00 2001 From: Nicolas Sebrecht Date: Thu, 7 Jul 2011 18:43:18 +0200 Subject: [PATCH 19/19] v6.3.4-rc3 Signed-off-by: Nicolas Sebrecht --- Changelog.draft.rst | 5 ----- Changelog.rst | 32 ++++++++++++++++++++++++++++++++ offlineimap/__init__.py | 2 +- 3 files changed, 33 insertions(+), 6 deletions(-) diff --git a/Changelog.draft.rst b/Changelog.draft.rst index fecce26..87df0cd 100644 --- a/Changelog.draft.rst +++ b/Changelog.draft.rst @@ -13,9 +13,6 @@ others. New Features ------------ -* Added StartTLS support, it will automatically be used if the server - supports it. - Changes ------- @@ -23,8 +20,6 @@ Bug Fixes --------- -* We protect more robustly against asking for inexistent messages from the - IMAP server, when someone else deletes or moves messages while we sync. Pending for the next major release ================================== diff --git a/Changelog.rst b/Changelog.rst index 717e6f8..9e4bf8a 100644 --- a/Changelog.rst +++ b/Changelog.rst @@ -12,6 +12,38 @@ ChangeLog releases announces. +OfflineIMAP v6.3.4-rc3 (2011-07-07) +=================================== + +Notes +----- + +Here is a surprising release. :-) + +As expected we have a lot bug fixes in this round (see git log for details), +including a fix for a bug we had for ages (details below) which is a very good +news. + +What makes this cycle so unusual is that I merged a feature to support StartTLS +automatically (thanks Sebastian!). Another very good news. + +We usually don't do much changes so late in a cycle. Now, things are highly +calming down and I hope a lot of people will test this release. Next one could +be the stable! + +New Features +------------ + +* Added StartTLS support, it will automatically be used if the server + supports it. + +Bug Fixes +--------- + +* We protect more robustly against asking for inexistent messages from the + IMAP server, when someone else deletes or moves messages while we sync. + + OfflineIMAP v6.3.4-rc2 (2011-06-15) =================================== diff --git a/offlineimap/__init__.py b/offlineimap/__init__.py index 688e50a..4a0e52c 100644 --- a/offlineimap/__init__.py +++ b/offlineimap/__init__.py @@ -1,7 +1,7 @@ __all__ = ['OfflineImap'] __productname__ = 'OfflineIMAP' -__version__ = "6.3.4-rc2" +__version__ = "6.3.4-rc3" __copyright__ = "Copyright 2002-2011 John Goerzen & contributors" __author__ = "John Goerzen" __author_email__= "john@complete.org"