On Sun, 26 Jun 2011, Graham Leggett wrote:
On 25 Jun 2011, at 11:24 PM, Stefan Fritsch wrote:
This is not so, to fix this, you would need to wrap every single
LDAP API function call[1] in an optional function, and if you did
that, you would solve the problem that caused you to want to
remove apr_ldap from APR in the first place, making the whole
exercise pointless - you may as well just have fixed apr-ldap in
place.
No, that is not correct. You only need to wrap all ap_ldap_* calls and
make the depending modules (mod_authnz_ldap, mod_vhost_ldap, ...) link
to the ldap libraries themselves.
This is one of the key reasons that got apr_ldap evicted from APR in the
first place.
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.
If the API is not good enough for APR, then the API is not good enough for
httpd. We must fix this problem, not shift it around.
Moving the ldap_init code from mod_ldap into apr with httpd 2.2.x
broke support for Solaris LDAP (PR 42682), which is a regression from
2.0.x. Fixing that would require changes both in mod_ldap and in apr-
ldap. It says a lot that mod_ldap in 2.2.x does not use
apr_ldap_init() in the way it is recommended in the documentation
(setting secure if ldaps is wanted without client certificates).
Because if it did, it would not work, because the apr_ldap code is
buggy.
Looking at this, the fix involves teaching mod_ldap to pass the correct
parameter to apr_ldap_ssl_init(), it doesn't require a change to the API that
I can see.
Changing mod_ldap in this simple way breaks with OpenLDAP. So it's not
that simple, unfortunately. If we make any changes to apr_ldap that
require a different calling order of the functions or change the semantics
in every other way, that's a change in the API. Even if all function
prototypes stay the same.
The long term fix is to hide the LDAP structure within something like
apr_ldap_t, and then lazy init when necessary. This is why
apr_ldap_ssl_init() is marked deprecated in v1.x.
To address the comment in the PR about only supporting openssl, we support
the following distinct LDAP toolkits across a myriad of platforms:
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.
- OpenLDAP
- Solaris
- Novell
- Microsoft
- Netscape
- Mozilla
- Tivoli
- z/OS
Do we still work after the API was moved? For MacOSX at least, no, for the
others, particularly many of the smaller platforms, probably not.
But if we have ap_ldap_init in HTTPD, we can work around changes in
behaviour by modifying how mod_ldap calls the API. This is not possible
with apr_ldap_init because it must continue to work with the unchanged,
released versions of 2.2.x's mod_ldap. Therefore moving the ldap code into
HTTPD gives more flexibility.
If MacOSX is broken, MacOSX users/devs should complain and provide more
info. I couldn't find a mail about this.
But this highlights another problem with how we deal ldap: There is no way
a developer can test changes on all or even on a significant subset of the
supported toolkits. This makes it very hard to do any changes at all.
Ideally we would only support toolkits/plattforms that are available at
the ASF for testing.
Doing these changes in APR 1.x in a way that does not break an
unmodified 2.2.x mod_ldap with some LDAP library is next to
impossible. So moving the apr_ldap stuff as ap_ldap into httpd and
therefore making it possible to change the API is an advantage.
That's incorrect, it's not possible to change the API - when we release httpd
v2.4.0, that's the API baked, and you don't get a chance fix this.
But 2.4.0 is not yet released. Until then we can change things. And if
something breaks, we can fix it in 2.4.1. This is much less severe than if
we break APR and affect all HTTPD 2.2.x user, too.
It is not fair on end users who have waited ages for httpd v2.4.0 to suddenly
force them to wait even longer while we fix the APIs in APR. It is even worse
to expect end users to deal with problems caused by APIs dumped in at the
last minute without a proper stabilisation process before we make a major
release.