Re: [SM-imapproxy] IMAPTalk: error parsing atom in IMAP response from up-imapproxy

[Paul L. <pa...@sq...>]

Michael, Kirill,

On Mon, Oct 31, 2011 at 11:57 AM, Kirill Miazine <km...@kr...> wrote:
> * Michael M Slusarz [31.10.2011 19:47]:
>>
>> Quoting Kirill Miazine <km...@kr...>:
>>
>>> Perl module Mail::IMAPTalk fails when being greeted by this line by
>>> up-imapproxy (which sits in front of Dovecot):
>>>
>>> * OK [CAPABILITY IMAP4rev1 LITERAL+ LOGIN-REFERRALS ID ENABLE AUTH=LOGIN
>>>  XIMAPPROXY] Dovecot ready.
>>>
>>> This is the error message:
>>>
>>> IMAPTalk: Error parsing atom in IMAP response : ' XIMAPPROXY' at
>>> /local/perl5/lib/perl5/Mail/IMAPTalk.pm line 3483.
>>>
>>> The problem is probably the extra whitespace before the XIMAPPROXY atom.
>>> I modified regexp and line 3390 and add a check for initial white space:
>>>
>>>    # Single item? (and any trailing space)
>>>    if ($Line =~ m/\G*\s**([^()\"{}\s]+)(?: |\z|(?=\)))/gc) {
>>> *^^^
>>> *This is a workaround, but the error is actually on the imapproxy side.
>>> RFC 2060 section 4.4 states this:

RFC 2060 has been obsolete for almost 9 years, but RFC 3501 is
unchanged here. You trimmed out the relevant text:

"Data structures are represented as a "parenthesized list"; a sequence
of data items, delimited by space..."

It makes no specification what is meant by "space" in this context and
if that corresponds to a space character as opposed to more generic
whitespace.  Insofar as the specification is not clear (what's new?),
I don't think it's very safe to write software with hard-coded
expectations of specific character sequences.  It's extremely common
(and much more safe IMHO) to parse whitespace delimited parameters
with something like "\s+" in your regular expression and I'm not sure
why that isn't your solution.

>> This seems appropriate.  The client that I have written is liberal about
>> handling extra whitespaces (in everything other than string data).
>>
>>> Michael Slusarzis the author of XIMAPPROXY patch for imapproxy, and
>>> probably the person who could address this issue in the imapproxy code. It's
>>> probably as easy as removing the line 1081 in the file
>>> http://squirrelmail.svn.sourceforge.net/viewvc/squirrelmail/trunk/imap_proxy/src/main.c?revision=14091&view=markup
>>> <http://squirrelmail.svn.sourceforge.net/viewvc/squirrelmail/trunk/imap_proxy/src/main.c?revision=14091&view=markup>

It is odd that lines 1081 and 1082 both add a space, but I didn't code
that.  I'm hoping Dave or whoever did can speak up, but we won't rush
out and change this too quickly without understanding the
implications, especially when it's commonly accepted that you parse
whitespace without looking for a single space character.

>> I don't maintain the imapproxy code.  You should report this issue to the
>> imapproxy folks.
>
> Their issue tracker at SourceForge didn't have a category for imapproxy. So
> I copy Paul Lesniewski from the SquirrelMail team on this mail.

I cc:'d this to the imapproxy mailing list and by all means, feel free
to tell us why you think your parsing is the better way and how my
interpretation is all wrong.  :)

Thanks for the heads-up.

-- 
Paul Lesniewski
SquirrelMail Team
Please support Open Source Software by donating to SquirrelMail!
http://squirrelmail.org/donate_paul_lesniewski.php