On Sat, 8 Jun 2013, Jan Kundrát wrote:
On Saturday, 8 June 2013 08:10:48 CEST, Jonathan Abbey wrote:
http://github.com/jonabey/panda-imap
Assuming your github username is with double Bs, the offending code is likely
found at
https://github.com/jonabbey/panda-imap/blob/master/src/imapd/imapd.c#L939 .
I'm not sure whether I'm interpreting it correctly, it's a foreign code with
a very different style than I'm used to.
True, Mark had a coding style that is recognizable by anybody who learned
programming 35 (or more ;) years ago. (when systems had 64KB of main memory
and 5MB of disk space, programmers needed an economy of style that bordered on
terse).
That code appears to be broken on a fourth read or so :) -- it iterates over
all messages and if a particular sequence of messages matches, it will output
the result as min:max. That is, however, the exact problem here -- the code
does not check whether the range of UIDs assigned to these messages has any
holes within. In other words, the code produces correct results for SEARCH
RETURN ..., but fails for UID SEARCH RETURN ... because the UIDs, unlike
sequence numbers, are not guaranteed to be consecutive.
Mark's long standing contention was that the ESEARCH rfc allowed his
interpretation, so his code wasn't broken, just different.
As he's no longer around to defend his position I'm not going to comment
on it. Anybody who's interested can read the ietf.org mail archives.
This is consistent with the output of `git blame` which suggests that the
code in question comes from commit 199adc96 which is marked as
"imap-2006i.tar.Z" in your repository. The capability advertised by the
user's server says 2007e.404, and that version is still affected.
You can easily check this yourself. Create new mailbox, append three
messages, remove the middle one and send `UID SEARCH RETURN (ALL) ALL`. If
you get back something like "1,3", the code is OK, if you get "1:3", it's
wrong.
I do not dare writing a patch for that code -- the assumption that one could
get away with acting as if the client knew which UIDs exist and which are
gone is a big one. The loop at line 952 looks like the obvious problem (it
shall check for a consecutive range of UIDs, but only if the command is the
UID variant of ESEARCH...), but more changes might be needed. Well, I've had
enough C for now :).
As somebody who has over 15 years experience hacking Mark's imap code, I feel
comfortable creating a patch for that section to bring its behavior inline
with what the rest of the world thinks it should do. See attached patch file.
It should apply to Panda-imap & UW-IMAPD 2007e.
Jon,
Would you please consider adding this patch to your project.
With kind regards,
Jan
[1] http://tools.ietf.org/html/rfc3676
--
Dave Funk University of Iowa
<dbfunk (at) engineering.uiowa.edu> College of Engineering
319/335-5751 FAX: 319/384-0549 1256 Seamans Center
Sys_admin/Postmaster/cell_admin Iowa City, IA 52242-1527
#include <std_disclaimer.h>
Better is not better, 'standard' is better. B{*** imapd.c.orig 2013-04-16 17:53:36.000000000 -0500
--- imapd.c 2013-06-09 03:06:37.000000000 -0500
***************
*** 32,37 ****
--- 32,38 ----
#include <signal.h>
#include <setjmp.h>
#include <time.h>
+ #include <limits.h>
#include "c-client.h"
#include "newsrc.h"
#include <sys/stat.h>
***************
*** 938,967 ****
/* wants ALL */
if (retval & 0x4) {
! unsigned long j;
/* find first match */
for (i = 1; (i <= nmsgs) && !mail_elt (stream,i)->searched;
++i);
if (i <= nmsgs) {
PSOUT (" ALL ");
! pnum (uid ? mail_uid (stream,i) : i);
! j = i; /* last message output */
}
! while (++i <= nmsgs) {
! if (mail_elt (stream,i)->searched) {
! while ((++i <= nmsgs) && mail_elt (stream,i)->searched);
! /* previous message is end of range */
! if (j != --i) {
! PBOUT (':');
! pnum (uid ? mail_uid (stream,i) : i);
}
}
/* search for next match */
! while ((++i <= nmsgs) && !mail_elt (stream,i)->searched);
! if (i <= nmsgs) {
! PBOUT (',');
! pnum (uid ? mail_uid (stream,i) : i);
! j = i; /* last message output */
}
}
}
--- 939,996 ----
/* wants ALL */
if (retval & 0x4) {
! unsigned long j,juid,iuid;
/* find first match */
for (i = 1; (i <= nmsgs) && !mail_elt (stream,i)->searched;
++i);
if (i <= nmsgs) {
+ juid= (uid ? mail_uid (stream,i) : i);
PSOUT (" ALL ");
! pnum (juid);
! j = i; /* last message index processed */
}
! if (uid) { /* doing UIDs, no MRC optimization */
! while (++i <= nmsgs) {
! if (mail_elt (stream,i)->searched) { /* this message
matches */
! iuid= mail_uid (stream,i);
! if (juid == (iuid - (i-j))) { /* in sequence of UIDs
*/
! if ( i < nmsgs) continue; /* may be more, check
next */
! PBOUT (':'); pnum(iuid); /* done with all, tie
it off */
! j = i; juid= iuid;
! } else { /* break in sequence of UIDs */
! if ((i-1) > j) { /* prev sequence, tie it off */
! j= i-1; juid= mail_uid (stream,j);
! PBOUT (':'); pnum(juid);
! }
! PBOUT (','); pnum(iuid);
! j = i; juid= iuid; /* record last done message/UID
*/
! }
! } else { /* this message doesn't match, have a pending
sequence? */
! /* if pending sequence, tie it off & reset flags */
! if ((i-1) > j) { /* prev sequence, tie it off */
! j= i-1; juid= mail_uid (stream,j);
! PBOUT (':'); pnum(juid);
! }
! j= nmsgs+1; juid= ULONG_MAX; /* flag that we're not
in a sequence of matches */
}
}
+ } else { /* doing message-IDs, use MRC optimization */
+ while (++i <= nmsgs) {
+ if (mail_elt (stream,i)->searched) {
+ while ((++i <= nmsgs) && mail_elt (stream,i)->searched);
+ /* previous message is end of range */
+ if (j != --i) {
+ PBOUT (':');
+ pnum (i);
+ }
+ }
/* search for next match */
! while ((++i <= nmsgs) && !mail_elt (stream,i)->searched);
! if (i <= nmsgs) {
! PBOUT (',');
! pnum (i);
! j = i; /* last message output */
! }
}
}
}
_______________________________________________
Imap-uw mailing list
[email protected]
http://mailman2.u.washington.edu/mailman/listinfo/imap-uw
