Re: [fetchmail-devel] [PATCH] handle imap EXPUNGE better

[Matthias A. <mat...@gm...>]

Sunil Shetye <sh...@bo...> writes:

> This is because fetchmail caches which mails are new and the size of
> those new mails. This caching is required to give a better
> performance, but leads to unexpected results when another email client
> is writing to the same mailbox:
>
> - An old mail could be delivered instead of a recent mail.
>
> - fetchmail passes the wrong size to the SMTP server in the MAIL FROM:
>   command. If the SMTP server has size restrictions, a wrong mail will
>   get rejected!
>
> - "fetchmail -vv" reports that there is a mismatch in the expected and
>   received message sizes.

Right.

> This patch causes fetchmail to quit the poll if there is a mismatch in
> the count of mails being deleted.

OK.

> This patch also handles servers which do not report RECENT on EXPUNGE
> in a better way. Now, recentcount is counted down on EXPUNGE of each
> mail along with count. This way, recentcount need not be reset to zero
> if the server does not give a new report of RECENT mails.

I have some concerns about this:

> +		    /* Some servers do not report RECENT after an EXPUNGE.
> +		     * For such servers, assume that the mail being
> +		     * expunged is a recent one. For other servers, we
> +		     * should get an updated RECENT report later and this
> +		     * assumption will have no effect. */
> +		    if (recentcount > 0)
> +			recentcount--;
> +		    actual_deletions++;

How do we know that the other client deleting messages is deleting
recent ones?  A getmail client with "delete_after" or moldremover or
such might remove old and seen messages and we're decreasing RECENT
without good reason. It looks as though the assumption were a bit bold.

It would be better to update the cache properly (watching out for
eventual iterator variables that need to be decremented, too) and
continue. From our own cached flags we should be able to deduce if the
expunged message was recent or not. If we don't know and the server
expunged a different mail.

Now, before you go delve into the code and hack away, take a step back
from the blackboard to see the whole picture: the RECENT flags are just
crap if several clients are accessing the mailbox. What happens to the
RECENT count if we reselect the mailbox? Can we actually use it? Do we
need to check \Seen flags instead, for lack of UID support in IMAP?

The long way to a fix, which is not going to be in 6.3.X, but 6.4.0 at
the earliest, will be to rewrite the UID code to be
protocol-independent, allow for storing attributes with messages (such
as arrival or retrieval timestamp, size) in the .fetchids file, and
store one file per (server, account, folder) tuple. We have your code
from 2003 that splits the uid files, and I made some .fetchids
reformatting change on top of that.  I still find uid.c ugly, and I
doubt it's up to the job, so we'll have to rewrite that.

Next, we'd have to teach the IMAP code to use UID/UIDVALIDITY on servers
that support it for client-side tracking and make that the default, and
make POP3+UIDL default. We have IMAP patches that use the UID, but
AFAICT they don't pay attention to the UIDVALIDITY and so require some
work. Again, this is all 6.4.X stuff.

Comments, suggestions?

-- 
Matthias Andree