[Home]

Summary:ASTERISK-11831: Bug in ldapsearch result check
Reporter:Nito Martinez (nito)Labels:
Date Opened:2008-04-11 07:09:08Date Closed:2008-04-11 16:14:07
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Resources/res_config_ldap
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) res_config_ldap_result_check_patch.diff
Description:
The realtime module res_config_ldap.c does not reconnect after an ldap restart, or ldap timeout.


This is because the result of the ldapsearch operation is not done correctly. See below.

*Note this has been tested in asterisk-1.6.0-beta6 and asterisk-1.6.0-beta7.1 not in 1.6.0-beta5*


****** ADDITIONAL INFORMATION ******



In several parts of res/res_ldap_config.c it checks after the ldap_search for connection or timeout errors and if they ocurr, it tries to reconnect. See the extract of the code:


result = ldap_search_ext_s(...not relevant...);
...
while (result < 0 && tries < 3 && is_ldap_connect_error(result)

But in

static int is_ldap_connect_error(int err)
{
       return (err == LDAP_SERVER_DOWN
                       || err == LDAP_TIMEOUT || err == LDAP_CONNECT_ERROR);
}


And if you check in /usr/include/ldap.h (openldap)

#define LDAP_SERVER_DOWN                0x51
#define LDAP_TIMEOUT                    0x55
#define LDAP_CONNECT_ERROR                              0x5b

Obviously it will never reconnect (the check is for result < 0).

The fix would be changing the comparison from:

while (result < 0 && tries < 3 && is_ldap_connect_error(result)

to:

while (result != 0 && tries < 3 && is_ldap_connect_error(result)

or

while (result != LDAP_SUCCESS && tries < 3 && is_ldap_connect_error(result)
Comments:By: Nito Martinez (nito) 2008-04-11 08:42:56

I added the simple patch that would fix this
By: Digium Subversion (svnbot) 2008-04-11 09:49:31

Repository: asterisk
Revision: 114061

U   trunk/res/res_config_ldap.c

------------------------------------------------------------------------
r114061 | tilghman | 2008-04-11 09:49:26 -0500 (Fri, 11 Apr 2008) | 6 lines

Errors are all greater than 0
(closes issue ASTERISK-11831)
Reported by: nito
Patches:
      res_config_ldap_result_check_patch.diff uploaded by nito (license 340)

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=114061
By: Digium Subversion (svnbot) 2008-04-11 09:51:47

Repository: asterisk
Revision: 114062

_U  branches/1.6.0/
U   branches/1.6.0/res/res_config_ldap.c

------------------------------------------------------------------------
r114062 | tilghman | 2008-04-11 09:51:25 -0500 (Fri, 11 Apr 2008) | 14 lines

Merged revisions 114061 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

........
r114061 | tilghman | 2008-04-11 09:54:22 -0500 (Fri, 11 Apr 2008) | 6 lines

Errors are all greater than 0
(closes issue ASTERISK-11831)
Reported by: nito
Patches:
      res_config_ldap_result_check_patch.diff uploaded by nito (license 340)

........

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=114062
By: Digium Subversion (svnbot) 2008-04-11 16:14:07

Repository: asterisk
Revision: 114070

_U  team/murf/bug11210/
U   team/murf/bug11210/CHANGES
U   team/murf/bug11210/channels/chan_local.c
U   team/murf/bug11210/channels/chan_sip.c
U   team/murf/bug11210/contrib/scripts/astcli
U   team/murf/bug11210/include/asterisk/lock.h
U   team/murf/bug11210/main/features.c
U   team/murf/bug11210/main/manager.c
U   team/murf/bug11210/main/utils.c
U   team/murf/bug11210/res/res_config_ldap.c
U   team/murf/bug11210/res/res_phoneprov.c
U   team/murf/bug11210/utils/Makefile
U   team/murf/bug11210/utils/ael_main.c
U   team/murf/bug11210/utils/astman.c
U   team/murf/bug11210/utils/check_expr.c
U   team/murf/bug11210/utils/conf2ael.c
U   team/murf/bug11210/utils/hashtest.c
U   team/murf/bug11210/utils/hashtest2.c

------------------------------------------------------------------------
r114070 | murf | 2008-04-11 16:14:03 -0500 (Fri, 11 Apr 2008) | 80 lines

Merged revisions 114042-114069 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

................
 r114042 | tilghman | 2008-04-10 13:04:29 -0600 (Thu, 10 Apr 2008) | 7 lines
 
 The hydra grows yet another head...
 (closes issue ASTERISK-11814)
  Reported by: davevg
  Patches:
        astcli.diff2 uploaded by davevg (license 209)
  Tested by: davevg, Corydon76
................
 r114046 | mmichelson | 2008-04-10 13:58:36 -0600 (Thu, 10 Apr 2008) | 14 lines
 
 Merged revisions 114045 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
 r114045 | mmichelson | 2008-04-10 14:55:33 -0500 (Thu, 10 Apr 2008) | 6 lines
 
 Be sure that we're not about to set bridgepvt NULL prior to dereferencing it.
 
 (closes issue ASTERISK-11244)
 Reported by: fujin
 
 
 ........
................
 r114049 | file | 2008-04-10 14:28:40 -0600 (Thu, 10 Apr 2008) | 2 lines
 
 A 'b' option has been added which causes chan_local to return the actual channel that is behind it when queried. This is useful for transfer scenarios as the actual channel will be transferred, not the Local channel. If you have been using Local channels as queue members and having issues when the agent did a blind transfer this option may solve the issue.
................
 r114052 | mmichelson | 2008-04-10 16:02:32 -0600 (Thu, 10 Apr 2008) | 11 lines
 
 Merged revisions 114051 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
 r114051 | mmichelson | 2008-04-10 15:59:49 -0500 (Thu, 10 Apr 2008) | 3 lines
 
 Fix 1.4 build when LOW_MEMORY is enabled.
 
 
 ........
................
 r114061 | tilghman | 2008-04-11 08:54:22 -0600 (Fri, 11 Apr 2008) | 6 lines
 
 Errors are all greater than 0
 (closes issue ASTERISK-11831)
  Reported by: nito
  Patches:
        res_config_ldap_result_check_patch.diff uploaded by nito (license 340)
................
 r114064 | mmichelson | 2008-04-11 09:49:35 -0600 (Fri, 11 Apr 2008) | 19 lines
 
 Merged revisions 114063 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
 r114063 | mmichelson | 2008-04-11 10:44:28 -0500 (Fri, 11 Apr 2008) | 11 lines
 
 Fix a race condition that may happen between a sip hangup
 and a "core show channel" command. This patch adds locking
 to prevent the resulting crash.
 
 (closes issue ASTERISK-11586)
 Reported by: tsearle
 Patches:
       show_channels_crash2.patch uploaded by tsearle (license 373)
 Tested by: tsearle
 
 
 ........
................
 r114067 | twilson | 2008-04-11 15:04:46 -0600 (Fri, 11 Apr 2008) | 3 lines
 
 Fix the fact that global_variables 1) weren't being updated on reload (thanks for the report, Doug), and 2) weren't actually being appended to the list of profile variables because build_profile was called before the list was populated. Also needed to free the contents returned by load_file().
................

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=114070