Enabled negative caching on attribute comparisons in the LDAP cache.

Fixed a problem where the default cache TTL was set in milliseconds
not microseconds causing the cache to time out almost immediately.
PR:
Obtained from:
Submitted by:
Reviewed by:

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@90340 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/CHANGES b/CHANGES
index 0e84804869b3467f32fe9440492e7c704fbff969..0fd6b8b230de52f540230308429ae5eb2a84020d 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,5 +1,10 @@
 Changes with Apache 2.0.25-dev
 
+  *) Enabled negative caching on attribute comparisons in the LDAP cache.
+     Fixed a problem where the default cache TTL was set in milliseconds
+     not microseconds causing the cache to time out almost immediately.
+     [Graham Leggett]
+
   *) Fixed all the #if APR_HAS_SHARED_MEMORY checks within the LDAP
      module code to follow APR. [Graham Leggett]
 

diff --git a/modules/aaa/mod_auth_ldap.c b/modules/aaa/mod_auth_ldap.c
index bdb44122aea324e19fbe729c5ef2a6e291ca4b94..7a1c316341c4d60c9479ba10585e54d4c836ea9d 100644 (file)
--- a/modules/aaa/mod_auth_ldap.c
+++ b/modules/aaa/mod_auth_ldap.c
@@ -505,8 +505,8 @@ int mod_auth_ldap_auth_checker(request_rec *r)
                     case LDAP_COMPARE_TRUE: {
                         ap_log_rerror(APLOG_MARK, APLOG_DEBUG|APLOG_NOERRNO, 0, r, 
                                       "[%d] auth_ldap authorise: require group: "
-                                      "authorisation successful (attribute %s)",
-                                      getpid(), ent[i].name);
+                                      "authorisation successful (attribute %s) [%s][%s]",
+                                      getpid(), ent[i].name, ldc->reason, ldap_err2string(result));
                         return OK;
                     }
                     default: {

diff --git a/modules/ldap/util_ldap.c b/modules/ldap/util_ldap.c
index 40c804a22ff7e33a4ff0e35b8d4af46d7fc63b58..5f3387a6ba2b5ad055a956d23799756175880f4f 100644 (file)
--- a/modules/ldap/util_ldap.c
+++ b/modules/ldap/util_ldap.c
@@ -60,16 +60,6 @@
  * Copyright 1999-2001 Dave Carrigan
  */
 
-/*
- * FIXME:
- *
- * - The compare cache presently does not have the ability to
- *   cache negatively. This has the negative effect of requiring
- *   a connect/bind/compare/unbind/disconnect when two or more
- *   atrributes are optional for group membership, and performance
- *   sucks as a result.
- */
-
 #include <apr_ldap.h>
 
 #ifdef APU_HAS_LDAP
@@ -209,7 +199,7 @@ void util_ldap_connection_close(util_ldap_connection_t *ldc)
 /*
  * Destroys an LDAP connection by unbinding. This function is registered
  * with the pool cleanup function - causing the LDAP connections to be
- * shut down cleanly on thread exit.
+ * shut down cleanly on graceful restart.
  */
 apr_status_t util_ldap_connection_destroy(void *param)
 {
@@ -646,6 +636,7 @@ int util_ldap_cache_compare(request_rec *r, util_ldap_connection_t *ldc,
     the_compare_node.dn = (char *)dn;
     the_compare_node.attrib = (char *)attrib;
     the_compare_node.value = (char *)value;
+    the_compare_node.result = 0;
 
     compare_nodep = util_ald_cache_fetch(curl->compare_cache, &the_compare_node);
 
@@ -659,8 +650,22 @@ int util_ldap_cache_compare(request_rec *r, util_ldap_connection_t *ldc,
             /* ...and it is good */
             /* unlock this read lock */
             apr_lock_release(util_ldap_cache_lock);
-            ldc->reason = "Comparison successful (cached)";
-            return LDAP_COMPARE_TRUE;
+            if (LDAP_COMPARE_TRUE == compare_nodep->result) {
+                ldc->reason = "Comparison true (cached)";
+                return compare_nodep->result;
+            }
+            else if (LDAP_COMPARE_FALSE == compare_nodep->result) {
+                ldc->reason = "Comparison false (cached)";
+                return compare_nodep->result;
+            }
+            else if (LDAP_NO_SUCH_ATTRIBUTE == compare_nodep->result) {
+                ldc->reason = "Comparison no such attribute (cached)";
+                return compare_nodep->result;
+            }
+            else {
+                ldc->reason = "Comparison undefined (cached)";
+                return compare_nodep->result;
+            }
         }
     }
     /* unlock this read lock */
@@ -685,15 +690,30 @@ start_over:
         ldc->reason = "ldap_compare_s() failed with server down";
         goto start_over;
     }
-  
-    if (result == LDAP_COMPARE_TRUE) {
-        /* compare succeeded; caching result */
+
+    ldc->reason = "Comparison complete";
+    if ((LDAP_COMPARE_TRUE == result) || 
+        (LDAP_COMPARE_FALSE == result) ||
+        (LDAP_NO_SUCH_ATTRIBUTE == result)) {
+        /* compare completed; caching result */
         apr_lock_acquire_rw(util_ldap_cache_lock, APR_WRITER);
         the_compare_node.lastcompare = curtime;
+        the_compare_node.result = result;
         util_ald_cache_insert(curl->compare_cache, &the_compare_node);
         apr_lock_release(util_ldap_cache_lock);
+        if (LDAP_COMPARE_TRUE == result) {
+            ldc->reason = "Comparison true (adding to cache)";
+            return LDAP_COMPARE_TRUE;
+        }
+        else if (LDAP_COMPARE_FALSE == result) {
+            ldc->reason = "Comparison false (adding to cache)";
+            return LDAP_COMPARE_FALSE;
+        }
+        else {
+            ldc->reason = "Comparison no such attribute (adding to cache)";
+            return LDAP_NO_SUCH_ATTRIBUTE;
+        }
     }
-    ldc->reason = "Comparison complete";
     return result;
 }
 
@@ -888,7 +908,7 @@ static const char *util_ldap_set_cache_ttl(cmd_parms *cmd, void *dummy, const ch
         (util_ldap_state_t *)ap_get_module_config(cmd->server->module_config, 
                                                  &ldap_module);
 
-    st->search_cache_ttl = atol(ttl) * 1000;
+    st->search_cache_ttl = atol(ttl) * 1000000;
 
     ap_log_error(APLOG_MARK, APLOG_DEBUG|APLOG_NOERRNO, 0, cmd->server, 
                       "[%d] ldap cache: Setting cache TTL to %ld microseconds.", 
@@ -922,7 +942,7 @@ static const char *util_ldap_set_opcache_ttl(cmd_parms *cmd, void *dummy, const
         (util_ldap_state_t *)ap_get_module_config(cmd->server->module_config, 
                                                  &ldap_module);
 
-    st->compare_cache_ttl = atol(ttl) * 1000;
+    st->compare_cache_ttl = atol(ttl) * 1000000;
 
     ap_log_error(APLOG_MARK, APLOG_DEBUG|APLOG_NOERRNO, 0, cmd->server, 
                       "[%d] ldap cache: Setting operation cache TTL to %ld microseconds.", 
@@ -978,9 +998,9 @@ void *util_ldap_create_config(apr_pool_t *p, server_rec *s)
     st->pool = p;
 
     st->cache_bytes = 100000;
-    st->search_cache_ttl = 600000;
+    st->search_cache_ttl = 600000000;
     st->search_cache_size = 1024;
-    st->compare_cache_ttl = 600000;
+    st->compare_cache_ttl = 600000000;
     st->compare_cache_size = 1024;
 
     st->connections = NULL;

diff --git a/modules/ldap/util_ldap_cache.c b/modules/ldap/util_ldap_cache.c
index 78a06ec17048ced5e61f6ae2e9dc21e4231ee15a..cd4888676e5f27627d5654580a0ba27e258bbdee 100644 (file)
--- a/modules/ldap/util_ldap_cache.c
+++ b/modules/ldap/util_ldap_cache.c
@@ -168,6 +168,7 @@ void *util_ldap_compare_node_copy(void *c)
     node->attrib = util_ald_strdup(n->attrib);
     node->value = util_ald_strdup(n->value);
     node->lastcompare = n->lastcompare;
+    node->result = n->result;
     return node;
 }
 

diff --git a/modules/ldap/util_ldap_cache.h b/modules/ldap/util_ldap_cache.h
index 44fdbd68844e7f041ef28b0d97efabb5c0ab547e..2cedb2f45b27fb34bf7b40a05b74e9aa485f6c8f 100644 (file)
--- a/modules/ldap/util_ldap_cache.h
+++ b/modules/ldap/util_ldap_cache.h
@@ -153,6 +153,7 @@ typedef struct util_compare_node_t {
     const char *attrib;                        
     const char *value;
     apr_time_t lastcompare;
+    int result;
 } util_compare_node_t;
 
 /*