[Twisted-Python] Re: IMAP fixes

[Jp Calderone]

On Mon, Jul 07, 2003 at 03:57:45PM +1200, Tony Meyer wrote:
> Regarding the new parameter parsing for the IMAP server:
> 
> At the moment, FETCH args are parsed with ParseNestedList, which means that
> any items inside [] are appended as an additional list.
> 
> For example, this command:
>     0003 FETCH 1 BODY.PEEK[HEADER.FIELDS (References X-Ref)] UID
> Becomes:
>     ['BODY.PEEK', ['HEADER.FIELDS', ['References', 'X-Ref']], UID]
> 
> There are really two items that have been requested - BODY.PEEK and UID, but
> the args list has three, because the BODY.PEEK information has been split
> into a separate item.

  Ideally, I'd like to see this parsed into a structure resembling this:

    [Header(peek=1, name=1, value=1, ['References', 'X-Ref']), UID()]

  The mailbox implementation should have as little knowledge of IMAP4 as
humanly possible.  The current implementation is an example of the "just
enough to work" philosophy.  parseNestedList was actually hacked pretty
awfully to support this after I initially wrote it, because I'd forgotten
about this particular aspect of the IMAP4 query syntax.

> 
> It seems to me that it would make more sense for the result to be:
>     [['BODY.PEEK', ['HEADER.FIELDS', ['References', 'X-Ref']]], UID]
> Or something like that, which keeps the body.peek information together.
> (Note that BODY and BODY.PEEK are the only two FETCH args that have a []
> part.)
> 

  It does seem like a slight improvement, but I'm not sure it is significant
enough to merit a stop in the API roadmap.  On the other hand, if you're
willing to implement it... ;)


> This is both closer to what was requested, and much easier to parse (IMO).

  Parsing the current form isn't so bad.  Here's a snippet from the mailbox
I've written:

        i = 0
        r = {}
        L = len(parts)
        while i < L:
            p = parts[i]
            fn = p.upper().replace('.', '_')
            f = getattr(self, '_fetch_' + fn, None)
            if not f:
                raise NotImplementedError, fn
            if i + 1 < L:
                if isinstance(parts[i + 1], types.ListType):
                    r.update(f(msg, parts[i + 1]))
                    i += 2
                    continue
            r.update(f(msg))
            i += 1
        return r

  The key section of interest here is the isinstance() check (the loop
dispatches actual fetching to methods named like "_fetch_BODY_PEEK" and so
on).  I believe parsing an alternate form that continued to intersperse
lists with strings would be pretty similar, with just a few checks
re-arranged.

  Would you be interested in patching the fetch methods to generate a
structure like the one I described above?  Or if you have another idea for a
parsed query structure that is similarly abstracted away from the protocol,
I'd be glad to entertain it.

  Lastly, I definitely recognize this as a weak point in the IMAP4 code, but
fixing it has not yet become a priority, as I already have a working parser.

  Jp

-- 
A disciple of another sect once came to Drescher as he was eating his
morning meal. "I would like to give you this personality test," said the
outsider, "because I want you to be happy." Drescher took the paper that was
offered him and put it into the toaster: "I wish the toaster to be happy, 
too."