Bug 667725 - imapx_untagged: code should not be reached
This code is evil.
When we scan a folder for new messages, we issue a 'FETCH 1:* (UID FLAGS)'
or similar command.
When we receive an untagged FETCH from the server telling us flags for a
message, we make a decision about whether that information was solicited
by such a command, or whether it was unsolicited.
If it was unsolicited, we process it normally as an asynchronous flags
update and all is well.
If it was solicited, we add the UID to a list. When the FETCH (UID FLAGS)
command *completes*, we'll sort that list and then fetch the full headers
for each message.
However, we weren't very good at telling when an update was solicited.
Assuming that only solicited messages will have a UID is bogus.
This was failing if an unsolicited update came in when the (UID FLAGS)
fetch had completed, and we were already fetching the message headers.
The "new" UID would be added to the end of the list, even if we were
already fetching that message or if we already had it in cache. We'd
issue a FETCH command for it, and the barf when the server complied,
because when the UID list wasn't sorted we wouldn't find the offending
uid when we looked for it.
The simple "fix" for this is to keep a boolean flag 'scan_changes' which
is TRUE only when that FETCH (UID FLAGS) command is running. If a flags
change comes in at any other time, it is definitely unsolicited and
should *not* be added to the uidset. This at least protects us from
having UIDs added after we've sorted the list and started to do other
things with it, which was causing the crash.
In fact, this whole 'solicited' vs. 'unsolicited' thing is a design
mistake. In imapx_untagged() we should never care about what we asked
for and what we didn't. That's why the responses are *untagged*. The
server tells us things about the state of the mailboxes, and we should
process that information into our own local cache — it shouldn't
*matter* what we asked for. But that's a more intrusive fix for another day.
In addition, we were reliably *triggering* this behaviour in some cases
because we had to issue a SELECT for the folder in question before
issuing the FETCH (UID FLAGS) command. And on completion of the SELECT,
if UIDNEXT had increased, we were automatically issuing a *new* FETCH
(UID FLAGS) command starting from the last-known-uid in our cache. This
was entirely gratiutous, so use the same scan_changes boolean flag to
avoid it in that situation.