Re: LDAP: bugfix and deprecated OpenLDAP API

On Mon, Jul 1, 2013 at 4:16 PM, Albe Laurenz <laurenz(dot)albe(at)wien(dot)gv(dot)at> wrote:
> Magnus Hagander wrote:
>> On Tue, Feb 5, 2013 at 10:39 AM, Albe Laurenz <laurenz(dot)albe(at)wien(dot)gv(dot)at> wrote:
>>> I found a small bug in the implementation of LDAP connection
>>> parameter lookup.
>
> [...]
>
>>> As coded now, the timeout won't work - if the LDAP server
>>> is down, ldap_simple_bind will wait for the network
>>> timeout, which will be quite longer than 2 seconds.
>>>
>>> The attached patch ldap-bug.patch fixes this problem;
>>> unfortunately I found no way that works both with OpenLDAP
>>> and Windows LDAP, so I had to add an #ifdef.
>>>
>>> I think that this patch should be applied and backpatched.
>>
>> So just to be clear - the difference is we're going from implicit
>> anonymous bind, to an explicit one? We're not actually causing an
>> extra bind compared to previous versions?
>
> No, it was an explicit bind before as well.

Ah, got it.

In that case, doesn't this patch break Windows? We no longer do the
anonymous bind on Windows, since it's now in the #ifdef HAVE_LIBLDAP.

Don't we need to keep the ldap_simple_bind() call in the Windows case,
or break it up so the call to ldap_sasl_bind_s() is moved outside the
#ifdef? At least I can't find anything in the docs that indicate that
ldap_connect() on Windows would actually call that for us - only the
other way around?

> I'll be on vacation from Wednesday on until July 20th.

Sorry I couldn't get back to you on that one earlier.

I'm going to set this patch as returned with feedback for now, but
please feel free to comment on above and possibly resubmit if
necessary before the CF and I'll see if I can deal with it before the
next CF anyway, as it's a bug fix.

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/