On Sunday 26 June 2011, Graham Leggett wrote: > On 26 Jun 2011, at 3:16 PM, Stefan Fritsch wrote: > > Nobody said that this would be magically fixed by moving the > > stuff to HTTPD. But it means that the apr libraries are no > > longer involved in the mess, which is already very helpful for > > distributions like Debian. With the apr 1.x situation, an ABI > > break (soname change) in openldap means we have to jump through > > hoops to prevent crashes when httpd uses one version of openldap > > and apr uses a different version. > > This is fixed by calling the ldap_get_option() function described > in section 9.2 of > http://www-archive.mozilla.org/directory/ietf-docs/draft-ietf-ldap > ext-ldap-c-api-05.txt . There is no need to move the code to > support this, this can be supported today by adding the > appropriate sanity check on init and failing as appropriate.
This does not help us. We need the information encoded in the package dependencies so that the package manager refuses to install a version of httpd together with a version of apr that don't work together. And I don't think you suggestion would work, at least not without a new API. The problem is this: If apr uses libldap.A and httpd uses libldap.B, httpd will call apr_ldap_init() which makes apr call libldap.A's ldap_init(). Apr then passes the resulting LDAP * pointer to httpd. But httpd will use libldap.B's other ldap_* functions on the object created by libldap.A. This usually causes segfaults, but it may also cause more subtle bugs. To detect this issue, both httpd and apr would need to call ldap_get_option() and httpd would need to compare the versions returned by both calls. > > I think you misread that comment. The patch attached to the PR > > makes it work with Solaris LDAP but breaks with OpenLDAP, > > therefore it is not acceptable. > > Looking at the patch in more detail, the patch makes indiscriminate > attempts to change the behaviour of all the platforms at the same > time, instead of targeting the Solaris platform only. OpenLDAP will > break by definition in this case, as will other platforms. > > What the patch needs to do is properly isolate the functions being > used on Solaris (ldapssl_init()? ldap_sslint()? something else?), > and modify them only, using #ifdef where appropriate. The problem is that we also need to modify mod_ldap. If we want true isolation, we would also get #ifdef APR_HAS_SOLARIS_LDAPSDK sections in mod_ldap, which is what what apr_ldap tried to avoid in the first place.
