<DKIM> maxage causes loss of local email

[Nicolas Sebrecht]

[ Adding Mike, the author of the maxage feature. ]

On Sun, Mar 01, 2015 at 02:02:37AM -0500, Janna Martl wrote:

> On Sun, Mar 01, 2015 at 01:18:16AM +0100, Nicolas Sebrecht wrote:
> >Could you rewrite your analisys and provide more context about the
> >current vs expected behaviours? Giving references to the code (methods,
> >etc) might be a good starting point.
> 
> Suppose messages A and B were delivered to the remote folder maxage +
> 1 days ago, A was downloaded to the local folder maxage + 1 days ago,
> but B was only downloaded maxage - 1 days ago (contrived scenario to
> illustrate the two things that could happen). The current behavior is
> that B gets deleted from the local folder, but A does not. The
> expected behavior is that neither is deleted (or, at the very least,

Yes, the expected behaviour is no deletion at all.

> Here's how I traced through the code (all the references are to 6.5.6;
> I had a brief look at 6.5.7 and don't think it changed anything I'm
> talking about):
> 
> Start where __syncmessagesto_delete(self, dstfolder, statusfolder) (in
> Base.py) is called where self is the remote folder and dstfolder is
> the local folder. It defines deletelist to be the list of messages in
> the status folder messagelist that aren't in the remote folder
> messagelist ("not self.uidexists(uid)"). A and B are both in the
> status folder. I claim they're also both *not* in the remote folder
> messagelist: this list is formed in IMAP.py cachemessagelist(), which
> calls _msgs_to_fetch(), which only asks the IMAP server for messages
> that are < maxage days old.

Yes.

> Back to Base.py __syncmessagesto_delete(), look at the call
> folder.deletemessages(deletelist), where folder is the local folder.
> This ends up calling Maildir.py deletemessage() for each message on
> the deletelist. But in line 402, we see that this function returns
> (instead of deleting anything) if the message is in the local folder's
> messagelist. This messagelist was created by Maildir.py's
> cachemessagelist(), which calls _scanfolder(), which tries to exclude
> messages based on maxage. So at this point, we *want* A and B to be
> excluded -- then they will be spared from deletion. This maxage check
> calls _iswithinmaxage(), and actually does the date comparison based
> on the time found at the beginning of the message's filename (line
> 109). These filenames were originally created in Maildir.py's
> new_message_filename() (the function I tried to change), which calls
> _gettimeseq() to get the current time (i.e. the time of retrieval).
> 
> Upshot: A's filename has an older timestamp than B's filename. A is
> excluded from the local folder messagelist in _scanfolder(), hence
> spared from deletion in deletemessage(); B is not excluded, and is
> deleted.

OMG.

> Sorry if that was more verbosity than you wanted, but hope it made
> things clearer.

Thank you for this very good analisys. Your statements looks plain true.

Now, I wonder if this issue is there since the beginning of the maxage
feature. From my quick review, I'm afraid it's actually broken from the
beginning

  cde94e5047d459bc728167d8de64dbe4ad0067b9

back in Aug 2009... At least, I couldn't find any commit patching the
behaviour of maxage in a way that it could break it since then.

Mike, do you remember if you took such issue into consideration while
implementing maxage? I'd like your feedback on this to get more
backgrounds and get it correctly fixed.

Janna, I'm considering your patch to tune the filenames of the local
Maildir but I'd like to think more deeply about that.

Cheers,

-- 
Nicolas Sebrecht