Re: search problem using white spaces...

On Sat, Dec 12, 2009 at 12:22 AM, Paul Lesniewski <[email protected]> wrote:
> On Sat, Dec 12, 2009 at 12:20 AM, Paul Lesniewski <[email protected]> wrote:
>> On Sat, Dec 12, 2009 at 12:15 AM, Paul Lesniewski <[email protected]> wrote:
>>> On Fri, Dec 11, 2009 at 11:55 PM, Paul Lesniewski <[email protected]> wrote:
>>>> On Fri, Dec 11, 2009 at 3:12 PM, Paul Lesniewski <[email protected]> wrote:
>>>>> Please do not top-post.  I believe you should know better.
>>>>>
>>>>>>> I'm using SM1.4.20rc2 and cyrus.
>>>>>>>
>>>>>>> Whenever I search for something let's say in the "From" header, using a
>>>>>>> word I have no problem, cyrus finds it inmedeatily.
>>>>>>> However, I a use two (or more) words, it takes a long time and I get:
>>>>>>> Connection terminated by IMAP server, "Query: SEARCH CHARSET ISO-8859-1 ALL
>>>>>>> FROM".
>>>>>>>
>>>>>>> I have disabled, charset search... same problem. If I do the search using
>>>>>>> telnet I have no problem either.
>>>>>>>
>>>>>>> For instance, searching "javier wilson": A06 UID SEARCH CHARSET
>>>>>>> "ISO-8859-1" ALL FROM "javier wilson"
>>>>>>> works fine using telnet. It does not work using Squirrelmail.
>>>>>>>
>>>>>>> Any ideas, anyone with a similar problem?
>>>>>>> Could it be because of " $multi_search = explode(' ', $search_what); "  in
>>>>>>> functions/imap_search.php ?
>>>>>>
>>>>>> I corrected the problem changing line 38 of functions/imap_search.php:
>>>>>>
>>>>>> < $multi_search = explode(' ', $search_what);
>>>>>>> $multi_search = array( $search_what );
>>>>>>
>>>>>> Of course this doesn't search "javier" *and* "wilson", but "javier wilson",
>>>>>> which is what I intended anyway.
>>>>>>
>>>>>> In summary, I would say multi_search is broken in SM1.4.20rc2
>>>>>
>>>>> Confirmed.  With search terms other than single words, I get:
>>>>>
>>>>> ERROR: Could not complete request.
>>>>> Query: FETCH (FLAGS UID RFC822.SIZE INTERNALDATE
>>>>> BODY.PEEK[HEADER.FIELDS (Date To Cc From Subject X-Priority Importance
>>>>> Priority Content-Type)])
>>>>> Reason Given: Error in IMAP command received by server.
>>>>
>>>> The problem seems to be that the SEARCH command is sending each word
>>>> as a separate string after the search key... something like:
>>>>
>>>> A01 SEARCH CHARSET ISO-8859-1 ALL FROM {6}
>>>> javier {6}
>>>> wilson
>>>>
>>>> That's the same as:
>>>>
>>>> A01 SEARCH CHARSET ISO-8859-1 ALL FROM javier wilson
>>>>
>>>> Which doesn't match RFC as far as I can tell.  The SEARCH command
>>>> accepts a single string argument for keys like FROM.  The examples
>>>> above are sending two strings, not one.  You either have to quote the
>>>> two strings (which makes the search different than we've had it before
>>>> wherein the whole string "javier wilson" is searched for as is.  I
>>>> think to search for the intersect of "javier" and "wilson", you have
>>>> to specify the key for each string, i.e.,
>>>>
>>>> A01 SEARCH CHARSET ISO-8859-1 ALL FROM {6}
>>>> javier FROM {6}
>>>> wilson
>>>>
>>>> Or this:
>>>>
>>>> A01 SEARCH CHARSET ISO-8859-1 ALL FROM javier FROM wilson
>>>>
>>>> My tests on the command line bear this out.
>>>
>>> Here is a patch that removes the use of literals (as it appears to me
>>> it didn't need to be added in the first place) and corrects this
>>> problem per my last message.  Since the patch is unnecessarily long
>>> due to reshuffling some comments and the resulting code is actually
>>> rather simple, here is what the code ends up looking like:
>>>
>>>    /* construct the search query, taking multiple search terms into account */
>>>    $multi_search = array();
>>>    $search_what  = trim($search_what);
>>>    $search_what  = preg_replace('/[ ]{2,}/', ' ', $search_what);
>>>    $multi_search = explode(' ', $search_what);
>>>    $search_string = '';
>>>
>>>    if (strtoupper($languages[$squirrelmail_language]['CHARSET'] ==
>>> 'ISO-2022-JP')) {
>>>        foreach($multi_search as $idx=>$search_part) {
>>>            $multi_search[$idx] = mb_convert_encoding($search_parth,
>>> 'JIS', 'auto');
>>>        }
>>>    }
>>>
>>>
>>>    /* it seems macosx and hmailserver do not support the prefered search
>>>       syntax so we fall back to the older style. This IMAP
>>>       server has a problem with multiple search terms. Instead
>>>       of returning the messages that match all the terms it
>>>       returns the messages that match each term. Could be fixed
>>>       on the client side, but should be fixed on the server
>>>       as per the RFC */
>>>
>>>    if ($imap_server_type == 'macosx' || $imap_server_type == 'hmailserver') {
>>>        $search_string .= $search_where . ' ' . implode(' ', $multi_search);
>>>    }
>>>    else {
>>>        foreach ($multi_search as $string)
>>>           $search_string .= $search_where . ' "' . $string . '"';
>>>    }
>>>
>>>    $search_string = trim($search_string);
>>>
>>>    /* now use $search_string in the imap search */
>>>    if ($allow_charset_search &&
>>> isset($languages[$squirrelmail_language]['CHARSET']) &&
>>>        $languages[$squirrelmail_language]['CHARSET']) {
>>>        $ss = "SEARCH CHARSET "
>>>            . strtoupper($languages[$squirrelmail_language]['CHARSET'])
>>>            . " ALL $search_string";
>>>    } else {
>>>        $ss = "SEARCH ALL $search_string";
>>>    }
>>>
>>>    /* read data back from IMAP */
>>>    $readin = sqimap_run_command($imapConnection, $ss, false, $result,
>>> $message, $uid_support);
>>
>> I am willing to bet that the supposed problems with hmailserver and
>> macosx were never the fault of those servers at all, but a poor
>> understanding of the RFC and maybe the fact that some other servers
>> were rather relaxed in what they accepted as "strings" for such
>> commands.  I can't test on these two servers, but I think we can just
>> remove the (RFC-invalid) code for them anyway.  The code becomes even
>> more simple.  See below or the new patch I have attached.  Please
>> comment before I make this change.
>>
>>    /* construct the search query, taking multiple search terms into account */
>>    $multi_search = array();
>>    $search_what  = trim($search_what);
>>    $search_what  = preg_replace('/[ ]{2,}/', ' ', $search_what);
>>    $multi_search = explode(' ', $search_what);
>>    $search_string = '';
>>
>>    if (strtoupper($languages[$squirrelmail_language]['CHARSET'] ==
>> 'ISO-2022-JP')) {
>>        foreach($multi_search as $idx=>$search_part) {
>>            $multi_search[$idx] = mb_convert_encoding($search_parth,
>> 'JIS', 'auto');
>>        }
>>    }
>>
>>    foreach ($multi_search as $string) {
>>       $search_string .= $search_where . ' "' . $string . '"';
>>    }
>
> This probably needs quote-escaping....

That's the cause of the problems I think.  The original author appears
to have chosen literal strings instead of worrying about escapes.
Fair enough, but the SM library for sending commands didn't know how
to wait for the IMAP server continuation response after each literal.
The literal+ extension probably helps in that regard, but we're trying
to support the widest possible variety of servers.

Here's the code before the most recent changes:

http://squirrelmail.svn.sourceforge.net/viewvc/squirrelmail/branches/SM-1_4-STABLE/squirrelmail/functions/imap_search.php?revision=13800&view=markup

The recent change to the code tried to add that (continuation
response) support, but it's broken because it is missing the SEARCH
key before all string arguments except the first one.

The hmailserver and macosx incompatibilities might be more about SM's
poor literal support.  I'm surprised it worked with any servers at
all.  The author's mumbo jumbo about RFC is a little misguided as far
as I can see... the "fix" for these servers doesn't even quote the
strings as it should.

So the two choices we have are:

1) use my patch (my most recent patch didn't get through as promised,
so it is attached here), but we'd need to add quote escaping

2) use the literal stuff, but it's really not written to spec -- at a
minimum, it needs the ability to inject other parts of the IMAP
command after each literal.  The better way to write literal support
into SM is to actually follow RFC - literals are just strings that are
sent in two parts, between which an IMAP response has to be accounted
for.

>>    $search_string = trim($search_string);
>>
>>    /* now use $search_string in the imap search */
>>    if ($allow_charset_search &&
>> isset($languages[$squirrelmail_language]['CHARSET']) &&
>>        $languages[$squirrelmail_language]['CHARSET']) {
>>        $ss = "SEARCH CHARSET "
>>            . strtoupper($languages[$squirrelmail_language]['CHARSET'])
>>            . " ALL $search_string";
>>    } else {
>>        $ss = "SEARCH ALL $search_string";
>>    }
>>
>>    /* read data back from IMAP */
>>    $readin = sqimap_run_command($imapConnection, $ss, false, $result,
>> $message, $uid_support);

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

------------------------------------------------------------------------------
Return on Information:
Google Enterprise Search pays you back
Get the facts.
http://p.sf.net/sfu/google-dev2dev

-----
squirrelmail-devel mailing list
Posting guidelines: http://squirrelmail.org/postingguidelines
List address: [email protected]
List archives: http://news.gmane.org/gmane.mail.squirrelmail.devel
List info (subscribe/unsubscribe/change options): https://lists.sourceforge.net/lists/listinfo/squirrelmail-devel

Index: functions/imap_search.php
===================================================================
--- functions/imap_search.php   (revision 13876)
+++ functions/imap_search.php   (working copy)
@@ -38,32 +38,15 @@
     $multi_search = explode(' ', $search_what);
     $search_string = '';
 
-    /* it seems macosx and hmailserver do not support the prefered search
-       syntax so we fall back to the older style. This IMAP
-       server has a problem with multiple search terms. Instead
-       of returning the messages that match all the terms it
-       returns the messages that match each term. Could be fixed
-       on the client side, but should be fixed on the server
-       as per the RFC */
-
     if (strtoupper($languages[$squirrelmail_language]['CHARSET'] == 'ISO-2022-JP')) {
         foreach($multi_search as $idx=>$search_part) {
             $multi_search[$idx] = mb_convert_encoding($search_parth, 'JIS', 'auto');
         }
     }
 
-    $search_lit = array();
-
-    if ($imap_server_type == 'macosx' || $imap_server_type == 'hmailserver') {
-        $search_string .= $search_where . ' ' . implode(' ', $multi_search);
+    foreach ($multi_search as $string) {
+       $search_string .= $search_where . ' "' . $string . '"';
     }
-    else {
-        $search_string .= $search_where;
-        $search_lit = array(
-                    'command' => '',
-                    'literal_args' => $multi_search
-                );
-    }
 
     $search_string = trim($search_string);
 
@@ -77,13 +60,8 @@
         $ss = "SEARCH ALL $search_string";
     }
 
-    if (empty($search_lit)) {
-        /* read data back from IMAP */
-        $readin = sqimap_run_command($imapConnection, $ss, false, $result, $message, $uid_support);
-    } else {
-        $search_lit['command'] = $ss;
-        $readin = sqimap_run_literal_command($imapConnection, $search_lit, false, $result, $message, $uid_support);
-    }
+    /* read data back from IMAP */
+    $readin = sqimap_run_command($imapConnection, $ss, false, $result, $message, $uid_support);
 
     /* try US-ASCII charset if search fails */
     if (isset($languages[$squirrelmail_language]['CHARSET'])