Mailing-List: contact dev-help@httpd.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@httpd.apache.org
Message-ID: <4797A82C.3080802@apache.org>
Date: Wed, 23 Jan 2008 21:48:44 +0100
From: Ruediger Pluem <rpluem@apache.org>
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US;
 rv:1.8.1.11) Gecko/20071128 SeaMonkey/1.1.7
MIME-Version: 1.0
To: dev@httpd.apache.org
Subject: Re: svn commit: r614605 - in /httpd/httpd/trunk: include/util_ldap.h
 modules/ldap/util_ldap.c
References: <20080123181450.D4D081A9832@eris.apache.org>
In-Reply-To: <20080123181450.D4D081A9832@eris.apache.org>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit


On 01/23/2008 07:14 PM, rederpj@apache.org wrote:
> Author: rederpj
> Date: Wed Jan 23 10:14:41 2008
> New Revision: 614605
> 
> URL: http://svn.apache.org/viewvc?rev=614605&view=rev
> Log:
> This adds Apache support (taking advantage of the new APR capability)
> for ldap rebind callback while chasing referrals. This allows direct
> searches on LDAP servers (in particular MS Active Directory 2003+)
> using referrals without the use of the global catalog.
> This addresses PRs 26538, 40268, and 42557
> 
> 
> Modified:
>     httpd/httpd/trunk/include/util_ldap.h
>     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=614605&r1=614604&r2=614605&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/include/util_ldap.h (original)
> +++ httpd/httpd/trunk/include/util_ldap.h Wed Jan 23 10:14:41 2008
> @@ -29,6 +29,7 @@
>  #include "apr_tables.h"
>  #include "apr_time.h"
>  #include "apr_ldap.h"
> +#include "apr_ldap_rebind.h"
>  
>  #if APR_HAS_MICROSOFT_LDAPSDK
>  #define AP_LDAP_IS_SERVER_DOWN(s)                ((s) == LDAP_SERVER_DOWN \
> @@ -112,11 +113,18 @@
>      apr_array_header_t *client_certs;   /* Client certificates on this connection */
>  
>      const char *reason;                 /* Reason for an error failure */
> +    int ChaseReferrals;                 /* [on|off] (on=1, off=0, default = On)*/
> +    int ReferralHopLimit;               /* # of referral hops to follow (default = 5) */

Hm. This requires a major bump. Append it to the end of the struct and you
only need a minor bump and the whole thing becomes backportable.

>  
>      struct util_ldap_connection_t *next;
>      struct util_ldap_state_t *st;        /* The LDAP vhost config this connection belongs to */
>      int keep;                            /* Will this connection be kept when it's unlocked */
>  } util_ldap_connection_t;
> +
> +typedef struct util_ldap_config_t {
> +    int ChaseReferrals;
> +    int ReferralHopLimit;
> +} util_ldap_config_t;
>  
>  /* LDAP cache state information */ 
>  typedef struct util_ldap_state_t {
> 
> Modified: httpd/httpd/trunk/modules/ldap/util_ldap.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ldap/util_ldap.c?rev=614605&r1=614604&r2=614605&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/ldap/util_ldap.c (original)
> +++ httpd/httpd/trunk/modules/ldap/util_ldap.c Wed Jan 23 10:14:41 2008

> @@ -2288,6 +2340,47 @@
>  }
>  
>  
> +static const char *util_ldap_set_chase_referrals(cmd_parms *cmd,
> +                                                 void *config,
> +                                                 int mode)
> +{
> +    util_ldap_config_t *dc =  config;
> +
> +    ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, cmd->server,
> +                      "LDAP: Setting refferal chasing %s",
> +                      mode?"ON":"OFF");
> +
> +    dc->ChaseReferrals = mode;
> +
> +    return(NULL);
> +}
> +
> +static const char *util_ldap_set_referral_hop_limit(cmd_parms *cmd,
> +                                                    void *config,
> +                                                    const char *hop_limit)
> +{
> +    util_ldap_config_t *dc =  config;
> +
> +    dc->ReferralHopLimit = atol(hop_limit);
> +
> +    ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, cmd->server,
> +                 "LDAP: Limit chased referrals to maximum of %d hops.",
> +                 dc->ReferralHopLimit);
> +
> +    return NULL;
> +}
> +
> +static void *util_ldap_create_dir_config(apr_pool_t *p, char *d) {
> +   util_ldap_config_t *dc =
> +       (util_ldap_config_t *) apr_pcalloc(p,sizeof(util_ldap_config_t));
> +
> +   dc->ChaseReferrals = 1;   /* default is to turn referral chasing on. */
> +   dc->ReferralHopLimit = 5; /* default is to chase a max of 5 hops. */

I would love to see #defines for these defaults (util_ldap.h) and have these
defines referred everywhere, even in the comments.

> +
> +   return dc;
> +}
> +
> +
>  static void *util_ldap_create_config(apr_pool_t *p, server_rec *s)
>  {
>      util_ldap_state_t *st =

> @@ -2638,7 +2743,7 @@
>  
>  module AP_MODULE_DECLARE_DATA ldap_module = {
>     STANDARD20_MODULE_STUFF,
> -   NULL,                        /* create dir config */
> +   util_ldap_create_dir_config, /* create dir config */
>     NULL,                        /* merge dir config */

Why no merge dir config? How do you inherit your settings in this case?

>     util_ldap_create_config,     /* create server config */
>     util_ldap_merge_config,      /* merge server config */
> 

Regards

Rüdiger