Abhijit Menon-Sen wrote: > I read through the patch, and it looks sensible.
Thanks for the thorough review! > I would have preferred the ldap_simple_bind_s() call in the HAVE_LIBLDAP > branch to not be inside an else {} (the if block above returns if there > is an error anyway), but that's a minor point. I agree, changed. > I tested the code as follows: > > 1. Built the patched source --with-ldap > 2. Set up ~/.pg_service.conf: > > [foo] > > ldap://localhost:3343/cn=dbserver,cn=hosts?pgconnectinfo?base?(objectclass=*) > > ldap://localhost:3443/cn=dbserver,cn=hosts?pgconnectinfo?base?(objectclass=*) > > 3. iptables -A INPUT -p tcp -d 127.0.0.1 --dport 3343 -j DROP > 4. netcat -l 127.0.0.1 3343 ; netcat -l 127.0.0.1 3443 > 5. PGSERVICE=foo bin/psql > > psql did connect to localhost:3443 after a few seconds of trying to > connect to :3343 and failing. (I tried without the iptables rule, so > I know that it does try to connect to both.) > > This doesn't seem to handle timeouts in the sense of a server that > doesn't respond after you connect (or perhaps the timeout was long > enough that it outlasted my patience), but that's not the fault of > this patch, anyway. That's a bug. I thought that setting LDAP_OPT_NETWORK_TIMEOUT would also take care of an unresponsive server, but it seems that I was wrong. The attached patch uses ldap_simple_bind / ldap_result to handle this kind of timeout (like the original code). > I can't say anything about the patch on Windows, but since Magnus seemed > to think it was OK, I'm marking this ready for committer. The Windows part should handle all kinds of timeouts the same. Yours, Laurenz Albe
ldap-bug-3.patch
Description: ldap-bug-3.patch
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers