You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by ch...@apache.org on 2008/04/09 19:25:34 UTC
svn commit: r646445 - in /httpd/httpd/trunk/modules/aaa: mod_auth_basic.c
mod_auth_digest.c
Author: chrisd
Date: Wed Apr 9 10:25:33 2008
New Revision: 646445
URL: http://svn.apache.org/viewvc?rev=646445&view=rev
Log:
Let each consumer of authn providers redefine the list_provider_names
callback in case they are loaded individually without mod_authn_core.
Modified:
httpd/httpd/trunk/modules/aaa/mod_auth_basic.c
httpd/httpd/trunk/modules/aaa/mod_auth_digest.c
Modified: httpd/httpd/trunk/modules/aaa/mod_auth_basic.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/aaa/mod_auth_basic.c?rev=646445&r1=646444&r2=646445&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/aaa/mod_auth_basic.c (original)
+++ httpd/httpd/trunk/modules/aaa/mod_auth_basic.c Wed Apr 9 10:25:33 2008
@@ -284,8 +284,15 @@
return OK;
}
+static apr_array_header_t *authn_ap_list_provider_names(apr_pool_t *ptemp)
+{
+ return ap_list_provider_names(ptemp, AUTHN_PROVIDER_GROUP, "0");
+}
+
static void register_hooks(apr_pool_t *p)
{
+ APR_REGISTER_OPTIONAL_FN(authn_ap_list_provider_names);
+
ap_hook_check_authn(authenticate_basic_user, NULL, NULL, APR_HOOK_MIDDLE,
AP_AUTH_INTERNAL_PER_CONF);
}
Modified: httpd/httpd/trunk/modules/aaa/mod_auth_digest.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/aaa/mod_auth_digest.c?rev=646445&r1=646444&r2=646445&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/aaa/mod_auth_digest.c (original)
+++ httpd/httpd/trunk/modules/aaa/mod_auth_digest.c Wed Apr 9 10:25:33 2008
@@ -1961,11 +1961,17 @@
return OK;
}
+static apr_array_header_t *authn_ap_list_provider_names(apr_pool_t *ptemp)
+{
+ return ap_list_provider_names(ptemp, AUTHN_PROVIDER_GROUP, "0");
+}
static void register_hooks(apr_pool_t *p)
{
static const char * const cfgPost[]={ "http_core.c", NULL };
static const char * const parsePre[]={ "mod_proxy.c", NULL };
+
+ APR_REGISTER_OPTIONAL_FN(authn_ap_list_provider_names);
ap_hook_post_config(initialize_module, NULL, cfgPost, APR_HOOK_MIDDLE);
ap_hook_child_init(initialize_child, NULL, NULL, APR_HOOK_MIDDLE);
Re: svn commit: r646445 - in /httpd/httpd/trunk/modules/aaa: mod_auth_basic.c
mod_auth_digest.c
Posted by Graham Leggett <mi...@sharp.fm>.
Chris Darroch wrote:
>> Can you check if mod_auth_form also needs this?
>
> Yes, it does -- looks like you're doing your own ap_lookup_provider()
> calls on the authn provider group.
This was based originally on the mod_auth_basic code. I didn't want to
make a change that I didn't fully understand yet.
> I can put it in for you if you
> like. I'd propose also taking out the AP_MODULE_MAGIC_AT_LEAST(20080403,1)
> check because in trunk you can just call the new function; if you're
> backporting to 2.2 before the "walk cache" stuff gets backported,
> just do it the old way (and don't define the list_provider_names callback).
The reason for the AP_MODULE_MAGIC_AT_LEAST was so that you can drop the
mod_auth_form stuff into httpd v2.2 and have it work (it's how I have
production tested the thing), even if the modules aren't formally
backported to v2.2 yet.
Regards,
Graham
--
Re: svn commit: r646445 - in /httpd/httpd/trunk/modules/aaa:
mod_auth_basic.c mod_auth_digest.c
Posted by Chris Darroch <ch...@pearsoncmg.com>.
Graham Leggett wrote:
>> Let each consumer of authn providers redefine the list_provider_names
>> callback in case they are loaded individually without mod_authn_core.
> Can you check if mod_auth_form also needs this?
Yes, it does -- looks like you're doing your own ap_lookup_provider()
calls on the authn provider group. I can put it in for you if you
like. I'd propose also taking out the AP_MODULE_MAGIC_AT_LEAST(20080403,1)
check because in trunk you can just call the new function; if you're
backporting to 2.2 before the "walk cache" stuff gets backported,
just do it the old way (and don't define the list_provider_names callback).
I'll let the "walk cache" stuff settle a little more but I hope
to propose it for backporting soon (see the relevant patch set under
http://people.apache.org/~chrisd/patches/walk_cache/). It should be
backwards-compatible and provides a performance boost, especially
for mod_dav when running behind authn/z (which it almost always is).
Chris.
--
GPG Key ID: 366A375B
GPG Key Fingerprint: 485E 5041 17E1 E2BB C263 E4DE C8E3 FA36 366A 375B
Re: svn commit: r646445 - in /httpd/httpd/trunk/modules/aaa: mod_auth_basic.c
mod_auth_digest.c
Posted by Graham Leggett <mi...@sharp.fm>.
chrisd@apache.org wrote:
> URL: http://svn.apache.org/viewvc?rev=646445&view=rev
> Log:
> Let each consumer of authn providers redefine the list_provider_names
> callback in case they are loaded individually without mod_authn_core.
>
> Modified:
> httpd/httpd/trunk/modules/aaa/mod_auth_basic.c
> httpd/httpd/trunk/modules/aaa/mod_auth_digest.c
Can you check if mod_auth_form also needs this?
Regards,
Graham
--
Re: svn commit: r646445 - in /httpd/httpd/trunk/modules/aaa: mod_auth_basic.c
mod_auth_digest.c
Posted by Graham Leggett <mi...@sharp.fm>.
chrisd@apache.org wrote:
> URL: http://svn.apache.org/viewvc?rev=646445&view=rev
> Log:
> Let each consumer of authn providers redefine the list_provider_names
> callback in case they are loaded individually without mod_authn_core.
>
> Modified:
> httpd/httpd/trunk/modules/aaa/mod_auth_basic.c
> httpd/httpd/trunk/modules/aaa/mod_auth_digest.c
Can you check if mod_auth_form also needs this?
Regards,
Graham
--
Re: svn commit: r646445 - in /httpd/httpd/trunk/modules/aaa:mod_auth_basic.c
mod_auth_digest.c
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Guenter Knauf wrote:
>
> We recently found that at least for Win32 install we missed to pack a bunch of mod_*.h includes with the distros (Bill fixed this then, and this reminds me that I should do same for NetWare...); however I'm asking me why we dont just put those mod_*.h files which are known to be non-private to one module only into the common ./include dir rather than keeping them in the module's folders?
> Wouldnt that simplify all installs + no need for config.m4 tweaks since the common ./include dir is anyway already with the include path?
unpublished internal API --- modules/foo/foo.h
external API -- include/mod_foo.h
+1
Re: svn commit: r646445 - in
/httpd/httpd/trunk/modules/aaa:mod_auth_basic.c mod_auth_digest.c
Posted by Guenter Knauf <fu...@apache.org>.
Hi Chris,
> True -- an alternative might be to do the following:
> - #define AUTHN/Z_PROVIDER_VERSION "0" in mod_auth.h
> - change all the authn/z modules to use #defines in place of hard-coded
> "0" provider versions (maybe not a bad thing to do anyway)
> - #include mod_auth.h in request.c (mostly involves lots of fiddling
> with config.m4 files and the like, for each platform)
> - eliminate the optional function call entirely
> I worked on this approach but concluded people might not appreciate
> all the changes to the config files and so forth, although the actual
> code per se is a little simpler and cleaner. If this is preferred by
> a majority, though, I'm happy to implement it this way instead. Thoughts?
We recently found that at least for Win32 install we missed to pack a bunch of mod_*.h includes with the distros (Bill fixed this then, and this reminds me that I should do same for NetWare...); however I'm asking me why we dont just put those mod_*.h files which are known to be non-private to one module only into the common ./include dir rather than keeping them in the module's folders?
Wouldnt that simplify all installs + no need for config.m4 tweaks since the common ./include dir is anyway already with the include path?
Any thoughts about such a change?
is there any reason why they should stay in the module folders?
Guenter.
Re: svn commit: r646445 - in
/httpd/httpd/trunk/modules/aaa:mod_auth_basic.c mod_auth_digest.c
Posted by Guenter Knauf <fu...@apache.org>.
Hi,
> Yes, and in fact, there's no need for a function call at all, you
> can just look up the provider list directly in that case. Much cleaner,
> except for all the config magic changes (as discussed by others);
> I can more or less guarantee that I'll break the Windows and/or the
> Netware builds trying to get mod_auth.h to be included. :-) But
> a +1 from me on the idea, if others are willing to do the repair
> work afterwards for those platforms.
no prob for NetWare (I'll change that if we dont move the mod_*.h), so my +1 for this change.
Guenter.
Re: [VOTE] move all mod_*.h with public APIs to ./include folder
Posted by Chris Darroch <ch...@pearsoncmg.com>.
Guenter Knauf wrote:
> in order to simplify future configuration, and most important to
> have same include path structure with both in-tree and installed
> compilations I think it makes sense to move all mod_*.h headers with
> public APIs to the common ./include folder.
+1 since it simplifies my life.
Chris.
--
GPG Key ID: 366A375B
GPG Key Fingerprint: 485E 5041 17E1 E2BB C263 E4DE C8E3 FA36 366A 375B
Re: [VOTE] move all mod_*.h with public APIs to ./include folder
Posted by "Silver W.Zuse" <si...@gmail.com>.
Graham Leggett wrote:
> Guenter Knauf wrote:
>
>> in order to simplify future configuration, and most important to have
>> same include path structure with both in-tree and installed
>> compilations I think it makes sense to move all mod_*.h headers with
>> public APIs to the common ./include folder.
>>
>> Current votes:
>>
>> +1 wrowe, fuankg
>
> +1.
>
> Regards,
> Graham
> --
>
-1
Silver W.Zuse
Re: [VOTE] move all mod_*.h with public APIs to ./include folder
Posted by Graham Leggett <mi...@sharp.fm>.
Guenter Knauf wrote:
> in order to simplify future configuration, and most important to have same include path structure with both in-tree and installed compilations I think it makes sense to move all mod_*.h headers with public APIs to the common ./include folder.
>
> Current votes:
>
> +1 wrowe, fuankg
+1.
Regards,
Graham
--
Re: [VOTE] move all mod_*.h with public APIs to ./include folder
Posted by Graham Dumpleton <gr...@gmail.com>.
2008/4/13 Guenter Knauf <fu...@apache.org>:
> Hi,
>
> > Please specify which headers specifically you consider to be public.
> at least:
>
> mod_cache.h
> mod_core.h
> mod_dav.h
> mod_dbd.h
> mod_proxy.h
> mod_session.h
Also:
mod_auth.h
So it doesn't get missed out of Windows installers like it has in the past.
Graham
Re: [VOTE] move all mod_*.h with public APIs to ./include folder
Posted by Guenter Knauf <fu...@apache.org>.
Hi,
> Please specify which headers specifically you consider to be public.
at least:
mod_cache.h
mod_core.h
mod_dav.h
mod_dbd.h
mod_proxy.h
mod_session.h
> For example, are mod_ssl's headers public?
I'm not sure; mod_ssl.h and a couple of other module headers contain APR_DECLARE_OPTIONAL_FN();
if these are meant to be consumed by other modules (which I think so) then yes.
> what about mod_cache? There are known 3rd party mod_cache backends, yet
> in the past we have 'broken' the API in 2.0.x, because we said they
> weren't public.
It is public since there are CACHE_DECLARE() ap* functions declared which get exported; and these lead to make 3rd-party modules consume them; same goes for mod_proxy.h -> PROXY_DECLARE() and mod_dav.h -> DAV_DECLARE(); mod_auth.h should probably also be included with this list although it doesnt contain any function declarations, but it does contain defines and structs which are used by other modules AFAICT.
Guenter.
Re: [VOTE] move all mod_*.h with public APIs to ./include folder
Posted by Paul Querna <ch...@force-elite.com>.
Guenter Knauf wrote:
> Hi,
> in order to simplify future configuration, and most important to have same include path structure with both in-tree and installed compilations I think it makes sense to move all mod_*.h headers with public APIs to the common ./include folder.
>
> Current votes:
>
> +1 wrowe, fuankg
>
> please vote, and also explain a reason if you vote against.
>
> Guenter.
>
Please specify which headers specifically you consider to be public.
For example, are mod_ssl's headers public?
what about mod_cache? There are known 3rd party mod_cache backends, yet
in the past we have 'broken' the API in 2.0.x, because we said they
weren't public.
I don't think this needs a 'vote' for trunk, just send an email to the
list explaining what modules headers are being made public.
Thanks,
-Paul
Re: [VOTE] move all mod_*.h with public APIs to ./include folder
Posted by Issac Goldstand <ma...@beamartyr.net>.
+1
Guenter Knauf wrote:
> Hi,
> in order to simplify future configuration, and most important to have same include path structure with both in-tree and installed compilations I think it makes sense to move all mod_*.h headers with public APIs to the common ./include folder.
>
> Current votes:
>
> +1 wrowe, fuankg
>
> please vote, and also explain a reason if you vote against.
>
> Guenter.
>
>
Re: [VOTE] move all mod_*.h with public APIs to ./include folder
Posted by Ruediger Pluem <rp...@apache.org>.
On 04/12/2008 12:41 AM, Guenter Knauf wrote:
> Hi, in order to simplify future configuration, and most important to have same include path structure with both
> in-tree and installed compilations I think it makes sense to move all mod_*.h headers with public APIs to the common
> ./include folder.
>
> Current votes:
>
> +1 wrowe, fuankg
>
> please vote, and also explain a reason if you vote against.
Currently I am -1 on this. As I found out with mod_request.h which was recently added
and already stored in the include directory this seems to break our current generation of the
exports.c file (exports.c then includes symbols it shouldn't). If this gets fixed in the same step
as we move the mod_*.h files there I am +1.
Regards
Rüdiger
Re: [VOTE] move all mod_*.h with public APIs to ./include folder
Posted by Issac Goldstand <ma...@beamartyr.net>.
+1
Guenter Knauf wrote:
> Hi,
> in order to simplify future configuration, and most important to have same include path structure with both in-tree and installed compilations I think it makes sense to move all mod_*.h headers with public APIs to the common ./include folder.
>
> Current votes:
>
> +1 wrowe, fuankg
>
> please vote, and also explain a reason if you vote against.
>
> Guenter.
>
>
[VOTE] move all mod_*.h with public APIs to ./include folder
Posted by Guenter Knauf <fu...@apache.org>.
Hi,
in order to simplify future configuration, and most important to have same include path structure with both in-tree and installed compilations I think it makes sense to move all mod_*.h headers with public APIs to the common ./include folder.
Current votes:
+1 wrowe, fuankg
please vote, and also explain a reason if you vote against.
Guenter.
Re: svn commit: r646445 - in /httpd/httpd/trunk/modules/aaa:
mod_auth_basic.c mod_auth_digest.c
Posted by Chris Darroch <ch...@pearsoncmg.com>.
Ruediger Pluem wrote:
> So we are using the optional function only because we do not have the defines
> for AUTHN_PROVIDER_GROUP and the version number around in request.c. If they
> would be around it would be quite fine to call ap_list_provider_names directly
> from there, correct?
Yes, and in fact, there's no need for a function call at all, you
can just look up the provider list directly in that case. Much cleaner,
except for all the config magic changes (as discussed by others);
I can more or less guarantee that I'll break the Windows and/or the
Netware builds trying to get mod_auth.h to be included. :-) But
a +1 from me on the idea, if others are willing to do the repair
work afterwards for those platforms.
Chris.
--
GPG Key ID: 366A375B
GPG Key Fingerprint: 485E 5041 17E1 E2BB C263 E4DE C8E3 FA36 366A 375B
Re: svn commit: r646445 - in /httpd/httpd/trunk/modules/aaa: mod_auth_basic.c
mod_auth_digest.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 11.04.2008 04:00, Chris Darroch wrote:
> Ruediger Pluem wrote:
>
>> The implementation of the function. IMHO it must be the same in all
>> modules. Otherwise it depends on the module load order what gets
>> called and done.
>
> True -- an alternative might be to do the following:
>
> - #define AUTHN/Z_PROVIDER_VERSION "0" in mod_auth.h
> - change all the authn/z modules to use #defines in place of hard-coded
> "0" provider versions (maybe not a bad thing to do anyway)
> - #include mod_auth.h in request.c (mostly involves lots of fiddling
> with config.m4 files and the like, for each platform)
> - eliminate the optional function call entirely
>
> I worked on this approach but concluded people might not appreciate
> all the changes to the config files and so forth, although the actual
> code per se is a little simpler and cleaner. If this is preferred by
> a majority, though, I'm happy to implement it this way instead. Thoughts?
So we are using the optional function only because we do not have the defines
for AUTHN_PROVIDER_GROUP and the version number around in request.c. If they
would be around it would be quite fine to call ap_list_provider_names directly
from there, correct?
Regards
Rüdiger
Re: svn commit: r646445 - in /httpd/httpd/trunk/modules/aaa:
mod_auth_basic.c mod_auth_digest.c
Posted by Chris Darroch <ch...@pearsoncmg.com>.
Ruediger Pluem wrote:
> The implementation of the function. IMHO it must be the same in all
> modules. Otherwise it depends on the module load order what gets
> called and done.
True -- an alternative might be to do the following:
- #define AUTHN/Z_PROVIDER_VERSION "0" in mod_auth.h
- change all the authn/z modules to use #defines in place of hard-coded
"0" provider versions (maybe not a bad thing to do anyway)
- #include mod_auth.h in request.c (mostly involves lots of fiddling
with config.m4 files and the like, for each platform)
- eliminate the optional function call entirely
I worked on this approach but concluded people might not appreciate
all the changes to the config files and so forth, although the actual
code per se is a little simpler and cleaner. If this is preferred by
a majority, though, I'm happy to implement it this way instead. Thoughts?
Chris.
--
GPG Key ID: 366A375B
GPG Key Fingerprint: 485E 5041 17E1 E2BB C263 E4DE C8E3 FA36 366A 375B
Re: svn commit: r646445 - in /httpd/httpd/trunk/modules/aaa: mod_auth_basic.c
mod_auth_digest.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 10.04.2008 21:47, Chris Darroch wrote:
> Ruediger Pluem wrote:
>
>> Lets hope that we never decide to change anything to
>> authn_ap_list_provider_names. Otherwise I bet that we run into
>> inconsistencies which might be hard to track and lead to funny bug
>> reports.
>> So I feel -0.5 on this.
>
> Do you mean changing the function definition, or the provider
> group/version whose providers it returns?
The implementation of the function. IMHO it must be the same in all
modules. Otherwise it depends on the module load order what gets
called and done.
Regards
Rüdiger
Re: svn commit: r646445 - in /httpd/httpd/trunk/modules/aaa:
mod_auth_basic.c mod_auth_digest.c
Posted by Chris Darroch <ch...@pearsoncmg.com>.
Ruediger Pluem wrote:
> Lets hope that we never decide to change anything to
> authn_ap_list_provider_names. Otherwise I bet that we run into inconsistencies
> which might be hard to track and lead to funny bug reports.
> So I feel -0.5 on this.
Do you mean changing the function definition, or the provider
group/version whose providers it returns?
Chris.
--
GPG Key ID: 366A375B
GPG Key Fingerprint: 485E 5041 17E1 E2BB C263 E4DE C8E3 FA36 366A 375B
Re: svn commit: r646445 - in /httpd/httpd/trunk/modules/aaa: mod_auth_basic.c
mod_auth_digest.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 09.04.2008 19:25, chrisd@apache.org wrote:
> Author: chrisd
> Date: Wed Apr 9 10:25:33 2008
> New Revision: 646445
>
> URL: http://svn.apache.org/viewvc?rev=646445&view=rev
> Log:
> Let each consumer of authn providers redefine the list_provider_names
> callback in case they are loaded individually without mod_authn_core.
>
> Modified:
> httpd/httpd/trunk/modules/aaa/mod_auth_basic.c
> httpd/httpd/trunk/modules/aaa/mod_auth_digest.c
>
> Modified: httpd/httpd/trunk/modules/aaa/mod_auth_basic.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/aaa/mod_auth_basic.c?rev=646445&r1=646444&r2=646445&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/aaa/mod_auth_basic.c (original)
> +++ httpd/httpd/trunk/modules/aaa/mod_auth_basic.c Wed Apr 9 10:25:33 2008
> @@ -284,8 +284,15 @@
> return OK;
> }
>
> +static apr_array_header_t *authn_ap_list_provider_names(apr_pool_t *ptemp)
> +{
> + return ap_list_provider_names(ptemp, AUTHN_PROVIDER_GROUP, "0");
> +}
> +
> static void register_hooks(apr_pool_t *p)
> {
> + APR_REGISTER_OPTIONAL_FN(authn_ap_list_provider_names);
> +
> ap_hook_check_authn(authenticate_basic_user, NULL, NULL, APR_HOOK_MIDDLE,
> AP_AUTH_INTERNAL_PER_CONF);
> }
>
> Modified: httpd/httpd/trunk/modules/aaa/mod_auth_digest.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/aaa/mod_auth_digest.c?rev=646445&r1=646444&r2=646445&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/aaa/mod_auth_digest.c (original)
> +++ httpd/httpd/trunk/modules/aaa/mod_auth_digest.c Wed Apr 9 10:25:33 2008
> @@ -1961,11 +1961,17 @@
> return OK;
> }
>
> +static apr_array_header_t *authn_ap_list_provider_names(apr_pool_t *ptemp)
> +{
> + return ap_list_provider_names(ptemp, AUTHN_PROVIDER_GROUP, "0");
> +}
>
> static void register_hooks(apr_pool_t *p)
> {
> static const char * const cfgPost[]={ "http_core.c", NULL };
> static const char * const parsePre[]={ "mod_proxy.c", NULL };
> +
> + APR_REGISTER_OPTIONAL_FN(authn_ap_list_provider_names);
>
> ap_hook_post_config(initialize_module, NULL, cfgPost, APR_HOOK_MIDDLE);
> ap_hook_child_init(initialize_child, NULL, NULL, APR_HOOK_MIDDLE);
Lets hope that we never decide to change anything to
authn_ap_list_provider_names. Otherwise I bet that we run into inconsistencies
which might be hard to track and lead to funny bug reports.
So I feel -0.5 on this.
Regards
Rüdiger