Hi Lubomir, On Friday 12 July 2013, Lubomir Rintel wrote: > I'm have submitted the following-up patches adding SASL > authentication to LDAP modules. Some wise person on an IRC channel > suggested that I'm breaking API and it's a good idea to take this > to the list as an extra work might be needed. > > Would someone be kind enought to take a look at the patches and let > me know what would need to be chaged for them to be accepted into > httpd tree?
if the change should go into 2.4 (as opposed to 2.6 or 3.0), it must not modify the API/ABI in an incompatible way. This means that you may only add new members at the end of public structs (see util_ldap_connection_t), and that you may not remove functions. So you have to keep the old uldap_connection_find() interface available, and add a new uldap_connection_find_ex() interface that supports sasl. Another problem is that you use additional ldap functions like ldap_sasl_interactive_bind_s() that may not be available in some LDAP libraries (I assume you use openldap). The correct way would be to add configure tests for those functions and use #ifdef in the code to ensure that it compiles with other LDAP libraries (or even old versions of openldap). Another issue is the use of a synchronus function ldap_sasl_interactive_bind_s(). We have removed any use of ldap_simple_bind_s() because not all ldap libraries support setting a timeout for that operation, which caused significant problems in practice. Either the code needs to be changed to use ldap_sasl_interactive_bind() instead of the _s function, or it should only be compiled in if LDAP_OPT_TIMEOUT is available. In openldap this has been introduced in version 2.4.4. But given the compatibility problems between ldap libraries, I would lean towards only enabling the new code on openldap >= 2.4.4 anyway, and support other ldap libraries only when they have been actually tested to work. Note that I haven't done an in-depth review of the code and don't have time for that at the moment. In general httpd developers are rather busy. So even if you do the suggested changes, it could take quite some time until the patches get reviewed and included. Cheers, Stefan
