From 3523532820419e678843408d5a31db951636fa55 Mon Sep 17 00:00:00 2001
From: tokul <tokul@7612ce4b-ef26-0410-bec9-ea0150e637f0>
Date: Sun, 4 Jun 2006 12:42:24 +0000
Subject: [PATCH] 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'.


git-svn-id: https://svn.code.sf.net/p/squirrelmail/code/trunk/squirrelmail@11181 7612ce4b-ef26-0410-bec9-ea0150e637f0
---
 ChangeLog                       |  6 +++
 functions/abook_database.php    | 16 +++----
 functions/abook_ldap_server.php |  3 +-
 functions/abook_local_file.php  | 16 +++----
 functions/addressbook.php       | 82 ++++++++++++++++++---------------
 src/addrbook_search.php         |  2 +-
 src/addrbook_search_html.php    | 29 +++---------
 src/addressbook.php             | 21 ++-------
 8 files changed, 82 insertions(+), 93 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 10f83cee..309f16ad 100644
--- a/ChangeLog
+++ b/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)
 --------------------------------------
diff --git a/functions/abook_database.php b/functions/abook_database.php
index a662b3da..77deaac7 100644
--- a/functions/abook_database.php
+++ b/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()) {
diff --git a/functions/abook_ldap_server.php b/functions/abook_ldap_server.php
index cb1bb70f..350fb5ac 100644
--- a/functions/abook_ldap_server.php
+++ b/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'])) {
diff --git a/functions/abook_local_file.php b/functions/abook_local_file.php
index 274e19da..63b6a4a7 100644
--- a/functions/abook_local_file.php
+++ b/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 */
diff --git a/functions/addressbook.php b/functions/addressbook.php
index ba9c6b0c..e6f2e844 100644
--- a/functions/addressbook.php
+++ b/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);
+        }
+    }
 }
diff --git a/src/addrbook_search.php b/src/addrbook_search.php
index 0091b182..b64d16d6 100644
--- a/src/addrbook_search.php
+++ b/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>' .
diff --git a/src/addrbook_search_html.php b/src/addrbook_search_html.php
index 17d3eb9f..ac0be560 100644
--- a/src/addrbook_search_html.php
+++ 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 {
diff --git a/src/addressbook.php b/src/addressbook.php
index f60066ee..68d85244 100644
--- a/src/addressbook.php
+++ b/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;';
-- 
2.25.1