This as been raised on users@.
---------- Forwarded message ----------
From: Yann Ylavic <ylavic....@gmail.com>
Date: Tue, May 12, 2015 at 10:09 AM
On Mon, May 11, 2015 at 10:54 PM, Michel Stam <mic...@reverze.net> wrote:
>
> I was tinkering over the weekend with mod_authz_dbd and mysql, and i could
> not get a RequireAny/RequireAll to match on multiple Require dbd-group
> statements. It would always match only the last result from the query, but
> once for every row in the resultset.
>
> Example:
> <LocationMatch "/(?<name>[^/]+)/">
> <RequireAny>
> Require user %{env:MATCH_NAME}
> Require dbd-group %{env:MATCH_NAME}
> Require dbd-group Administrators
> </RequireAny>
> </LocationMatch>
>
> After some searching, it appeared to me to be a regression of this:
> https://bz.apache.org/bugzilla/show_bug.cgi?id=46421
The fix mentioned there is about APR's dbd (mysql) code but has never
been pushed to a release (the bugzilla report is still open).
As already discussed in [1] (with a simililar fix for mod_authn_dbd in
[2]), I don't think it should be addressed in APR though (but in httpd
as you and the OP of bugzilla #46421 proposed).
There also seems to be other misuses of apr_dbd_get_entry() returned
values in httpd, I'll start a thread on the dev@ mailing-list and
propose a fix.
[1] http://www.mail-archive.com/dev@apr.apache.org/msg26024.html
[2] http://svn.apache.org/r1663647
---------- End of forwarded message ----------
The issue is that apr_dbd_get_row()'s entries (usually pointed to by
apr_dbd_get_entry(), depending on dbd though) get destroyed whenever
apr_dbd_get_row() returns -1 (no more rows in iterative mode).
This seem to be the case for several dbd systems implemented in APR,
so I think we should take care of the entries' lifetime when used
after an apr_dbd_get_row() loop.
Thus, I think the attached patch should be applied, thoughts?
PS: there are also APR dbd systems where the entries are duplicated on
the apr_dbd_results' pool, so APR is not really consistent...
Index: modules/aaa/mod_authz_dbd.c
===================================================================
--- modules/aaa/mod_authz_dbd.c (revision 1678763)
+++ modules/aaa/mod_authz_dbd.c (working copy)
@@ -174,7 +174,9 @@ static int authz_dbd_login(request_rec *r, authz_d
action, r->user, message?message:noerror);
}
else if (newuri == NULL) {
- newuri = apr_dbd_get_entry(dbd->driver, row, 0);
+ newuri =
+ apr_pstrdup(r->pool,
+ apr_dbd_get_entry(dbd->driver, row, 0));
}
/* we can't break out here or row won't get cleaned up */
}
@@ -204,7 +206,6 @@ static int authz_dbd_group_query(request_rec *r, a
apr_dbd_prepared_t *query;
apr_dbd_results_t *res = NULL;
apr_dbd_row_t *row = NULL;
- const char **group;
if (cfg->query == NULL) {
ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(01649)
@@ -224,8 +225,9 @@ static int authz_dbd_group_query(request_rec *r, a
rv != -1;
rv = apr_dbd_get_row(dbd->driver, r->pool, res, &row, -1)) {
if (rv == 0) {
- group = apr_array_push(groups);
- *group = apr_dbd_get_entry(dbd->driver, row, 0);
+ APR_ARRAY_PUSH(groups, const char *) =
+ apr_pstrdup(r->pool,
+ apr_dbd_get_entry(dbd->driver, row, 0));
}
else {
message = apr_dbd_error(dbd->driver, dbd->handle, rv);
Index: modules/mappers/mod_rewrite.c
===================================================================
--- modules/mappers/mod_rewrite.c (revision 1678763)
+++ modules/mappers/mod_rewrite.c (working copy)
@@ -1384,12 +1384,14 @@ static char *lookup_map_dbd(request_rec *r, char *
while ((rv = apr_dbd_get_row(db->driver, r->pool, res, &row, -1)) == 0) {
++n;
if (ret == NULL) {
- ret = apr_dbd_get_entry(db->driver, row, 0);
+ ret = apr_pstrdup(r->pool,
+ apr_dbd_get_entry(db->driver, row, 0));
}
else {
/* randomise crudely amongst multiple results */
if ((double)rand() < (double)RAND_MAX/(double)n) {
- ret = apr_dbd_get_entry(db->driver, row, 0);
+ ret = apr_pstrdup(r->pool,
+ apr_dbd_get_entry(db->driver, row, 0));
}
}
}
@@ -1402,11 +1404,11 @@ static char *lookup_map_dbd(request_rec *r, char *
case 0:
return NULL;
case 1:
- return apr_pstrdup(r->pool, ret);
+ return ret;
default:
/* what's a fair rewritelog level for this? */
rewritelog((r, 3, NULL, "Multiple values found for %s", key));
- return apr_pstrdup(r->pool, ret);
+ return ret;
}
}
Index: modules/session/mod_session_dbd.c
===================================================================
--- modules/session/mod_session_dbd.c (revision 1678763)
+++ modules/session/mod_session_dbd.c (working copy)
@@ -138,7 +138,8 @@ static apr_status_t dbd_load(request_rec * r, cons
return APR_EGENERAL;
}
if (*val == NULL) {
- *val = apr_dbd_get_entry(dbd->driver, row, 0);
+ *val = apr_pstrdup(r->pool,
+ apr_dbd_get_entry(dbd->driver, row, 0));
}
/* we can't break out here or row won't get cleaned up */
}
