You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Ruediger Pluem <rp...@apache.org> on 2008/01/01 13:30:41 UTC

Re: svn commit: r607766 - in /httpd/httpd/trunk: CHANGES include/util_ldap.h modules/aaa/mod_authnz_ldap.c modules/ldap/util_ldap.c


On 12/31/2007 08:20 PM, covener@apache.org wrote:
> Author: covener
> Date: Mon Dec 31 11:20:25 2007
> New Revision: 607766
> 
> URL: http://svn.apache.org/viewvc?rev=607766&view=rev
> Log:
> When using the MS SDK, re-establish LDAP backend connections on a
> return code of LDAP_UNAVAILABLE as if it were LDAP_SERVER_DOWN.
> 
> With this SDK, LDAP_UNAVAIALBLE is returned when the socket had been closed 
> between LDAP API calls.
> 
> PR 39095
> 
> 
> Modified:
>     httpd/httpd/trunk/CHANGES
>     httpd/httpd/trunk/include/util_ldap.h
>     httpd/httpd/trunk/modules/aaa/mod_authnz_ldap.c
>     httpd/httpd/trunk/modules/ldap/util_ldap.c
> 

> Modified: httpd/httpd/trunk/include/util_ldap.h
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/include/util_ldap.h?rev=607766&r1=607765&r2=607766&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/include/util_ldap.h (original)
> +++ httpd/httpd/trunk/include/util_ldap.h Mon Dec 31 11:20:25 2007
> @@ -30,6 +30,13 @@
>  #include "apr_time.h"
>  #include "apr_ldap.h"
>  
> +#if APR_HAS_MICROSOFT_LDAPSDK
> +#define AP_LDAP_IS_SERVER_DOWN(s)                ((s) == LDAP_SERVER_DOWN \
> +                ||(s) == LDAP_UNAVAILABLE)
> +#else
> +#define AP_LDAP_IS_SERVER_DOWN(s)                ((s) == LDAP_SERVER_DOWN)
> +#endif
> +
>  #if APR_HAS_SHARED_MEMORY
>  #include "apr_rmm.h"
>  #include "apr_shm.h"
> 

> Modified: httpd/httpd/trunk/modules/ldap/util_ldap.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ldap/util_ldap.c?rev=607766&r1=607765&r2=607766&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/ldap/util_ldap.c (original)
> +++ httpd/httpd/trunk/modules/ldap/util_ldap.c Mon Dec 31 11:20:25 2007

> @@ -733,10 +733,9 @@
>      }
>  
>      /* search for reqdn */
> -    if ((result = ldap_search_ext_s(ldc->ldap, (char *)reqdn, LDAP_SCOPE_BASE,
> +    if (AP_LDAP_IS_SERVER_DOWN(result = ldap_search_ext_s(ldc->ldap, (char *)reqdn, LDAP_SCOPE_BASE,
>                                      "(objectclass=*)", NULL, 1,
> -                                    NULL, NULL, NULL, APR_LDAP_SIZELIMIT, &res))
> -            == LDAP_SERVER_DOWN)
> +                                    NULL, NULL, NULL, APR_LDAP_SIZELIMIT, &res)))

Hm. Doesn't this mean that you call ldap_search_ext_s twice in the case that you use the MS SDK and the
result of the first call is not LDAP_SERVER_DOWN?
I think you need to store the result of ldap_search_ext_s in result first and apply the
AP_LDAP_IS_SERVER_DOWN macro on result.

>      {
>          ldc->reason = "DN Comparison ldap_search_ext_s() "
>                        "failed with server down";
> @@ -873,11 +872,10 @@
>          return result;
>      }
>  
> -    if ((result = ldap_compare_s(ldc->ldap,
> +    if (AP_LDAP_IS_SERVER_DOWN(result = ldap_compare_s(ldc->ldap,
>                                   (char *)dn,
>                                   (char *)attrib,
> -                                 (char *)value))
> -                                               == LDAP_SERVER_DOWN) {
> +                                 (char *)value))) {

Same comment as above with ldap_search_ext_s

>          /* connection failed - try again */
>          ldc->reason = "ldap_compare_s() failed with server down";
>          uldap_connection_unbind(ldc);


> @@ -1443,11 +1441,10 @@
>      }
>  
>      /* try do the search */
> -    if ((result = ldap_search_ext_s(ldc->ldap,
> +    if (AP_LDAP_IS_SERVER_DOWN(result = ldap_search_ext_s(ldc->ldap,
>                                      (char *)basedn, scope,
>                                      (char *)filter, attrs, 0,
> -                                    NULL, NULL, NULL, APR_LDAP_SIZELIMIT, &res))
> -            == LDAP_SERVER_DOWN)
> +                                    NULL, NULL, NULL, APR_LDAP_SIZELIMIT, &res)))

Same comment as above

>      {
>          ldc->reason = "ldap_search_ext_s() for user failed with server down";
>          uldap_connection_unbind(ldc);
> @@ -1501,9 +1498,9 @@
>       * fails, it means that the password is wrong (the dn obviously
>       * exists, since we just retrieved it)
>       */
> -    if ((result = ldap_simple_bind_s(ldc->ldap,
> +    if (AP_LDAP_IS_SERVER_DOWN(result = ldap_simple_bind_s(ldc->ldap,
>                                       (char *)*binddn,
> -                                     (char *)bindpw)) == LDAP_SERVER_DOWN) {
> +                                     (char *)bindpw))) {


Same comment as above with ldap_search_ext_s

>          ldc->reason = "ldap_simple_bind_s() to check user credentials "
>                        "failed with server down";
>          ldap_msgfree(res);
> @@ -1692,11 +1689,10 @@
>      }
>  
>      /* try do the search */
> -    if ((result = ldap_search_ext_s(ldc->ldap,
> +    if (AP_LDAP_IS_SERVER_DOWN(result = ldap_search_ext_s(ldc->ldap,
>                                      (char *)basedn, scope,
>                                      (char *)filter, attrs, 0,
> -                                    NULL, NULL, NULL, APR_LDAP_SIZELIMIT, &res))
> -            == LDAP_SERVER_DOWN)
> +                                    NULL, NULL, NULL, APR_LDAP_SIZELIMIT, &res)))

Same comment as above.

Regards

RĂ¼diger


Re: svn commit: r607766 - in /httpd/httpd/trunk: CHANGES include/util_ldap.h modules/aaa/mod_authnz_ldap.c modules/ldap/util_ldap.c

Posted by Eric Covener <co...@gmail.com>.
On Jan 1, 2008 7:30 AM, Ruediger Pluem <rp...@apache.org> wrote:
> Hm. Doesn't this mean that you call ldap_search_ext_s twice in the case that you use the MS SDK and the
> result of the first call is not LDAP_SERVER_DOWN?
> I think you need to store the result of ldap_search_ext_s in result first and apply the
> AP_LDAP_IS_SERVER_DOWN macro on result.

Much appreciated, r607841 and backport updated.

-- 
Eric Covener
covener@gmail.com