As Gmail was only announcing the presence of the UIDPLUS extension
after we logged in, and we were then only getting server
capabilities before, a hack was introduced that checked the
existence of an APPENDUID reply, even if the server did not claim
to support it.
However, John Wiegley reports problems, where the APPENDUID would
be None, and we attempt to go this path (it seems that imaplib2
returns [None] if there is no such reply, so our test here for "!="
might fail. Given that this is an undocumented imaplib2 function
anyway, and we do fetch gmail capabilities after authentication,
this hack should no longer be necessary.
We had problems there earlier, where imapobj.response() would
return [None] although we had received a APPENDUID response from
the server, this might need more debugging and testing.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
Reported by sharat87 in https://github.com/spaetz/offlineimap/pull/38,
he would often get an unhandled Exception when trying to
releaseconnection() a connection that was not in the pool of
connections.
The reason this could happen is that when folder.IMAP.quickchanged()
raises an Exception in select(), we would release the connection in the
"except" handling, and than release the same connection in the "finally"
clause, which led to the error. The right thing is to only release the
connection once, of course.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
This reverts commit 4d47f7bf3c.
This is one of two candidates for introducing the instabilities that
John Wiegley observed. We need to reintroduce with careful testing only.
The original patch has been mostly reverted.
This reverts commit 47390e03d6.
It is one of two potential candidates for the APPENDUID
regression that John Wiegley reported. We need to examine this
carefully before reintroducing this patch.
Resolved Changelog.draft.rst conflict.
Prevent savemessage(), and savemessageflags() to occur in dryrun mode in
all backends. Still need to protect against deletemessage().
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
If we cannot identify the new UID after a sendmessage(), log a better error
message, including the server response for better debugging.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
This won't work in python3 anymore, so just use sorted() when needed.
In one case, we could remove the sort() completely as were were sanity checking
one line above, that we only having one UID as response which makes sorting
unneeded.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
'set' is builtin since python2.6, so remove the imports. Also 'ssl' exists
since 2.6 and has everything we need, so no need for conditional import
tests here anymore.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
To have the code work in python3, we need to convert all occurences of
raise Exception, "text" to be proper functions. This style also adheres to PEP8.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
Most servers support the UIDPLUS extension, and we don't have to search
headers after each uploaded message. There is no need to CHECK the imap
server after each message when there is no need to search headers.
I have not measured the performance impact on real world servers, but
this lets us do less unneeded work in the common case.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
We were not cleaning out possibly existing APPENDUID messages before
APPENDing a new message. In case an old message were still hanging
around, this could *possibly* lead to retrieving and old UID. Things
should have been fine, but we do want to play safe here.
Also, make use of the "official" imaplib2 .response() command rather
than the internal _get_untagged_response() function.
Remove the hack that we would be looking for APPENDUID responses even if
the server claimed not to support the UIDPLUS ext. We now poll server
CAPABILITIES after login, and Gmail does provide us with the UIDPLUS
capability after login.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
Rename getuidvalidity -> get_uidvalidity and cache the IMAP result.
1) Start modernizing our function names using more underscores
2) IMAPs implementation of get_uidvalidity was removing the UIDVALIDITY result
from the imaplib2 result stack, so subsequent calls would return "None".
As various functions can invoke this, this led to some errors that we
avoid by caching the current UIDVALIDITY value in the Folder instance.
There are more simplifications and improvements to be made.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
Recently the internal function use of imaplib2's _get_untagged_response()
was switched to use the public documented .response() function (which
should return the same data). However within a few fays we received reports
that both uses of a) the UIDVALIDITY fetching and b) the APPENDUID fetching
returned [None] as data although the IMAP log definitely shows that data
was returned. Revert to using the undocumented internal imaplib2 function,
that seemed to have worked without problems. This needs to be taken up to
the imaplib2 developer.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
We have a reported case where response('UIDVALIDITY') returned [None]
which results in an ugly non-intuitive crash. Sanity check and report
something nicer.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
Pass through the 'force' argument from selectro() to select() so that it
can also enforce a new SELECT even if we already are on that folder.
Also change the default parameter from '0' to 'False' to make clear that
this is a Bool.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
We were using the internal imaplib2 _get_untagged_response() functions a
few times. Replace 3 of these calls with 2 calls to the public function
response() rather than fudging with internals that could change anytime.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
This change looks harmless, but it fixes a severe bugfix, potentially
leading to data loss! It fixes the "on n new uploads, it will redownload
n-1, n-2, n-3,... messages during the next syncs" condition, and this is
what happens:
If there are more than one Mails to upload to a server, we do that by
repeatedly invoking folder.IMAP.savemessage(). If the server supports
the UIDPLUS extension we query the resulting UID by doing a:
imapobj._get_untagged_response('APPENDUID', True)
and that is exactly the problem. The "True" part causes the reply to
remain in the "response stack" of the imaplib2 library. When we do
the same call on a subsequent message and the connection is still on the
same folder, we will get the same UID response back (imaplib2 only looks
for the first matching response and returns that). The only time we
clear the response stack, is when the IMAP connection SELECTS a
different folder.
This means that when we upload 10 messages, the IMAP server gives us
always the same UID (that of the first one) back. And trying to write
out 10 different messages with the same UID will confuse OfflineIMAP.
This is the reason why we saw the ongoing UPLOADING/DOWNLOADING behavior
that people reported. And this is the reason why we saw the
inconsistency in the UID mapping in the IMAP<->IMAP case.
I urge everyone to upgrade ASAP. Sorry for that, I don't know why the
problem only became prevalent in the recent few releases as this code
has been there for quite a while.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
Previously, assigning a new UID to a mapped IMAP or Maildir repository
was done by loading the "local" item, saving it under a new UID and
deleting the old one. This involved lots of disk activity for nothing
more than an effective file rename in Maildirs, and lots of network
usage in the MappedUID cases.
We do this on every upload from a local to a remote item, so that can
potentially be quite expensive. This patch lets backends that support it
(Maildir, MappedUID) efficiently rename the file rather than having to
read the mail content, write it out as a new file and delete the old
file. This speeds up uploads from Maildir and the MappedUID server.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
1) Differentiate error messages between imaplib.abort and imaplib.error
exceptions in the log.
2) Drop connections in the case of imapobj.error, it also might denote a
broken connection.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
The default parameter value was "None", and we were comparing that
directly to the imaplib2 value of is_readonly which is False or True, so
the comparison always returned "False".
Fix this by setting the default parameter to "False" and not
"None". Also convert all users of that function to use False/True.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
A repositories 'reference value is always prefixed to the full folder
path, so we should do so when creating a new one. The code had existed
but was commented out since 2003, I guess the "reference" option is not
too often used.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
When checking for the IMAP4.abort() exception, we need of course to
perform:
except imapobj.abort:
and not
except imapobj.abort():
Thanks to Johannes Stezenbach <js@sig21.net> for pointing to the glitch.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
Dave identified a case where our new dropped connection handling did
not work out correctly: we use the retry_left variable to signify
success (0=success if no exception occured).
However, we were decrementing the variable AFTER all the exception
checks, so if there was one due to a dropped connection, it
could well be that we 1) did not raise an exception (because we want to
retry), and 2) then DECREMENTED retry_left, which indicated "all is
well, no need to retry".
The code then continued to check() the append, which failed with the
above message (because we obtained a new connection which had not even
selected the current folder and we were still in mode AUTH). The fix is
of course, to fix our logic: Decrement retry_left first, THEN decide
whether to raise() (retry_left==0) or retry (retry_left>0) which would
then correctly attempt another loop. I am sorry for this newbie type of
logic error. The retry count loop was too hastily slipped in, it seems.
Reported-by: Dave Abrahams <dave@boostpro.com>
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
If APPEND raises abort(), the (typ, dat) variables will not be set, so
we should not be using it for the OfflineImapError Exception
string. Fixing and prettifying the string formatting a bit at the same
time.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
getvisiblename() was only defined on IMAP(derived) foldertypes, but we
want it on eg. Maildirs too, so we define it centrally in Folder.Base.py
rather than only in folder.IMAP.py.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
IMAPFolder has the repository and foldername values so it can get the
transposed (aka visiblename) of a folder itself just fine. There is no
need to pass it in as an separate parameter.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
Signed-off-by: Nicolas Sebrecht <nicolas.s-dev@laposte.net>
It is possible to get the config parameter from the Repository() which is
set in BaseFolder, so we set self.config there and remove the various
methods and 'config' parameters that are superfluous.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
Signed-off-by: Nicolas Sebrecht <nicolas.s-dev@laposte.net>
We passed in the accountname to all derivatives of BaseFolder, such as
IMAPFolder(...,repository,...,accountname), although it is perfectly
possible to get the accountname from the Repository(). So remove this
unneeded parameter. Each backend had to define getaccountname() (although
the function is hardly used and most accessed .accountname directly).
On the other hand BaseFolder was using getaccountname but it never defined
the function. So make the sane thing, remove all definitions from backends
and define accountname() once in Basefolder. It was made a property and not
just a (public) attribute, so it will show up in our developer
documentation as public API.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
Signed-off-by: Nicolas Sebrecht <nicolas.s-dev@laposte.net>
As all Folders share these parameters, we can safely handle them in
BaseFolder. This makes sense, as BaseFolder has a getname() function
that returns self.name but nothing actually set self.name.
It also saves a few lines of code.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
Signed-off-by: Nicolas Sebrecht <nicolas.s-dev@laposte.net>
In getmessage() we were releaseing a connection when we detected a
dropped connection, but it turns out that this was not enough, we need
to explicitely discard it when we detect a dropped one. So add the
drop_conn=True parameter that was recently introduced to force the
discarding of the dead conection.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
Signed-off-by: Nicolas Sebrecht <nicolas.s-dev@laposte.net>
The quickchanged() function was not handling dropped connections yet. If
IMAP4.select() throws a FOLDER_RETRY error, we will now discard the
connection, reconnect and retry.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
Signed-off-by: Nicolas Sebrecht <nicolas.s-dev@laposte.net>
We rely on the number of mails being returned by the imapobj.select()
call, however that only happens if we "force" a real select() to occur.
Pass in the force parameter that I dropped earlier (we did not make use
of the return value when I dropped it, that is how it slipped through).
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
Signed-off-by: Nicolas Sebrecht <nicolas.s-dev@laposte.net>
We were retrying indefinitely on imapobj.abort() (as that is what
imaplib2 suggests), but if the failure occurs repeatedly, we'll never
quit this loop. So implement a counter that errs out after unsuccessful
retries.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
Signed-off-by: Nicolas Sebrecht <nicolas.s-dev@laposte.net>
If maxage is set too large, we would even SEARCH for negative
years. With devastating results. So implement some sanity check and err
out in case the year does not make sense.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
Signed-off-by: Nicolas Sebrecht <nicolas.s-dev@laposte.net>
We can use Imaplib's monthnames and shorten the construction of the date
by using them rather than hardcoding them again.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
Signed-off-by: Nicolas Sebrecht <nicolas.s-dev@laposte.net>
Results are delivered in a 1-element list, and somehow I managed to drop
a [0] in the previous patches. We need to look at the element of course,
or our string splitting will fail horribly. Sorry this somehow slipped
through.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
Signed-off-by: Nicolas Sebrecht <nicolas.s-dev@laposte.net>
If a folder is empty, most servers will return EXISTS 0 and imaplib2
passes back ['0'] as return value to a select(). It returns [None] if
no EXISTS response was given by the server at all.
Attempting to fetch the UIDs of 0 emails which leads to
various error messages (One server responds with "NO No matching
messages", Gmail seems to say "BAD Bad message sequence 1:*" for some
(although it is working fine for me with Gmail, so it might behave
different for different people).
In case we get an None or 0 back, we simply stop caching messages as the
folder is empty. This should fix the various error reports that have
popped up.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
Signed-off-by: Nicolas Sebrecht <nicolas.s-dev@laposte.net>
Make sure that when a connection is dropped during append, we really
discard the broken connection and get a new one, retrying. We retry
indefinitely on the specific abort() Exception, as this is what imaplib2
suggests us to do.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
Signed-off-by: Nicolas Sebrecht <nicolas.s-dev@laposte.net>
This function was badly named and completely undocumented. Rework it to
avoid copying the full UID list using an iterator. Make it possible to
hand it a list of UIDs as strings rather than implicitely relying on the
fact that they are numeric already. Document the code.
The behavior off the function itself remained otherwise unchanged.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
Signed-off-by: Nicolas Sebrecht <nicolas.s-dev@laposte.net>
SEARCH and FETCH were never checking that the IMAP server actually
returned OK. Throw OfflineImapErrors at severity FOLDER in case one of
them fails.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
Signed-off-by: Nicolas Sebrecht <nicolas.s-dev@laposte.net>
Some code cleanup. If we want to examine all messages of a folder, don't
try to find out how many there are and request a long list of all of them,
but simply request 1:*. This obliviates us from the need to force a select
even if we already had the folder selected and it requires us to send a
few less bytes over the wire.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
Signed-off-by: Nicolas Sebrecht <nicolas.s-dev@laposte.net>
Do away with the wrapping of this code in a try...except KeyError, as
this code cannot conceivably throw a KeyError. Even if it could, it
should be documented why we should simply return() in this case.
Shorten some of the variable names and minor code cleanup while taking
the git blame anyway.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
Signed-off-by: Nicolas Sebrecht <nicolas.s-dev@laposte.net>
Rather than passing in huge lists of continuous numbers which eventually
overflow the maximum command line length, we coalesce number ranges
before passing the UID sequence to SEARCH. This should do away with the
error that has been reported with busy mailing lists and 'maxage'.
Signed-off-by: Sebastian Spaeth <Sebastian@SSpaeth.de>
Signed-off-by: Nicolas Sebrecht <nicolas.s-dev@laposte.net>
