You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Chris Darroch <ch...@pearsoncmg.com> on 2006/08/29 08:27:47 UTC
mod_authn_dbd fix?
Hi --
I noticed some recent activity in mod_dbd.c to deal with virtual
host configurations, but didn't pay a lot of attention, I confess.
Today I happened to upgrade a system from 2.2.2 to 2.2.3 and discovered
my AuthDBDUserRealmQuery directives now weren't getting inherited from
the main server config down to the virtual hosts the way they used to.
So, I whipped up this patch -- does it make sense to anyone else?
I'll have to be away from the keyboard for a week, so I won't be
committing anything for a while. Maybe if someone else (Nick?) could
noodle on this? Thanks!
Chris.
====================================
--- modules/database/mod_dbd.c.orig 2006-08-29 02:25:54.018358838 -0400
+++ modules/database/mod_dbd.c 2006-08-29 02:16:41.411371370 -0400
@@ -617,10 +617,27 @@
{
svr_cfg *svr;
server_rec *sp;
+ dbd_prepared *main_server_prepared;
for (sp = s; sp; sp = sp->next) {
svr = ap_get_module_config(sp->module_config, &dbd_module);
svr->prepared = apr_hash_get(dbd_prepared_defns, sp->server_hostname,
APR_HASH_KEY_STRING);
+ if (sp == s) {
+ main_server_prepared = svr->prepared;
+ }
+ else if (main_server_prepared != NULL) {
+ if (svr->prepared == NULL) {
+ svr->prepared = main_server_prepared;
+ }
+ else {
+ dbd_prepared *last_prepared = svr->prepared;
+
+ while (last_prepared->next != NULL) {
+ last_prepared = last_prepared->next;
+ }
+ last_prepared->next = main_server_prepared;
+ }
+ }
}
return OK;
}
Re: mod_authn_dbd fix?
Posted by Chris Darroch <ch...@pearsoncmg.com>.
Nick:
> I don't think we ever had that kind of merge. And it also looks like
> possible overspill/pollution, where directives from the main config
> are not necessarily wanted in a vhost.
>
> The complexity here seems to stem from two things. One is
> specifically the treatment of prepared statements. In retrospect,
> this is a poor approach: we should instead have exported an
> optional "dbd_construct" hook which modules could use to
> prepare statements more cleanly. My fault, of course, and I'm going
> to think about whether that can be changed without breaking things.
>
> The other lies in the different configuration hierarchies:
> mod_dbd lives in the server hierarchy, but prepared statements
> commonly apply per-directory. This is falling into a similar trap to
> mod_proxy in 2.0. I *think* that if we can move to a hook, that'll
> go away, if the prepared statements for other modules can be
> taken away from the mod_dbd config.
Just to clarify my particular bug, here's the configuration I'm
working with:
<Location /foo>
AuthDigestProvider dbd
AuthDBDUserRealmQuery "SELECT password FROM users WHERE username = %s"
...
</Location>
<VirtualHost ...>
## no auth settings, but expect to inherit from main server
</VirtualHost>
With the change to mod_dbd in 2.2.3, the auth settings are inherited
by the virtual host, as you'd expect. However, because the prepared
statement isn't inherited any longer, they don't work in the virtual
host. Hence my quick attempt at a patch to ensure that they do:
from the admin's perspective, I think, the fact that there are prepared
statements involved here is invisible, and auth settings should
"just work" when they're inherited by a virtual host. At least,
that was my quick take; sorry, I've been travelling and generally
away from the keyboard lately.
It didn't seem entirely bad to me that mod_dbd would allow
prepared statements to be inherited by virtual hosts, anyway --
isn't that what you'd normally expect with other directives?
Would recognizing a "none" argument to DBDPrepareSQL help, perhaps?
Other directives often use this to allow inherited values to
be overridden and effectively "unset". Like I said, I'm not 100%
on top of this at the moment, but I'm pretty sure that the
DBD auth settings should inherit cleanly with no other action
required by the admin. Thoughts, flames? :-)
Chris.
--
GPG Key ID: 366A375B
GPG Key Fingerprint: 485E 5041 17E1 E2BB C263 E4DE C8E3 FA36 366A 375B
Re: mod_authn_dbd fix?
Posted by Nick Kew <ni...@webthing.com>.
On Tuesday 29 August 2006 07:27, Chris Darroch wrote:
> Hi --
>
> I noticed some recent activity in mod_dbd.c to deal with virtual
> host configurations, but didn't pay a lot of attention, I confess.
> Today I happened to upgrade a system from 2.2.2 to 2.2.3 and discovered
> my AuthDBDUserRealmQuery directives now weren't getting inherited from
> the main server config down to the virtual hosts the way they used to.
http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/database/mod_dbd.c?r1=424798&r2=432562
fixes a problem in 2.2.3
Your patch is not against the same problem, but appears related.
> So, I whipped up this patch -- does it make sense to anyone else?
> svr_cfg *svr;
> server_rec *sp;
> + dbd_prepared *main_server_prepared;
> for (sp = s; sp; sp = sp->next) {
> svr = ap_get_module_config(sp->module_config, &dbd_module);
> svr->prepared = apr_hash_get(dbd_prepared_defns,
> sp->server_hostname, APR_HASH_KEY_STRING);
> + if (sp == s) {
> + main_server_prepared = svr->prepared;
> + }
> + else if (main_server_prepared != NULL) {
> + if (svr->prepared == NULL) {
> + svr->prepared = main_server_prepared;
> + }
That'll do inheritance that should (afaics) be dealt with by merge_config?
> + else {
> + dbd_prepared *last_prepared = svr->prepared;
> +
> + while (last_prepared->next != NULL) {
> + last_prepared = last_prepared->next;
> + }
> + last_prepared->next = main_server_prepared;
> + }
I don't think we ever had that kind of merge. And it also looks like
possible overspill/pollution, where directives from the main config
are not necessarily wanted in a vhost.
The complexity here seems to stem from two things. One is
specifically the treatment of prepared statements. In retrospect,
this is a poor approach: we should instead have exported an
optional "dbd_construct" hook which modules could use to
prepare statements more cleanly. My fault, of course, and I'm going
to think about whether that can be changed without breaking things.
The other lies in the different configuration hierarchies:
mod_dbd lives in the server hierarchy, but prepared statements
commonly apply per-directory. This is falling into a similar trap to
mod_proxy in 2.0. I *think* that if we can move to a hook, that'll
go away, if the prepared statements for other modules can be
taken away from the mod_dbd config.
--
Nick Kew