(CC'ing Michel, sorry for the resend, my initial omission)
On Tue, May 12, 2015 at 10:41 AM, Yann Ylavic <ylavic....@gmail.com> wrote:
> 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 */
}
