On Wed, Jul 26, 2017 at 10:40 PM, Ashutosh Bapat <ashutosh.ba...@enterprisedb.com> wrote: > On Wed, Jul 26, 2017 at 6:51 AM, Thomas Munro > <thomas.mu...@enterprisedb.com> wrote: >> Something like the attached. > > The patch prints errdetail() as "No LDAP diagnostic message > available." when LDAP doesn't provide diagnostics. May be some error > messages do not have any diagnostic information. In that case above > error detail may be confusing. May be we should just omit error > details when diagnostic message is not available.
Agreed. Here's a version that skips those useless detail messages using a coding pattern I found elsewhere. For example, on my system, trying to log in with the wrong password causes an "Invalid credentials" error with no extra "DETAIL:" line logged, but trying to use TLS when it hasn't been configured properly logs a helpful diagnostic message. Thanks for the review! I also noticed a couple of other things in passing and fixed them in this new version: 1. In one place we call ldap_get_option(LDAP_OPT_ERROR_NUMBER) after ldap_unbind_s(). My man page says "Once [ldap_unbind()] is called, the connection to the LDAP server is closed, and the ld structure is invalid." So I don't think we should do that, even if it didn't return LDAP_SUCCESS. I have no idea if any implementation would actually fail to unbind and what state the LDAP object would be in if it did: this is essentially the destructor function for LDAP connections, so what are you supposed to do after that? But using the LDAP object again seems wrong to me. 2. In several code paths we don't call ldap_unbind() on the way out, which is technically leaking a connection. Failure to authenticate is FATAL to the backend anyway so it probably doesn't matter much, but hanging up without saying goodbye is considered discourteous by some[1]. Thoughts anyone? [1] https://www.ldap.com/the-ldap-unbind-operation -- Thomas Munro http://www.enterprisedb.com
ldap-diagnostic-message-v2.patch
Description: Binary data
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers