httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stefan Fritsch ...@sfritsch.de>
Subject Re: load order dependency between mod_ldap and mod_authnz_ldap
Date Sun, 26 Jun 2011 13:16:05 GMT
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.

Mime
View raw message