Added some tests for the LDAP connection parameters lookup functionality with the attached patch. It is based off of the tests that were added recently that cover the connection service file libpq functionality as well as the existing ldap test framework.
On 23.03.25 04:05, Andrew Jackson wrote: > > This is the first complaint I can recall hearing about that, so > exactly which ones are "many"? > > I've tested a 2 before figuring out about the v3 issue. lldap[0] and the > docker image osixia/docker-openldap[1]. > - lldap gives the following error message when I attempt to connect > without the patch "Service Error: while handling incoming messages: > while receiving LDAP op: Bind request version is not equal to 3. This is > a serious client bug.". With the attached patch this error message does > not appear > - osixia/docker-openlap gives the following error message without the > patch "67df745e conn=1001 op=0 RESULT tag=97 err=2 text=historical > protocol version requested, use LDAPv3 instead". > " > > > Also, are we really sufficiently compliant with v3 that just adding > this bit is enough? > > I believe that this bit is all that is needed. Per the man page for > ldap_set_option [2]: "The protocol version used by the library defaults > to LDAPv2 (now historic), which corresponds to the LDAP_VERSION2 macro. > Application developers are encouraged to explicitly set > LDAP_OPT_PROTOCOL_VERSION to LDAPv3, using the LDAP_VERSION3 macro, or > to allow users to select the protocol version." > > > src/test/ldap/ doesn't do it for you? > > Looking through the tests here it seems like they are all tests for the > serverside auth functionality that is configurable in pg_hba.conf. I > don't see any tests that test the client side "LDAP Lookup of Connection > Parameters" described in [3]
Ah yes. There are two independent pieces of LDAP functionality. One is the client authentication support in the backend, the other is the connection parameter lookup in libpq. The former does set the LDAP protocol version, the latter does not. This was clearly just forgotten. Your patch makes sense.
