[imp] PATCH: IMP_ACL_rfc2086 - things I learnt yesterday!

Chris Hastie lists at oak-wood.co.uk
Wed Feb 5 11:20:09 PST 2003


Attached is a patch to imp/lib/ACL/rfc2086.php that deals with various
issues that came up yesterday, along with a small patch to imp/acl.php.

*   Warnings include more detailed information about what the problem
    was. Hopefully this will make further debugging easier.

*   canEdit() always returns true as we can't actually establish if a
    user is able to edit an ACL or not since we know nothing about which
    groups s/he's in. Leave the decision to the server. The patch to
    imp/acl.php corrects handling of the server's response.

*   better handling of passwords containing linear white space, '"',
    ''', or '/'  when using plaintext logins.

*   correct bug introduced by the last cvs update which always claims
    the server doesn't support ACL (patch from Didi Rieder)

I had prepared a patch with a little bit more attention to
docs/CODING_STANDARDS but hadn't noticed that Chuck had already cleaned
the code up. This is a steep learning curve for me, but I am trying,
honest ("yes, very trying indeed" you cry:) ) so please bear with me. I
have a couple of questions on this:

1   Line length. If I wrap lines, as I have with the warnings in this
    patch, should I indent the wrapped part (which is what I've gone
    with) or line it up with first section?

2   Why is it preferable to get the values of variables in a class by
    calling functions that return them rather than simply looking at the
    variable? Is it a question of preferred style or is there some
    intrinsic problem with just looking at variables?
-- 
Chris Hastie
-------------- next part --------------
--- imp/lib/ACL/rfc2086.php,v 1.2
+++ imp/lib/ACL/rfc2086.php	Wed Feb  5 10:06:37 2003
@@ -44,9 +44,9 @@
         // If we couldn't get the server's capability, we'll assume
         // ACL is supported for now.
         if (!empty($this->_caps) && !array_key_exists('acl', $this->_caps)) {
-            $this->_isSupported = false;
+            $this->_supported = false;
         } else {
-            $this->_isSupported = true;
+            $this->_supported = true;
         }
 
         $this->_protected = array($_SESSION['imp']['user']);
@@ -155,7 +155,8 @@
             if (version_compare(phpversion(), '4.3.0') >= 0) {
                 $server = $sub_proto . '://' . $server;
             } else {
-                $notification->push(_("Could not retrieve server's capabilities"), 'horde.warning');
+                $notification->push(_("Could not retrieve server's capabilities") 
+                    . " - " . _("SSL connections require PHP 4.3 or better"), 'horde.warning');
                 return null;
             }
         }
@@ -163,7 +164,8 @@
         $imap = fsockopen($server, $_SESSION['imp']['port'], $errno, $errstr, 30);
 
         if (!$imap) {
-            $notification->push(_("Could not retrieve server's capabilities"), 'horde.warning');
+            $notification->push(_("Could not retrieve server's capabilities") 
+                . " - " . _("Connection failed: ") . $errno . " : " . $errstr, 'horde.warning');
             return null;
         } else {
             $response = fgets($imap, 4096);
@@ -182,10 +184,12 @@
                     }
 
                 } else {
-                    $notification->push(_("Could not retrieve server's capabilities"), 'horde.warning');
+                    $notification->push(_("Could not retrieve server's capabilities") 
+                        . " - " . _("Unexpected response from server to: ") . "'x CAPABILITY' : " . $response, 'horde.warning');
                 }
             } else {
-                $notification->push(_("Could not retrieve server's capabilities"), 'horde.warning');
+                $notification->push(_("Could not retrieve server's capabilities") 
+                    . " - " . _("Unexpected response from server on connection: ") . $response, 'horde.warning');
             }
             fclose ($imap);
         }
@@ -270,7 +274,8 @@
             if (version_compare(phpversion(), '4.3.0') >= 0) {
                 $server = $sub_proto . '://' . $server;
             } else {
-                $notification->push(_("Could not retrieve ACL"), 'horde.warning');
+                $notification->push(_("Could not retrieve ACL") 
+                    . " - " . _("SSL connections require PHP 4.3 or better"), 'horde.warning');
                 return null;
             }
         }
@@ -284,7 +289,8 @@
         $imap = fsockopen($server, $_SESSION['imp']['port'], $errno, $errstr, 30);
 
         if (!$imap) {
-            $notification->push(_("Could not retrieve ACL"), 'horde.warning');
+            $notification->push(_("Could not retrieve ACL")
+                . " - " . _("Connection failed: ") . $errno." : " . $errstr, 'horde.warning');
             return null;
         } else {
             $response = fgets($imap, 4096);
@@ -314,12 +320,17 @@
                     $response = explode (" ", trim(fgets ($imap,1024)));
                     $response = base64_decode($response[1]);
                     if (!preg_match("/rspauth=/", $response)) {
-                        $notification->push(_("Could not retrieve ACL"), 'horde.warning');
+                        $notification->push(_("Could not retrieve ACL") 
+                            . " - " . _("Unexpected response from server to: ") . "Digest-MD5 response", 'horde.warning');
                         return null;
                     }
                     fputs($imap, "\r\n");
 
                 } else {
+                    if (preg_match('/\W/',  $pass)) {
+                        $pass = addslashes($pass);
+                        $pass = '"' . $pass . '"';
+                    }                          
                     fputs ($imap, "$txid LOGIN " . $_SESSION['imp']['user'] . " " . $pass . "\r\n");
 
                 }
@@ -346,13 +357,16 @@
                             }
                         }
                     } else {
-                        $notification->push(_("Could not retrieve ACL"), 'horde.warning');
+                        $notification->push(_("Could not retrieve ACL") 
+                            . " - " . _("Unexpected response from server to: ") . "'$txid GETACL' : " .$response, 'horde.warning');
                     }
                 } else {
-                    $notification->push(_("Could not retrieve ACL"), 'horde.warning');
+                    $notification->push(_("Could not retrieve ACL") 
+                        . " - " . _("Unexpected response from server to: ") . "login : " . $response, 'horde.warning');
                 }
             } else {
-                $notification->push(_("Could not retrieve ACL"), 'horde.warning');
+                $notification->push(_("Could not retrieve ACL") 
+                    . " - " . _("Unexpected response from server on connection: ") . $response, 'horde.warning');
             }
             fclose($imap);
         }   
@@ -369,17 +383,11 @@
      *
      * @returns boolean  True if $user has 'a' right or $acl is empty
      */
-    function canEdit($acl, $user)
-    {
-        /* Possible for getACL to fail but createACL still to work 
-           (eg ssl, PHP <4.3), so return true if $acl is empty */
-        if (empty($acl)) {
-            return true;
-        } elseif (array_key_exists('a', $acl[$user]) && $acl[$user]['a'] > 0) {
-            return true;
-        } else {
-            return false;
-        }
+    function canEdit($acl, $user) {
+        /* We can't establish if the user is in a group with the 
+           'a' privilege, so just return true and leave the decision
+           to the server */   
+        return true;
     }
 
     /**
-------------- next part --------------
--- imp/acl.php,v 1.4
+++ imp/acl.php	Wed Feb  5 09:45:04 2003
@@ -64,7 +64,7 @@
 
     if ($ok_form) {
         $result = $ACLDriver->createACL($folder, $share_user, $acl);
-        if ($result == 'no_support') {
+        if ($result === 'no_support') {
             header('Location: ' . Horde::applicationUrl('prefs.php', true));
             exit;
         }
@@ -87,7 +87,7 @@
 
     if ($ok_form) {
         $result = $ACLDriver->editACL($folder, $share_user, $acl);
-        if ($result == 'no_support') {
+        if ($result === 'no_support') {
             header('Location: ' . Horde::applicationUrl('prefs.php', true));
             exit;
         }


More information about the imp mailing list