Removed html formating from address book backend classes. Added

fullname() method to addressbook_backend class. Moved
htmlspecialchars() sanitizing from address book backend classes to
html output code. If third party code displays errors from address
book object in html, errors must be sanitized and ASCII line feeds
should be converted to html line breaks.

'addressbook' strings are replaced with 'address book'. Spellcheck
fails on 'addressbook'.

ChangeLog

@@ -72,6 +72,12 @@ Version 1.5.2 - CVS
   - Added write support to address book LDAP backend. Patch by David 
     Hardeman (#1495763).
   - Added message copy options.
+  - Removed html formating from address book backend classes. Added
+    fullname() method to addressbook_backend class. Moved
+    htmlspecialchars() sanitizing from address book backend classes to
+    html output code. If third party code displays errors from address
+    book object in html, errors must be sanitized and ASCII line feeds
+    should be converted to html line breaks.
 
 Version 1.5.1 (branched on 2006-02-12)
 --------------------------------------

functions/abook_database.php

@@ -112,9 +112,9 @@ class abook_database extends addressbook_backend {
         /* test if Pear DB class is available and freak out if it is not */
         if (! class_exists('DB')) {
             // same error also in db_prefs.php
-            $error  = _("Could not include PEAR database functions required for the database backend.") . "<br />\n";
+            $error  = _("Could not include PEAR database functions required for the database backend.") . "\n";
             $error .= sprintf(_("Is PEAR installed, and is the include path set correctly to find %s?"),
-                              '<tt>DB.php</tt>') . "<br />\n";
+                              'DB.php') . "\n";
             $error .= _("Please contact your system administrator and report this error.");
             return $this->set_error($error);
         }
@@ -248,7 +248,7 @@ class abook_database extends addressbook_backend {
 
         while ($row = $res->fetchRow(DB_FETCHMODE_ASSOC)) {
             array_push($ret, array('nickname'  => $row['nickname'],
-                                   'name'      => "$row[firstname] $row[lastname]",
+                                   'name'      => $this->fullname($row['firstname'], $row['lastname']),
                                    'firstname' => $row['firstname'],
                                    'lastname'  => $row['lastname'],
                                    'email'     => $row['email'],
@@ -287,7 +287,7 @@ class abook_database extends addressbook_backend {
 
         if ($row = $res->fetchRow(DB_FETCHMODE_ASSOC)) {
             return array('nickname'  => $row['nickname'],
-                         'name'      => "$row[firstname] $row[lastname]",
+                         'name'      => $this->fullname($row['firstname'], $row['lastname']),
                          'firstname' => $row['firstname'],
                          'lastname'  => $row['lastname'],
                          'email'     => $row['email'],
@@ -325,7 +325,7 @@ class abook_database extends addressbook_backend {
 
         while ($row = $res->fetchRow(DB_FETCHMODE_ASSOC)) {
             array_push($ret, array('nickname'  => $row['nickname'],
-                                   'name'      => "$row[firstname] $row[lastname]",
+                                   'name'      => $this->fullname($row['firstname'], $row['lastname']),
                                    'firstname' => $row['firstname'],
                                    'lastname'  => $row['lastname'],
                                    'email'     => $row['email'],
@@ -343,7 +343,7 @@ class abook_database extends addressbook_backend {
      */
     function add($userdata) {
         if (!$this->writeable) {
-            return $this->set_error(_("Addressbook is read-only"));
+            return $this->set_error(_("Address book is read-only"));
         }
 
         if (!$this->open()) {
@@ -386,7 +386,7 @@ class abook_database extends addressbook_backend {
      */
     function remove($alias) {
         if (!$this->writeable) {
-            return $this->set_error(_("Addressbook is read-only"));
+            return $this->set_error(_("Address book is read-only"));
         }
 
         if (!$this->open()) {
@@ -424,7 +424,7 @@ class abook_database extends addressbook_backend {
      */
     function modify($alias, $userdata) {
         if (!$this->writeable) {
-            return $this->set_error(_("Addressbook is read-only"));
+            return $this->set_error(_("Address book is read-only"));
         }
 
         if (!$this->open()) {

functions/abook_ldap_server.php

@@ -475,8 +475,7 @@ class abook_ldap_server extends addressbook_backend {
                 $surname = trim($this->charset_decode($row['sn'][0]));
             }
 
-            // FIXME: Write generic function to handle name order 
-            $fullname = trim($firstname . " " . $surname);
+            $fullname = $this->fullname($firstname,$surname);
 
             /* Add one row to result for each e-mail address */
             if(isset($row['mail']['count'])) {

functions/abook_local_file.php

@@ -283,7 +283,7 @@ class abook_local_file extends addressbook_backend {
             // errors on eregi call are suppressed in order to prevent display of regexp compilation errors
             if(@eregi($expr, $line)) {
                 array_push($res, array('nickname'  => $row[0],
-                    'name'      => $row[1] . ' ' . $row[2],
+                    'name'      => $this->fullname($row[1], $row[2]),
                     'firstname' => $row[1],
                     'lastname'  => $row[2],
                     'email'     => $row[3],
@@ -314,7 +314,7 @@ class abook_local_file extends addressbook_backend {
         while ($row = @fgetcsv($this->filehandle, 2048, '|')) {
             if(strtolower($row[0]) == $alias) {
                 return array('nickname'  => $row[0],
-                  'name'      => $row[1] . ' ' . $row[2],
+                  'name'      => $this->fullname($row[1], $row[2]),
                   'firstname' => $row[1],
                   'lastname'  => $row[2],
                   'email'     => $row[3],
@@ -343,7 +343,7 @@ class abook_local_file extends addressbook_backend {
 
         while ($row = @fgetcsv($this->filehandle, 2048, '|')) {
             array_push($res, array('nickname'  => $row[0],
-                'name'      => $row[1] . ' ' . $row[2],
+                'name'      => $this->fullname($row[1], $row[2]),
                 'firstname' => $row[1],
                 'lastname'  => $row[2],
                 'email'     => $row[3],
@@ -361,7 +361,7 @@ class abook_local_file extends addressbook_backend {
      */
     function add($userdata) {
         if(!$this->writeable) {
-            return $this->set_error(_("Addressbook is read-only"));
+            return $this->set_error(_("Address book is read-only"));
         }
         /* See if user exists already */
         $ret = $this->lookup($userdata['nickname']);
@@ -385,7 +385,7 @@ class abook_local_file extends addressbook_backend {
         /* Reopen file, just to be sure */
         $this->open(true);
         if(!$this->writeable) {
-            return $this->set_error(_("Addressbook is read-only"));
+            return $this->set_error(_("Address book is read-only"));
         }
 
         /* Lock the file */
@@ -402,7 +402,7 @@ class abook_local_file extends addressbook_backend {
         /* Test write result */
         if($r === FALSE) {
             /* Fail */
-            $this->set_error(_("Write to addressbook failed"));
+            $this->set_error(_("Write to address book failed"));
             return FALSE;
         }
 
@@ -416,7 +416,7 @@ class abook_local_file extends addressbook_backend {
      */
     function remove($alias) {
         if(!$this->writeable) {
-            return $this->set_error(_("Addressbook is read-only"));
+            return $this->set_error(_("Address book is read-only"));
         }
 
         /* Lock the file to make sure we're the only process working
@@ -453,7 +453,7 @@ class abook_local_file extends addressbook_backend {
      */
     function modify($alias, $userdata) {
         if(!$this->writeable) {
-            return $this->set_error(_("Addressbook is read-only"));
+            return $this->set_error(_("Address book is read-only"));
         }
 
         /* See if user exists */

functions/addressbook.php

@@ -52,7 +52,7 @@ function addressbook_init($showerr = true, $onlylocal = false) {
                             'owner' => $username,
                             'table' => $addrbook_table));
         if (!$r && $showerr) {
-            $abook_init_error.=_("Error initializing addressbook database.") . "<br />\n" . $abook->error;
+            $abook_init_error.=_("Error initializing address book database.") . "\n" . $abook->error;
         }
     } else {
         /* File */
@@ -95,8 +95,8 @@ function addressbook_init($showerr = true, $onlylocal = false) {
 
         /* global abook init error is not fatal. add error message and continue */
         if (!$r && $showerr) {
-            if ($abook_init_error!='') $abook_init_error.="<br />\n";
-            $abook_init_error.=_("Error initializing global addressbook.") . "<br />\n" . $abook->error;
+            if ($abook_init_error!='') $abook_init_error.="\n";
+            $abook_init_error.=_("Error initializing global address book.") . "\n" . $abook->error;
         }
     }
 
@@ -115,8 +115,8 @@ function addressbook_init($showerr = true, $onlylocal = false) {
                                      'table' => $addrbook_global_table));
       /* global abook init error is not fatal. add error message and continue */
       if (!$r && $showerr) {
-          if ($abook_init_error!='') $abook_init_error.="<br />\n";
-          $abook_init_error.=_("Error initializing global addressbook.") . "<br />\n" . $abook->error;
+          if ($abook_init_error!='') $abook_init_error.="\n";
+          $abook_init_error.=_("Error initializing global address book.") . "\n" . $abook->error;
       }
     }
 
@@ -132,8 +132,8 @@ function addressbook_init($showerr = true, $onlylocal = false) {
     $abook = $hookReturn[1];
     $r = $hookReturn[2];
     if (!$r && $showerr) {
-        if ($abook_init_error!='') $abook_init_error.="<br />\n";
-        $abook_init_error.=_("Error initializing other address books.") . "<br />\n" . $abook->error;
+        if ($abook_init_error!='') $abook_init_error.="\n";
+        $abook_init_error.=_("Error initializing other address books.") . "\n" . $abook->error;
     }
 
 
@@ -150,9 +150,8 @@ function addressbook_init($showerr = true, $onlylocal = false) {
 
             $r = $abook->add_backend('ldap_server', $param);
             if (!$r && $showerr) {
-                if ($abook_init_error!='') $abook_init_error.="<br />\n";
-                $abook_init_error.=sprintf(_("Error initializing LDAP server %s:") .
-                                           "<br />\n", $param['host']);
+                if ($abook_init_error!='') $abook_init_error.="\n";
+                $abook_init_error.=sprintf(_("Error initializing LDAP server %s:"), $param['host'])."\n";
                 $abook_init_error.= $abook->error;
             }
         }
@@ -162,7 +161,7 @@ function addressbook_init($showerr = true, $onlylocal = false) {
      * display address book init errors.
      */
     if ($abook_init_error!='' && $showerr) {
-        error_box($abook_init_error);
+        error_box(nl2br(htmlspecialchars($abook_init_error)));
     }
 
     /* Return the initialized object */
@@ -421,14 +420,6 @@ function show_abook_sort_button($abook_sort_order, $alt_tag, $Down, $Up ) {
  * @subpackage addressbook
  */
 class AddressBook {
-
-    /*
-       Cleaning errors from html with htmlspecialchars:
-       Errors from the backend are cleaned up in this class because we not always
-       have control over it when error output is generated in the backend.
-       If this appears to be wrong place then clean it up at the source (the backend)
-    */
-
     /**
      * Enabled address book backends
      * @var array
@@ -594,7 +585,7 @@ class AddressBook {
                 if (is_array($res)) {
                     $ret = array_merge($ret, $res);
                 } else {
-                    $this->error .= "<br />\n" . htmlspecialchars($backend->error);
+                    $this->error .= "\n" . $backend->error;
                     $failed++;
                 }
             }
@@ -610,7 +601,7 @@ class AddressBook {
 
             $ret = $this->backends[$bnum]->search($expression);
             if (!is_array($ret)) {
-                $this->error .= "<br />\n" . htmlspecialchars($this->backends[$bnum]->error);
+                $this->error .= "\n" . $this->backends[$bnum]->error;
                 $ret = FALSE;
             }
         }
@@ -651,7 +642,7 @@ class AddressBook {
             if (is_array($res)) {
                return $res;
             } else {
-               $this->error = htmlspecialchars($this->backends[$bnum]->error);
+               $this->error = $this->backends[$bnum]->error;
                return false;
             }
         }
@@ -665,7 +656,7 @@ class AddressBook {
                if(!empty($res))
               return $res;
             } else {
-               $this->error = htmlspecialchars($backend->error);
+               $this->error = $backend->error;
                return false;
             }
         }
@@ -695,7 +686,7 @@ class AddressBook {
             if (is_array($res)) {
                $ret = array_merge($ret, $res);
             } else {
-               $this->error = htmlspecialchars($backend->error);
+               $this->error = $backend->error;
                return false;
             }
         }
@@ -729,6 +720,7 @@ class AddressBook {
             $userdata['nickname'] = $userdata['email'];
         }
 
+        /* Blocks use of space, :, |, #, " and ! in nickname */
         if (eregi('[ \\:\\|\\#\\"\\!]', $userdata['nickname'])) {
             $this->error = _("Nickname contains illegal characters");
             return false;
@@ -736,7 +728,7 @@ class AddressBook {
 
         /* Check that specified backend accept new entries */
         if (!$this->backends[$bnum]->writeable) {
-            $this->error = _("Addressbook is read-only");
+            $this->error = _("Address book is read-only");
             return false;
         }
 
@@ -745,7 +737,7 @@ class AddressBook {
         if ($res) {
             return $bnum;
         } else {
-            $this->error = htmlspecialchars($this->backends[$bnum]->error);
+            $this->error = $this->backends[$bnum]->error;
             return false;
         }
 
@@ -773,7 +765,7 @@ class AddressBook {
 
         /* Check that specified backend is writable */
         if (!$this->backends[$bnum]->writeable) {
-            $this->error = _("Addressbook is read-only");
+            $this->error = _("Address book is read-only");
             return false;
         }
 
@@ -782,7 +774,7 @@ class AddressBook {
         if ($res) {
             return $bnum;
         } else {
-            $this->error = htmlspecialchars($this->backends[$bnum]->error);
+            $this->error = $this->backends[$bnum]->error;
             return false;
         }
 
@@ -828,7 +820,7 @@ class AddressBook {
 
         /* Check that specified backend is writable */
         if (!$this->backends[$bnum]->writeable) {
-            $this->error = _("Addressbook is read-only");;
+            $this->error = _("Address book is read-only");;
             return false;
         }
 
@@ -837,7 +829,7 @@ class AddressBook {
         if ($res) {
             return $bnum;
         } else {
-            $this->error = htmlspecialchars($this->backends[$bnum]->error);
+            $this->error = $this->backends[$bnum]->error;
             return false;
         }
 
@@ -916,7 +908,7 @@ class addressbook_backend {
      * @return bool
      */
     function search($expression) {
-        $this->set_error('search not implemented');
+        $this->set_error('search is not implemented');
         return false;
     }
 
@@ -926,7 +918,7 @@ class addressbook_backend {
      * @return bool
      */
     function lookup($alias) {
-        $this->set_error('lookup not implemented');
+        $this->set_error('lookup is not implemented');
         return false;
     }
 
@@ -938,7 +930,7 @@ class addressbook_backend {
      * @return bool
      */
     function list_addr() {
-        $this->set_error('list_addr not implemented');
+        $this->set_error('list_addr is not implemented');
         return false;
     }
 
@@ -948,7 +940,7 @@ class addressbook_backend {
      * @return bool
      */
     function add($userdata) {
-        $this->set_error('add not implemented');
+        $this->set_error('add is not implemented');
         return false;
     }
 
@@ -958,7 +950,7 @@ class addressbook_backend {
      * @return bool
      */
     function remove($alias) {
-        $this->set_error('delete not implemented');
+        $this->set_error('delete is not implemented');
         return false;
     }
 
@@ -969,7 +961,25 @@ class addressbook_backend {
      * @return bool
      */
     function modify($alias, $newuserdata) {
-        $this->set_error('modify not implemented');
+        $this->set_error('modify is not implemented');
         return false;
     }
+
+    /**
+     * Creates full name from given name and surname
+     *
+     * Handles name order differences
+     * @param string $firstname given name
+     * @param string $lastname surname
+     * @return string full name
+     * @since 1.5.2
+     */
+    function fullname($firstname,$lastname) {
+        global $squirrelmail_language;
+        if ($squirrelmail_language=='ja_JP') {
+            return trim($lastname . ' ' . $firstname);
+        } else {
+            return trim($firstname . ' ' . $lastname);
+        }
+    }
 }

src/addrbook_search.php

@@ -282,7 +282,7 @@ if ($show == 'form' && ! isset($listall)) {
         if (!is_array($res)) {
             echo html_tag( 'p', '<b><br />' .
                            _("Your search failed with the following error(s)") .
-                           ':<br />' . $abook->error . "</b>\n" ,
+                           ':<br />' . nl2br(htmlspecialchars($abook->error)) . "</b>\n" ,
                            'center' );
         } elseif (sizeof($res) == 0) {
             echo html_tag( 'p', '<br /><b>' .

src/addrbook_search_html.php

@@ -116,21 +116,7 @@ function addr_display_result($res, $includesource = true) {
         } else {
             $tr_bgcolor = $color[4];
         }
-        if ($squirrelmail_language == 'ja_JP')
-            {
-        echo html_tag( 'tr', '', '', $tr_bgcolor, 'style="white-space: nowrap;"' ) .
-        html_tag( 'td',
-             '<input type="checkbox" name="send_to_search[T' . $line . ']" value = "' .
-             htmlspecialchars($email) . '" />&nbsp;' . _("To") . '&nbsp;' .
-             '<input type="checkbox" name="send_to_search[C' . $line . ']" value = "' .
-             htmlspecialchars($email) . '" />&nbsp;' . _("Cc") . '&nbsp;' .
-             '<input type="checkbox" name="send_to_search[B' . $line . ']" value = "' .
-             htmlspecialchars($email) . '" />&nbsp;' . _("Bcc") . '&nbsp;' ,
-        'center', '', 'width="5%" style="white-space: nowrap;"' ) .
-        html_tag( 'td', '&nbsp;' . htmlspecialchars($row['lastname']) . ' ' . htmlspecialchars($row['firstname']) . '&nbsp;', 'left', '', 'style="white-space: nowrap;"' ) .
-        html_tag( 'td', '&nbsp;' . htmlspecialchars($row['email']) . '&nbsp;', 'left', '', 'style="white-space: nowrap;"' ) .
-        html_tag( 'td', '&nbsp;' . htmlspecialchars($row['label']) . '&nbsp;', 'left', '', 'style="white-space: nowrap;"' );
-            } else {
+
         echo html_tag( 'tr', '', '', $tr_bgcolor, 'style="white-space: nowrap;"' ) .
         html_tag( 'td',
             addCheckBox('send_to_search[T'.$line.']', FALSE, $email).
@@ -143,13 +129,12 @@ function addr_display_result($res, $includesource = true) {
         html_tag( 'td', '&nbsp;' . htmlspecialchars($row['name']) . '&nbsp;', 'left', '', 'style="white-space: nowrap;"' ) .
         html_tag( 'td', '&nbsp;' . htmlspecialchars($row['email']) . '&nbsp;', 'left', '', 'style="white-space: nowrap;"' ) .
         html_tag( 'td', '&nbsp;' . htmlspecialchars($row['label']) . '&nbsp;', 'left', '', 'style="white-space: nowrap;"' );
-            }
 
-         if ($includesource) {
-             echo html_tag( 'td', '&nbsp;' . $row['source'] . '&nbsp;', 'left', '', 'style="white-space: nowrap;"' );
-         }
-         echo "</tr>\n";
-         $line ++;
+        if ($includesource) {
+            echo html_tag( 'td', '&nbsp;' . $row['source'] . '&nbsp;', 'left', '', 'style="white-space: nowrap;"' );
+        }
+        echo "</tr>\n";
+        $line ++;
     }
     if ($includesource) { $td_colspan = '5'; } else { $td_colspan = '4'; }
     echo html_tag( 'tr',
@@ -268,7 +253,7 @@ if ($addrquery == '' || ! empty($listall)) {
     if (!is_array($res)) {
         echo html_tag( 'p', '<b><br />' .
                        _("Your search failed with the following error(s)") .
-                       ':<br />' . $abook->error . "</b>\n" ,
+                       ':<br />' . nl2br(htmlspecialchars($abook->error)) . "</b>\n" ,
                        'center' ) . "\n";
         $oTemplate->display('footer.tpl');
     } else {

src/addressbook.php

@@ -215,7 +215,7 @@ if (!empty($formerror)) {
             html_tag( 'tr',
                 html_tag( 'td',
                     "\n". '<br /><strong><font color="' . $color[2] .
-                    '">' . _("ERROR") . ': ' . $formerror . '</font></strong>' ."\n",
+                    '">' . _("ERROR") . ': ' . nl2br(htmlspecialchars($formerror)) . '</font></strong>' ."\n",
                     'center' )
                 ),
             'center', '', 'width="100%"' );
@@ -227,7 +227,7 @@ if ($showaddrlist) {
     /* Get and sort address list */
     $alist = $abook->list_addr();
     if(!is_array($alist)) {
-        plain_error_message($abook->error, $color);
+        plain_error_message(nl2br(htmlspecialchars($abook->error)), $color);
         exit;
     }
 
@@ -325,20 +325,9 @@ if ($showaddrlist) {
                            '&nbsp;' . htmlspecialchars($row['nickname']) . '&nbsp;',
                            'left', '', 'valign="top" width="1%" style="white-space: nowrap;"' );
 
-            // different full name display formating for Japanese translation
-            if ($squirrelmail_language == 'ja_JP') {
-                /*
-                 * translation uses euc-jp character set internally.
-                 * htmlspecialchars() should not break any characters.
-                 */
-                echo html_tag( 'td',
-                               '&nbsp;' . htmlspecialchars($row['lastname']) . ' ' . htmlspecialchars($row['firstname']) . '&nbsp;',
-                               'left', '', 'valign="top" width="1%" style="white-space: nowrap;"' );
-            } else {
-                echo html_tag( 'td',
-                               '&nbsp;' . htmlspecialchars($row['name']) . '&nbsp;',
-                               'left', '', 'valign="top" width="1%" style="white-space: nowrap;"' );
-            }
+            echo html_tag( 'td',
+                           '&nbsp;' . htmlspecialchars($row['name']) . '&nbsp;',
+                           'left', '', 'valign="top" width="1%" style="white-space: nowrap;"' );
 
             // email address column
             echo html_tag( 'td', '', 'left', '', 'valign="top" width="1%" style="white-space: nowrap;"' ) . '&nbsp;';