Re: noselect_fix_enable incomplete?

Hi Dave,

> I discovered that Squirrelmail 1.4.22 with $noselect_fix_enable set does not
> seem to be a complete fix, at least in a Cyrus 2.4.x environment. I'm not
> asserting that it would be a complete solution for earlier Cyrus versions
> (or for other IMAP servers, for that matter) but I've only tested it in a
> Cyrus 2.4.x environment.
>
> What I observed is that even with $noselect_fix_enable set to true, I had
> hierarchy levels (not mailboxes) being rendered in left_main.php as
> clickable links.  If a user clicks on one of these links, SquirrelMail
> issues a SELECT to the IMAP server, and the server returns an error because
> NoSelect is set for that hierarchy level.  I also discovered a similar issue
> when selecting "All Folders" from the search page. SquirrelMail would
> eventually attempt to SELECT a hierarchy level to search it and an error
> would be returned.
>
> The code at line 610 in functions/imap_mailbox.php that conditionally
> constructs the LSUB does the correct thing:
>
>        if ($noselect_fix_enable) {
>            $lsub_args = "LSUB \"$folder_prefix\" \"*%\"";
>        } else {
>            $lsub_args = "LSUB \"$folder_prefix\" \"*\"";
>        }
>
> However, even if the NoSelect flag is returned in the LSUB response, it's
> thrown away when find_mailbox_name() is called at line 633.  No later code
> expects to be able to glean NoSelect status from this LSUB response, so this
> isn't a problem.  Instead, SquirrelMail calls LIST for each subscribed
> mailbox at line 662 to get all the mailbox flags.  The problem I'm seeing is
> that the LIST isn't constructed similarly to the above LSUB and no data is
> returned at all for things like hierarchy levels that aren't mailboxes.
>  Here's a protocol example using imtest against the cmu.misc level of
> hierarchy on our IMAP server:
>
> 1 LIST "" "cmu.misc"
> 1 OK Completed (0.000 secs 30 calls)
> 2 LIST "" "cmu.misc%"
> * LIST (\Noselect \NonExistent \HasChildren) "." cmu.misc
> 2 OK Completed (0.000 secs 30 calls)
>
> Attached is a patch that completes the $noselect_fix_enable fix in our
> environment.  Can anyone else confirm whether or not $noselect_fix_enable
> works in different environments than ours?

Thanks a lot for the thorough inquiry.  It looks to me like this was
unintentional, and you can in fact confirm that this was addressed in
a similar manner in version 1.5.2.  It's not clear if the code works
for some people as-is, but I have my doubts (the spec and various
implementations seem all too brittle here).

While we're here, though, it occurs that doing one LIST per folder is
egregious.  Again, version 1.5.2 fixes this, too, and my tests show
that the folder listing gains 11 to 12 times the performance by doing
just a single LIST.

So I'm prepared to add a slightly different patch, which is attached
to this message.  Note that it depends on another fix I've added to
SVN just now which was a bug (and for you, Dave, you'll have to remove
your patch as well).

Here's the fix I just made:

http://squirrelmail.svn.sourceforge.net/viewvc/squirrelmail/branches/SM-1_4-STABLE/squirrelmail/functions/imap_mailbox.php?view=patch&r1=14175&r2=14174&pathrev=14175

Apply that, then the patch attached to this message and let me know if
this works for you.

Thanks again,

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

------------------------------------------------------------------------------
Cloud Services Checklist: Pricing and Packaging Optimization
This white paper is intended to serve as a reference, checklist and point of 
discussion for anyone considering optimizing the pricing and packaging model 
of a cloud services business. Read Now!
http://www.accelacomm.com/jaw/sfnl/114/51491232/

-----
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_mailbox.php
===================================================================
--- functions/imap_mailbox.php	(revision 14175)
+++ functions/imap_mailbox.php	(working copy)
@@ -650,31 +650,34 @@
            * Note: according RFC2060 an imap server may provide \NoSelect flags in the LSUB response.
            * in other words, we cannot rely on it.
          */
+        if ($noselect_fix_enable) {
+            $list_args = "LIST \"$folder_prefix\" \"*%\"";
+        } else {
+            $list_args = "LIST \"$folder_prefix\" \"*\"";
+        }
+        /* LIST array */
+        $list_ary = sqimap_run_command ($imap_stream, $list_args,
+                                        true, $response, $message);
         $sorted_list_ary = array();
-        for ($i=0; $i < count($sorted_lsub_ary); $i++) {
-            if (substr($sorted_lsub_ary[$i], -1) == $delimiter) {
-                $mbx = substr($sorted_lsub_ary[$i], 0, strlen($sorted_lsub_ary[$i])-1);
-            }
-            else {
-                $mbx = $sorted_lsub_ary[$i];
-            }
-
-            $read = sqimap_run_command ($imap_stream, "LIST \"\" \"$mbx\"",
-                                        true, $response, $message);
-
-            /* Another workaround for literals */
-
-            if (isset($read[1]) && substr($read[0],-3) == "}\r\n") {
+        for ($i = 0, $cnt = count($list_ary);$i < $cnt; $i++) {
+            /*
+             * Workaround for mailboxes returned as literal
+             * Doesn't work if the mailbox name is multiple lines
+             * (larger then fgets buffer)
+             */
+            if (isset($list_ary[$i + 1]) && substr($list_ary[$i],-3) == "}\r\n") {
                 if (preg_match('/^(\* [A-Z]+.*)\{[0-9]+\}([ \n\r\t]*)$/',
-                     $read[0], $regs)) {
-                    $read[0] = $regs[1] . '"' . addslashes(trim($read[1])) . '"' . $regs[2];
+                     $list_ary[$i], $regs)) {
+                        $i++;
+                        $list_ary[$i] = $regs[1] . '"' . addslashes(trim($list_ary[$i])) . '"' . $regs[2];
                 }
             }
+            $temp_mailbox_name = find_mailbox_name($list_ary[$i]);
 
-            if (isset($read[0])) {
-                $sorted_list_ary[$i] = $read[0];
-            } else {
-                $sorted_list_ary[$i] = '';
+            // is_numeric() because before PHP 4.2.0, bad return
+            // value is NULL; whereas in newer versions, it's FALSE
+            if (is_numeric($key = array_search($temp_mailbox_name, $sorted_lsub_ary))) {
+                $sorted_list_ary[$key] = $list_ary[$i];
             }
         }