Re: [thomas@webmail0.xs4all.nl: fastsquirrel.patch3]

[Thomas W. <th...@xs...>]

On Fri, Jun 22, 2001 at 09:16:42AM -0500, Luke Ehresman wrote:
> > Here the diffs for turbo-squirrelmail :) They're against a 2 week old
> > CVS, so i hope we're not too far out of sync with CVS..

> GREAT!  I just successfully applied the patch and am currently testing it
> before I commit it to our CVS.  I did find a typo that you probably will
> want to fix in your version as well... noted below.

> On or around line 62 of imap_general.php:
>     $break;
> should be
>     break;

> Looks good though..

Thanx. (I'm the guy that wrote it.) I should point out a couple of things
though:

 - It's a hack, but less of a hack than the old way. It's a hack because it
still doesn't really parse the responses the IMAP server sends back, but
rather guesses at them. This will probably work with all IMAP4rev1 compliant
IMAP servers, though. 

 - The response-sorting routines in sqimap_get_small_header_list() and
sqimap_get_flags_list() do not take into account that a response might be
smaller than the original request. That is, if passed a list like

   1,3,2,1

some IMAP servers will respond exactly three times, once for each message,
in unspecified order, while others might respond four times, twice for
message one, in unspecified order. Fortunately, no piece of code actually
does this, as showMessagesForMailbox() only deals in ranges, and
sqimap_search() only deals in responses from the IMAP server... I hope it's
safe to assume no IMAP server is so broken that they return duplicates in
SEARCH responses, but collapse duplicates in FETCH responses.

 - The code could use some refactoring and sanitizing. Especially
sqimap_search(), which basically contains showMessagesForMailbox verbatim,
minus the restrict-number-of-messages branch (which my patch reduced to a few
lines of code.) But I gather from Cor that the main focus currently is not
refactoring and sanitizing current code ;)

- We got one bugreport from a user using the Spamcop plugin, who had a
mixed-up mailbox overview after he submitted a spamreport. I suspect this is
a caching bug, not specific to this patch though, but I haven't looked at it
enough yet.

-- 
Thomas Wouters <th...@xs...>

Hi! I'm a .signature virus! copy me into your .signature file to help me spread!