[turba] Possible Bug (and fix) For "1-click add_address" From Message Display

[Andy Dorman]

First my sincere thanks to the horde/turba team.  You have put  
together what I consider one of the premier open source applications.

Next, if everyone (anyone?) agrees this is a real bug, I will be happy  
to submit it as such along with the suggested fix.

Finally, to the actual issue...our system uses the latest stable Turba  
(just updated to 2.3.2) & 2.4.xx OpenLDAP for address and address book  
storage.

I noticed last week that clicking on the little "add" icon next to a  
new address (ie, an address not yet in my default address book) in the  
IMP message display gave me an error "Read failed: (0) Success"

Clicking on "add" for an existing address returned the correct  
"Already exists" result.

I found the issue to be the method, _get_results, in  
turba/lib/Driver/ldap.php, that checks the results of a search for a  
duplicate address before adding the new one .  When nothing is found  
(ie, the results array is empty), _get_results (line 491) considered  
that to be a PEAR error.  However, lack of results only means a search  
came up empty, not that there was an LDAP error.  We should check the  
error code before declaring a problem.

The good news is that fixing it, for OpenLDAP anyway, is simple.   
OpenLDAP returns an error code of 0 when there were no LDAP problems.   
So it was a simple matter to add a check of the error code before  
declaring that there was a problem.

The existing line in context is:

if (!($entries = @ldap_get_entries($this->_ds, $res))) {
             return PEAR::raiseError(sprintf(_("Read failed: (%s)  
%s"), ldap_errno($this->_ds), ldap_error($this->_ds)));
}

The revised if condition we are now using is:

if (!($entries = @ldap_get_entries($this->_ds, $res)) and  
ldap_errno($this->_ds)) {
             return PEAR::raiseError(sprintf(_("Read failed: (%s)  
%s"), ldap_errno($this->_ds), ldap_error($this->_ds)));
}

And just to make sure I am working with the current version, the stamp  
at the top of the applicable file is:

* $Horde: turba/lib/Driver/ldap.php,v 1.54.4.22 2009/08/18 17:00:06 jan Exp $

I have tested with normal address book functions and have not  
encountered any problems with this fix.

Again, thanks for all your work.

-- 
Andy Dorman


----------------------------------------------------------------
This message was sent using IMP, the Internet Messaging Program,
part of the Horde framework and provided by HomeFreeMail.com.