You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by bn...@apache.org on 2005/04/09 21:00:20 UTC

svn commit: r160707 - in httpd/httpd/branches/2.0.x: CHANGES STATUS docs/manual/mod/mod_ldap.xml include/util_ldap.h modules/experimental/util_ldap.c

Author: bnicholes
Date: Sat Apr  9 12:00:18 2005
New Revision: 160707

URL: http://svn.apache.org/viewcvs?view=rev&rev=160707
Log:
Added a new LDAPConnectionTimeout directive to util_ldap so that the socket connection timeout value is configurable.

Reviewed by: bnicholes, trawick, jim

Modified:
    httpd/httpd/branches/2.0.x/CHANGES
    httpd/httpd/branches/2.0.x/STATUS
    httpd/httpd/branches/2.0.x/docs/manual/mod/mod_ldap.xml
    httpd/httpd/branches/2.0.x/include/util_ldap.h
    httpd/httpd/branches/2.0.x/modules/experimental/util_ldap.c

Modified: httpd/httpd/branches/2.0.x/CHANGES
URL: http://svn.apache.org/viewcvs/httpd/httpd/branches/2.0.x/CHANGES?view=diff&r1=160706&r2=160707
==============================================================================
--- httpd/httpd/branches/2.0.x/CHANGES (original)
+++ httpd/httpd/branches/2.0.x/CHANGES Sat Apr  9 12:00:18 2005
@@ -1,5 +1,9 @@
 Changes with Apache 2.0.54
 
+  *) mod_ldap: Added the directive LDAPConnectionTimeout to configure
+     the ldap socket connection timeout value.  
+     [Brad Nicholes]
+
   *) Correctly export all mod_dav public functions.
      [Branko Èibej <brane xbc.nu>]
 

Modified: httpd/httpd/branches/2.0.x/STATUS
URL: http://svn.apache.org/viewcvs/httpd/httpd/branches/2.0.x/STATUS?view=diff&r1=160706&r2=160707
==============================================================================
--- httpd/httpd/branches/2.0.x/STATUS (original)
+++ httpd/httpd/branches/2.0.x/STATUS Sat Apr  9 12:00:18 2005
@@ -94,11 +94,6 @@
     identify exactly what the proposed changes are! ]
   [ please append new backports at the end of this list not the top. ]
 
-    *) util_ldap: Add the directive LDAPConnectionTimeout to control
-       the socket timeout value when binding to an LDAP server
-       svn rev 126565
-       +1: bnicholes, trawick (no need for APLOG_NOERRNO in Apache >=2), jim
-
     *) several changes to improve logging of connection-oriented errors, including
        ap_log_cerror() API (needs minor bump in addition to changes below)
          http://cvs.apache.org/viewcvs.cgi/httpd-2.0/server/core.c?r1=1.289&r2=1.291

Modified: httpd/httpd/branches/2.0.x/docs/manual/mod/mod_ldap.xml
URL: http://svn.apache.org/viewcvs/httpd/httpd/branches/2.0.x/docs/manual/mod/mod_ldap.xml?view=diff&r1=160706&r2=160707
==============================================================================
--- httpd/httpd/branches/2.0.x/docs/manual/mod/mod_ldap.xml (original)
+++ httpd/httpd/branches/2.0.x/docs/manual/mod/mod_ldap.xml Sat Apr  9 12:00:18 2005
@@ -340,4 +340,19 @@
 </usage>
 </directivesynopsis>
 
+<directivesynopsis>
+<name>LDAPConnectionTimeout</name>
+<description>Specifies the socket connection timeout in seconds</description>
+<syntax>LDAPConnectionTimeout <var>seconds</var></syntax>
+<contextlist><context>server config</context></contextlist>
+
+<usage>
+    <p>Specifies the timeout value (in seconds) in which the module will
+    attempt to connect to the LDAP server.  If a connection is not
+    successful with the timeout period, either an error will be 
+    returned or the module will attempt to connect to a secondary LDAP 
+    server if one is specified. The default is 10 seconds.</p>
+</usage>
+</directivesynopsis>
+
 </modulesynopsis>

Modified: httpd/httpd/branches/2.0.x/include/util_ldap.h
URL: http://svn.apache.org/viewcvs/httpd/httpd/branches/2.0.x/include/util_ldap.h?view=diff&r1=160706&r2=160707
==============================================================================
--- httpd/httpd/branches/2.0.x/include/util_ldap.h (original)
+++ httpd/httpd/branches/2.0.x/include/util_ldap.h Sat Apr  9 12:00:18 2005
@@ -126,6 +126,7 @@
     /* cache ald */
     void *util_ldap_cache;
     char *lock_file;           /* filename for shm lock mutex */
+    int connectionTimeout;
 
 } util_ldap_state_t;
 

Modified: httpd/httpd/branches/2.0.x/modules/experimental/util_ldap.c
URL: http://svn.apache.org/viewcvs/httpd/httpd/branches/2.0.x/modules/experimental/util_ldap.c?view=diff&r1=160706&r2=160707
==============================================================================
--- httpd/httpd/branches/2.0.x/modules/experimental/util_ldap.c (original)
+++ httpd/httpd/branches/2.0.x/modules/experimental/util_ldap.c Sat Apr  9 12:00:18 2005
@@ -1330,6 +1330,30 @@
     return(NULL);
 }
 
+static const char *util_ldap_set_connection_timeout(cmd_parms *cmd, void *dummy, const char *ttl)
+{
+    util_ldap_state_t *st = 
+        (util_ldap_state_t *)ap_get_module_config(cmd->server->module_config, 
+						  &ldap_module);
+    const char *err = ap_check_cmd_context(cmd, GLOBAL_ONLY);
+
+    if (err != NULL) {
+        return err;
+    }
+
+#ifdef LDAP_OPT_NETWORK_TIMEOUT
+    st->connectionTimeout = atol(ttl);
+
+    ap_log_error(APLOG_MARK, APLOG_DEBUG|APLOG_NOERRNO, 0, cmd->server, 
+                      "[%d] ldap connection: Setting connection timeout to %ld seconds.", 
+                      getpid(), st->connectionTimeout);
+#else
+    ap_log_error(APLOG_MARK, APLOG_NOTICE, 0, cmd->server,
+                     "LDAP: Connection timout option not supported by the LDAP SDK in use." );
+#endif
+
+    return NULL;
+}
 
 void *util_ldap_create_config(apr_pool_t *p, server_rec *s)
 {
@@ -1347,6 +1371,7 @@
     st->cert_auth_file = NULL;
     st->cert_file_type = LDAP_CA_TYPE_UNKNOWN;
     st->ssl_support = 0;
+    st->connectionTimeout = 10;
 
     return st;
 }
@@ -1379,6 +1404,7 @@
 
     void *data;
     const char *userdata_key = "util_ldap_init";
+    struct timeval timeOut = {10,0};    /* 10 second connection timeout */
 
     /* util_ldap_post_config() will be called twice. Don't bother
      * going through all of the initialization on the first call
@@ -1603,6 +1629,20 @@
                          "LDAP: SSL support unavailable" );
     }
     
+#ifdef LDAP_OPT_NETWORK_TIMEOUT
+    if (st->connectionTimeout > 0) {
+        timeOut.tv_sec = st->connectionTimeout;
+    }
+
+    if (st->connectionTimeout >= 0) {
+        rc = ldap_set_option(NULL, LDAP_OPT_NETWORK_TIMEOUT, (void *)&timeOut);
+        if (APR_SUCCESS != rc) {
+            ap_log_error(APLOG_MARK, APLOG_ERR, 0, s,
+                             "LDAP: Could not set the connection timeout" );
+        }
+    }
+#endif
+
     return(OK);
 }
 
@@ -1667,6 +1707,11 @@
                  "    DER_FILE      - file in binary DER format "
                  "    BASE64_FILE   - file in Base64 format "
                  "    CERT7_DB_PATH - Netscape certificate database file "),
+
+    AP_INIT_TAKE1("LDAPConnectionTimeout", util_ldap_set_connection_timeout, NULL, RSRC_CONF,
+                  "Specifies the LDAP socket connection timeout in seconds. "
+                  "Default is 10 seconds. "),
+
     {NULL}
 };
 



Re: svn commit: r160707 - in httpd/httpd/branches/2.0.x: CHANGES STATUS docs/manual/mod/mod_ldap.xml include/util_ldap.h modules/experimental/util_ldap.c

Posted by Graham Leggett <mi...@sharp.fm>.
Paul Querna wrote:

> It looks like this change is causing crashes with PHP:
> http://issues.apache.org/bugzilla/show_bug.cgi?id=34618
> http://issues.apache.org/bugzilla/show_bug.cgi?id=34620
> 
> I think the call to ldap_set_option is the cause.
> 
> Quoting from http://docs.sun.com/source/816-5616-10/function.htm#24534
> 
> """If NULL, you are setting the default options that will apply to any
> new LDAP connection handles that are subsequently created.  """
> 
> Doesn't it seem bad to set this globally?  We are trampling on other
> citizens inside the process.
> 
> Why isn't this call to ldap_set_option done per-LDAP context?

I think it probably should be done per context when each connection is 
created, rather than set globally.

+1.

Regards,
Graham
--

Re: svn commit: r160707 - in httpd/httpd/branches/2.0.x: CHANGES STATUS docs/manual/mod/mod_ldap.xml include/util_ldap.h modules/experimental/util_ldap.c

Posted by Graham Leggett <mi...@sharp.fm>.
Paul Querna wrote:

> It looks like this change is causing crashes with PHP:
> http://issues.apache.org/bugzilla/show_bug.cgi?id=34618
> http://issues.apache.org/bugzilla/show_bug.cgi?id=34620
> 
> I think the call to ldap_set_option is the cause.
> 
> Quoting from http://docs.sun.com/source/816-5616-10/function.htm#24534
> 
> """If NULL, you are setting the default options that will apply to any
> new LDAP connection handles that are subsequently created.  """
> 
> Doesn't it seem bad to set this globally?  We are trampling on other
> citizens inside the process.
> 
> Why isn't this call to ldap_set_option done per-LDAP context?

I think it probably should be done per context when each connection is 
created, rather than set globally.

+1.

Regards,
Graham
--

Re: svn commit: r160707 - in httpd/httpd/branches/2.0.x: CHANGES STATUS docs/manual/mod/mod_ldap.xml include/util_ldap.h modules/experimental/util_ldap.c

Posted by Paul Querna <ch...@force-elite.com>.
bnicholes@apache.org wrote:
> Author: bnicholes
> Date: Sat Apr  9 12:00:18 2005
> New Revision: 160707
> 
> URL: http://svn.apache.org/viewcvs?view=rev&rev=160707
> Log:
> Added a new LDAPConnectionTimeout directive to util_ldap so that the socket connection timeout value is configurable.
> 
> Reviewed by: bnicholes, trawick, jim
....
> @@ -1379,6 +1404,7 @@
>  
>      void *data;
>      const char *userdata_key = "util_ldap_init";
> +    struct timeval timeOut = {10,0};    /* 10 second connection timeout */
>  
>      /* util_ldap_post_config() will be called twice. Don't bother
>       * going through all of the initialization on the first call
> @@ -1603,6 +1629,20 @@
>                           "LDAP: SSL support unavailable" );
>      }
>      
> +#ifdef LDAP_OPT_NETWORK_TIMEOUT
> +    if (st->connectionTimeout > 0) {
> +        timeOut.tv_sec = st->connectionTimeout;
> +    }
> +
> +    if (st->connectionTimeout >= 0) {
> +        rc = ldap_set_option(NULL, LDAP_OPT_NETWORK_TIMEOUT, (void *)&timeOut);
> +        if (APR_SUCCESS != rc) {
> +            ap_log_error(APLOG_MARK, APLOG_ERR, 0, s,
> +                             "LDAP: Could not set the connection timeout" );
> +        }
> +    }
> +#endif
> +
>      return(OK);
>  }
>  
....

It looks like this change is causing crashes with PHP:
http://issues.apache.org/bugzilla/show_bug.cgi?id=34618
http://issues.apache.org/bugzilla/show_bug.cgi?id=34620

I think the call to ldap_set_option is the cause.

Quoting from http://docs.sun.com/source/816-5616-10/function.htm#24534

"""If NULL, you are setting the default options that will apply to any
new LDAP connection handles that are subsequently created.  """

Doesn't it seem bad to set this globally?  We are trampling on other
citizens inside the process.

Why isn't this call to ldap_set_option done per-LDAP context?

-Paul

Re: svn commit: r160707 - in httpd/httpd/branches/2.0.x: CHANGES STATUS docs/manual/mod/mod_ldap.xml include/util_ldap.h modules/experimental/util_ldap.c

Posted by Paul Querna <ch...@force-elite.com>.
bnicholes@apache.org wrote:
> Author: bnicholes
> Date: Sat Apr  9 12:00:18 2005
> New Revision: 160707
> 
> URL: http://svn.apache.org/viewcvs?view=rev&rev=160707
> Log:
> Added a new LDAPConnectionTimeout directive to util_ldap so that the socket connection timeout value is configurable.
> 
> Reviewed by: bnicholes, trawick, jim
....
> @@ -1379,6 +1404,7 @@
>  
>      void *data;
>      const char *userdata_key = "util_ldap_init";
> +    struct timeval timeOut = {10,0};    /* 10 second connection timeout */
>  
>      /* util_ldap_post_config() will be called twice. Don't bother
>       * going through all of the initialization on the first call
> @@ -1603,6 +1629,20 @@
>                           "LDAP: SSL support unavailable" );
>      }
>      
> +#ifdef LDAP_OPT_NETWORK_TIMEOUT
> +    if (st->connectionTimeout > 0) {
> +        timeOut.tv_sec = st->connectionTimeout;
> +    }
> +
> +    if (st->connectionTimeout >= 0) {
> +        rc = ldap_set_option(NULL, LDAP_OPT_NETWORK_TIMEOUT, (void *)&timeOut);
> +        if (APR_SUCCESS != rc) {
> +            ap_log_error(APLOG_MARK, APLOG_ERR, 0, s,
> +                             "LDAP: Could not set the connection timeout" );
> +        }
> +    }
> +#endif
> +
>      return(OK);
>  }
>  
....

It looks like this change is causing crashes with PHP:
http://issues.apache.org/bugzilla/show_bug.cgi?id=34618
http://issues.apache.org/bugzilla/show_bug.cgi?id=34620

I think the call to ldap_set_option is the cause.

Quoting from http://docs.sun.com/source/816-5616-10/function.htm#24534

"""If NULL, you are setting the default options that will apply to any
new LDAP connection handles that are subsequently created.  """

Doesn't it seem bad to set this globally?  We are trampling on other
citizens inside the process.

Why isn't this call to ldap_set_option done per-LDAP context?

-Paul