Re: load order dependency between mod_ldap and mod_authnz_ldap
On Sat, Jun 25, 2011 at 10:11:20PM +0200, Graham Leggett wrote: On 06 Jun 2011, at 11:53 PM, William A. Rowe Jr. wrote: Since the move from apr-util-ldap to ap_ldap, mod_ldap needs to be loaded before mod_authnz_ldap. This is somewhat annoying because the default httpd.conf tries to load mod_authnz_ldap first. Any ideas how to fix this or do we just change the order in the default httpd.conf? I believe the entire fix may be an entry point to apr_ldap_parse_uri (check your own binaries to confirm). Setting up a single entry point should be trivial, if its appropriate. 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. mod_authnz_ldap is an existence proof that we don't need to wrap every LDAP API function call for mod_ldap to be useful in its current form, surely? In it's current form, this change introduces module ordering bugs to httpd that we haven't suffered for a decade. Fixed in r1140069. In addition, the autoconf build is currently broken against apr v1.x on MacOSX, and this is probably broken on other platforms as well. Can you give details on this? The timing cannot be worse This is probably true, but it doesn't seem like technical justification for a veto. Release schedules can move at the pace we want them to. There is no need for this move at all This is strictly true, in the sense that there may be other ways to solve the problems fixed by moving the code. If other solutions are presented, we can have a consensus vote on which to choose. In the absence of other solutions being implemented, asserting that better solutions exist can never be sufficient reason to veto a change. Regards, Joe
Re: svn commit: r916377 - in /httpd/httpd/trunk: CHANGES docs/manual/programs/rotatelogs.xml support/rotatelogs.c
On Mon, Jun 20, 2011 at 04:14:10PM +0200, Graham Leggett wrote: On 20 Jun 2011, at 12:58 PM, Plüm, Rüdiger, VF-Group wrote: more general -p mode just added - is it worth keeping? I think it is worth keeping for those people that only need the link. Creating a post rotation script that does this seems to be a little bit of overkill in this case. +1. OK, fair enough - thanks guys. I've fixed the error case and simplified the code a little in r1140138. Regards, Joe
Re: load order dependency between mod_ldap and mod_authnz_ldap
On 27 Jun 2011, at 12:28 PM, Joe Orton 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. mod_authnz_ldap is an existence proof that we don't need to wrap every LDAP API function call for mod_ldap to be useful in its current form, surely? mod_ldap - An LDAP shared memory cache mod_authnz_ldap - A user of the LDAP shared memory cache The LDAP API exposes way more functionality than mod_ldap exposes, so while you may have fixed the problem for the special case that is mod_authnz_ldap, you won't have fixed the problem for any other module that makes LDAP calls directly. The v1.x LDAP API was removed from APR because it was an incomplete abstraction, and for this exact same set of reasons it is inappropriate to add it to httpd. Over and above that, given that httpd has no library component common to all modules (well, it has, it's called APR), and given that the httpd core is inappropriate for an LDAP portability library, the only place to put the LDAP bindings is in mod_ldap, which means we either have to accept the module ordering problems that result, or we have to create optional functions for every single LDAP API call, and if we do that, we end up solving the reason apr_ldap was removed from APR in the first place. In addition, we moved all portability code out of httpd in v2.0, it is a huge step backwards to go back on that effort. It is really ugly to have apr_xml, apr_dbd, apr_crypto and apr_dbm in APR, and then ap(r)_ldap somewhere else. This is the essence of my veto. The timing cannot be worse This is probably true, but it doesn't seem like technical justification for a veto. Release schedules can move at the pace we want them to. This was an observation, not part of the veto. It's important because there are people who are really keen to get v2.4 out the door, and this means practically is that a suboptimal API is being rushed unnecessarily into GA, and that is really really bad. There is no need for this move at all This is strictly true, in the sense that there may be other ways to solve the problems fixed by moving the code. If other solutions are presented, we can have a consensus vote on which to choose. In the absence of other solutions being implemented, asserting that better solutions exist can never be sufficient reason to veto a change. The solution we agreed on in the past was to rewrite apr_ldap to fit the pattern of apr_dbd, but this effort was held up by a whole lot of problems in the apr_dbd API that were identified during review of the apr_crypto API, which was modeled on the apr_dbd API. This effort was also held up by the focus on getting httpd v2.4 out the door. wrowe got impatient and removed apr_ldap from apr-trunk, which on the face of it isn't a problem for httpd because apr-trunk isn't a released entity yet, nor is it likely to be until the issues with apr_dbd (and as I understand apr_dbm) are fixed (among other issues). This move just forces us to solve the apr_ldap issue before apr v2.0 is released. It doesn't however put httpd under any obligation to take any drastic measures for httpd v2.4, because the existing apr_ldap library exists and is supported in apr v1.x. In short, when httpd v2.4 is released, we will be free to focus our attention on apr_ldap and get it fixed. There is no need for any temporary hacks before then (where temporary is defined as permanently baked into the httpd v2.4 API). Regards, Graham --
TR of httpd-2.3.13-beta on June 28 (Tues)
Subj sez it all...
Re: load order dependency between mod_ldap and mod_authnz_ldap
On 6/27/2011 8:19 AM, Graham Leggett wrote: mod_ldap - An LDAP shared memory cache mod_authnz_ldap - A user of the LDAP shared memory cache The LDAP API exposes way more functionality than mod_ldap exposes, so while you may have fixed the problem for the special case that is mod_authnz_ldap, you won't have fixed the problem for any other module that makes LDAP calls directly. The v1.x LDAP API was removed from APR because it was an incomplete abstraction, and for this exact same set of reasons it is inappropriate to add it to httpd. Over and above that, given that httpd has no library component common to all modules (well, it has, it's called APR), and given that the httpd core is inappropriate for an LDAP portability library, the only place to put the LDAP bindings is in mod_ldap, which means we either have to accept the module ordering problems that result, or we have to create optional functions for every single LDAP API call, and if we do that, we end up solving the reason apr_ldap was removed from APR in the first place. This is precisely the reason that you are wrong. Only mod_ldap and mod_authnz_ldap (and apparently mod_vhost_ldap) need functionality of these helpers (hard to call it an API, since it doesn't actually expose LDAP functionality). If no other module of, nor httpd itself, needs ldap then none should bind to ldap libraries. Of course, following Joe's commit, we no longer have the module load order issue (thanks Joe! I'll review win32 within three days per Gregg's notes). But you can't veto the status quo. Today, mod_ldap and mod_authnz_ldap already must bind to the ldap provider. They are consumers of the stub functions And I'd nix your definition of mod_ldap... if it an ldap shared cache provider then it should have been suitably named. One can omit such a feature and still use mod_authnz_ldap. Of course, that was not possible because mod_authnz_ldap required mod_ldap required the ldap helpers required an ldap client library. This was an observation, not part of the veto. It's important because there are people who are really keen to get v2.4 out the door, and this means practically is that a suboptimal API is being rushed unnecessarily into GA, and that is really really bad. Suboptimal? Sure. But IJFW, which has been the critera for httpd for as long as I've been here. Not acceptable at APR, but just fine, here. The solution we agreed on in the past was to rewrite apr_ldap to fit the pattern of apr_dbd, but this effort was held up by a whole lot of problems in the apr_dbd API that were identified during review of the apr_crypto API, which was modeled on the apr_dbd API. This effort was also held up by the focus on getting httpd v2.4 out the door. No. We didn't agree upon a solution. We agreed upon a design pattern. But that was never implemented, ergo never solved, ergo it's vapor. wrowe got impatient and removed apr_ldap from apr-trunk, which on the face of it isn't a problem for httpd because apr-trunk isn't a released entity yet, nor is it likely to be until the issues with apr_dbd (and as I understand apr_dbm) are fixed (among other issues). This move just forces us to solve the apr_ldap issue before apr v2.0 is released. It doesn't however put httpd under any obligation to take any drastic measures for httpd v2.4, because the existing apr_ldap library exists and is supported in apr v1.x. You are absolutely right, httpd is under no obligation to support APR 2.0, or APR 1.x, or pocore, or any other library. It is an independent project. In short, when httpd v2.4 is released, we will be free to focus our attention on apr_ldap and get it fixed. There is no need for any temporary hacks before then (where temporary is defined as permanently baked into the httpd v2.4 API). I have no expectation that it will be fixed, and the majority voted at APR to evict apr_ldap, therefore it is gone (not be veto or fiat, but by the preference of the majority). I have reacted accordingly because httpd would not have been happy if it had abandoned these helpers and they were not present for httpd's consumption. On the assumption that APR and httpd are now distinct, that httpd will not require httpd-deps to run, and that httpd will hopefully consume apr 1.x or apr 2.0 once it is released, this solution is correct for the short term health of httpd. httpd and apr major.minor lifespans have been measured in decades (.5 .. 1.5) and it would be foolish to ship something another project has abandoned, which was the basis of questioning if expat was the right choice going forwards. With the addition of libxml2 support it is no longer a question or risk. apr_ldap was a risk, and this change proposed to fix it. I agree with Joe's work to complete this, and it is not a veto'able change for reasons rehashed on two dev lists. Call a vote to reject providing ap_ldap in httpd 2.4, if you like. Because
Re: load order dependency between mod_ldap and mod_authnz_ldap
On 6/27/2011 9:04 AM, William A. Rowe Jr. wrote: Of course, following Joe's commit, we no longer have the module load order issue (thanks Joe! I'll review win32 within three days per Gregg's notes). Just read Jim's post, which sounds great. I'll get to this tonight.
Re: http_protocol.h, ap_rputs inline spells disaster on MSVC
Hi Gregg, On Monday 27 June 2011, Gregg L. Smith wrote: looking at the submitted by in r1131465 this is a part of all the code cleanup I have been seeing in bugzilla of late. It seems however that inlining ap_rputs is causing problems on MSVC. I am using VC9 and here is what I am seeing everywhere the header is included. please check if r1140249 fixes it. Bill and Joe have also made commits which should fix the other two issues you pointed out (thanks!). Cheers, Stefan
Re: load order dependency between mod_ldap and mod_authnz_ldap
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.
Re: Re: http_protocol.h, ap_rputs inline spells disaster on MSVC
Hi Stefan, Yes, switching to APR_INLINE did the trick, thanks. Cheers, Gregg -Original Message- From: Stefan Fritsch s...@sfritsch.de To: dev@httpd.apache.org Date: Mon, 27 Jun 2011 19:57:50 +0200 Subject: Re: http_protocol.h, ap_rputs inline spells disaster on MSVC Hi Gregg, On Monday 27 June 2011, Gregg L. Smith wrote: looking at the submitted by in r1131465 this is a part of all the code cleanup I have been seeing in bugzilla of late. It seems however that inlining ap_rputs is causing problems on MSVC. I am using VC9 and here is what I am seeing everywhere the header is included. please check if r1140249 fixes it. Bill and Joe have also made commits which should fix the other two issues you pointed out (thanks!). Cheers, Stefan
