Merge r591499, r593919 from trunk:

spurious 401s with message "DN has not been defined" when cache expiration happens in another thread

PR 43786

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.2.x@595675 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/CHANGES b/CHANGES
index cdb86bdb5013e5cbac8ea34fa968b98bd64a73c6..c09c827154bb477d7286b76e4b56e4c803ac8c67 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,10 @@
                                                         -*- coding: utf-8 -*-
 Changes with Apache 2.2.7
 
+  *) mod_ldap: Give callers a reference to data copied into the request
+     pool instead of references directly into the cache
+     PR 43786 [Eric Covener]
+    
   *) mod_ldap: Stop passing a reference to pconf around for
      (limited) use during request processing, avoiding possible 
      memory corruption and crashes.  [Eric Covener]

diff --git a/STATUS b/STATUS
index c45abf445b194a25411811803bb31af9ca3a1d52..650573473f17b435ae4db76822312487cdc4cdee 100644 (file)
--- a/STATUS
+++ b/STATUS
@@ -79,18 +79,6 @@ RELEASE SHOWSTOPPERS:
 PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
   [ start all new proposals below, under PATCHES PROPOSED. ]
 
-   * mod_ldap: Don't return references into shared memory to the caller, 
-     as these may expire at any time because callers don't hold
-     a cache lock
-         http://svn.apache.org/viewvc?view=rev&revision=591499
-         http://svn.apache.org/viewvc?view=rev&revision=593919 
-     +1: covener, rpluem, rederpj
-     rederpj: Though it should never be a problem (famous last words), should
-              there be some sort of verification of i vs. k? (since you
-              allocate based on k and copy based on i)
-     covener: attrs/vals are defined as being the same length and null terminated,
-              we just need to count the length of one to allocate the other
-
 PATCHES PROPOSED TO BACKPORT FROM TRUNK:
   [ New proposals should be added at the end of the list ]
 

diff --git a/modules/ldap/util_ldap.c b/modules/ldap/util_ldap.c
index 1d9195ed68a12db163bc81adc869e50e38d62b83..2cce2348148a7d9c714d2f343cdc71364344fbda 100644 (file)
--- a/modules/ldap/util_ldap.c
+++ b/modules/ldap/util_ldap.c
@@ -921,8 +921,16 @@ static int uldap_cache_checkuserid(request_rec *r, util_ldap_connection_t *ldc,
                      && (strcmp(search_nodep->bindpw, bindpw) == 0))
             {
                 /* ...and entry is valid */
-                *binddn = search_nodep->dn;
-                *retvals = search_nodep->vals;
+                *binddn = apr_pstrdup(r->pool, search_nodep->dn);
+                if (attrs) {
+                    int i = 0, k = 0;
+                    while (attrs[k++]);
+                    *retvals = apr_pcalloc(r->pool, sizeof(char *) * k);
+                    while (search_nodep->vals[i]) {
+                        (*retvals)[i] = apr_pstrdup(r->pool, search_nodep->vals[i]);
+                        i++;
+                    }
+                }
                 LDAP_CACHE_UNLOCK();
                 ldc->reason = "Authentication successful (cached)";
                 return LDAP_SUCCESS;
@@ -1161,8 +1169,16 @@ static int uldap_cache_getuserdn(request_rec *r, util_ldap_connection_t *ldc,
             }
             else {
                 /* ...and entry is valid */
-                *binddn = search_nodep->dn;
-                *retvals = search_nodep->vals;
+                *binddn = apr_pstrdup(r->pool, search_nodep->dn);
+                if (attrs) {
+                    int i = 0, k = 0;
+                    while (attrs[k++]);
+                    *retvals = apr_pcalloc(r->pool, sizeof(char *) * k);
+                    while (search_nodep->vals[i]) {
+                        (*retvals)[i] = apr_pstrdup(r->pool, search_nodep->vals[i]);
+                        i++;
+                    }
+                }
                 LDAP_CACHE_UNLOCK();
                 ldc->reason = "Search successful (cached)";
                 return LDAP_SUCCESS;