I'm not sure that I understand what the problem is. By updating the binddn/bindpw in the connection record, we always know exactly which userid the connection is bound to. During the util_ldap_connection_find() routine there are two searches performed. The first search looks for a connection within the cache that exactly matches the connection credentials that were passed in. If a connection is found, it is used without the overhead of having to rebind the connection. If an exact connection is not found, then a second search is performed looking for a connection that matches the connection attributes but not the credentials. If a connection is found that matches the connection attributes then the l->bound flag is turned off which forces the connection to be rebound to the correct user during the call to util_ldap_conection_open() routine. The advantage to this is avoiding unnecessary rebinds when the credential caching is turned off. The patch you submitted rebinds the connection always whether it needs to be or not. But as a result, I did find one problem with the current code that does have an affect when a bind fails during the util_ldap_cache_checkuserid() processing. If the bind fails while validating the user credentials then the connection is left in an unbound state although the connection record still indicates that the connection is bound to a specific user. This could cause problems for a subsequent ldap search. This patch unbinds the connection which will force it to be reinitialized the next time that it is used.
Brad Brad Nicholes Senior Software Engineer Novell, Inc., the leading provider of Net business solutions http://www.novell.com >>> Denis Gervalle <[EMAIL PROTECTED]> Monday, May 10, 2004 3:33:08 PM >>> Brad, After a second look to util_ldap patches, I notice that my original mail is already about r1.22 which only solve the problem half way. I have tried to held your attention on that problem and you seems to have missed the point. Patch 1.22 has still a serious flaw when a wrong binddn/bindpw are provided. IMHO, my patch proposal (Bug 27134), which delegate all binding operation to the single util_ldap_connection_open function, avoids all possible trap that could messed up the connection cache now and in the future. Hopeless, it is now no more applicable as-is to the cvs tree. However, my patch has been test against version 1.24 from the cvs. Albert Lunde who did these tests have provided his conclusions under the same bug number (Bug 27134). These results (for which he send me a detailled report) shown that version 1.24 (which obviously include 1.22 patch) is entirely unreliable. We are still in close relation about these problems, and he recently confirm me that my proposal provide him good results with a number of connections proportional (as expected) with the number of process involved in prefork mode. Hope that you will have a closer look to my patch this time. Do not hesitate too contact me if you need more information. Best regards, Denis Gervalle SOFTEC sa http://www.softec.st Brad Nicholes wrote: > There are still 2 more patches for util_ldap.c that are proposed for >backport to the 2.0 tree. These are r1.22 and r1.24. These 2 patches >should fix the problem that you discribed. Once the backport proposal >recieves a third +1 vote, I will apply them to the 2.0 tree. > >Brad > >Brad Nicholes >Senior Software Engineer >Novell, Inc., the leading provider of Net business solutions >http://www.novell.com > > > >>>>Denis Gervalle <[EMAIL PROTECTED]> Tuesday, April 20, 2004 4:04:47 PM >>>> >>>> >>>> >Brad, > >May I suggest you to have a look at bug 27134 that provide a patch >regarding the rebinding of the connection during >util_ldap_cache_checkuserid(). >The recent correction you provide in the CVS of apache 2.0 has a flaw >when the binding fails (because of wrong binddn or password). In that >case, the util_ldap_connection_open will not repair the connection >since >it will appear to be still bound. In my patch proposal, I prefer to >delegate the bind to util_ldap_connection_open which make it the only >place for connection binding. It also take care of retries in case of >server down. >Hope this helps, >Regards, > >Denis Gervalle >SOFTEC > >
util_ldap.c.patch1
Description: Binary data
