Fred Moyer wrote: > Hi Phillipe :) > > Here is the unit test I was telling you about for the bug I found where > pushing handlers onto a different phase of the request cycle causes the > push to act as a set_handler instead. I dug into the guts of > modperl_handler.c looking for what is causing this but I'm still green > in dealing with the mod_perl internals.
Definitely a bug. Happens not quite a generically than originally thought,
however.
The only time this will be a problem is when you call push_handlers
from a hook that's per-server into a hook that's per-dir (i.e. PostReadRequest
=> Authz).
The attached patch includes your test case, and fixes the problem.
However, it special cases PostReadRequest and needs to be improved to
derive what to do by comparing the context of the current handler vs.
the target handler to push to.
> I'll track you down later and demonstrate this issue.
>
> - Fred
>
> Index: t/hooks/TestHooks/stacked_handlers2.pm
> ===================================================================
> --- t/hooks/TestHooks/stacked_handlers2.pm (revision 584452)
> +++ t/hooks/TestHooks/stacked_handlers2.pm (working copy)
> @@ -45,6 +45,17 @@
> return Apache2::Const::SERVER_ERROR;
> }
>
> +sub push_fixup_handler {
> +
> + my $r = shift;
> +
> + $r->push_handlers(PerlFixupHandler => \&ok);
> +
> + callback($r);
> +
> + return Apache2::Const::OK;
> +}
> +
> sub push_handlers {
>
> my $r = shift;
> @@ -150,7 +161,8 @@
>
> PerlModule TestHooks::stacked_handlers2
>
> - # all 2 run
> + # all 3 run
> + PerlPostReadRequestHandler
> TestHooks::stacked_handlers2::push_fixup_handler
> PerlPostReadRequestHandler TestHooks::stacked_handlers2::ok
> TestHooks::stacked_handlers2::ok
>
> # 1 run, 1 left behind
>
> Index: t/hooks/stacked_handlers2.t
> ===================================================================
> --- t/hooks/stacked_handlers2.t (revision 584452)
> +++ t/hooks/stacked_handlers2.t (working copy)
> @@ -18,7 +18,7 @@
>
> plan tests => 1;
>
> -my $expected = q!ran 2 PerlPostReadRequestHandler handlers
> +my $expected = q!ran 3 PerlPostReadRequestHandler handlers
> ran 1 PerlTransHandler handlers
> ran 1 PerlMapToStorageHandler handlers
> ran 4 PerlHeaderParserHandler handlers
> @@ -26,7 +26,7 @@
> ran 2 PerlAuthenHandler handlers
> ran 2 PerlAuthzHandler handlers
> ran 1 PerlTypeHandler handlers
> -ran 4 PerlFixupHandler handlers
> +ran 3 PerlFixupHandler handlers
> ran 2 PerlResponseHandler handlers
> ran 2 PerlOutputFilterHandler handlers!;
>
--
Philippe M. Chiasson GPG: F9BFE0C2480E7680 1AE53631CB32A107 88C3A5A5
http://gozer.ectoplasm.org/ m/gozer\@(apache|cpan|ectoplasm)\.org/
Index: src/modules/perl/modperl_handler.c
===================================================================
--- src/modules/perl/modperl_handler.c (revision 595352)
+++ src/modules/perl/modperl_handler.c (working copy)
@@ -380,8 +380,13 @@
switch (type) {
case MP_HANDLER_TYPE_PER_DIR:
avp = &dcfg->handlers_per_dir[idx];
+
if (rcfg) {
+ if ((idx == MP_POST_READ_REQUEST_HANDLER) ||
+ (rcfg->current_callback != MP_POST_READ_REQUEST_HANDLER))
+ {
ravp = &rcfg->handlers_per_dir[idx];
+ }
}
set_desc(per_dir);
break;
Index: src/modules/perl/modperl_callback.c
===================================================================
--- src/modules/perl/modperl_callback.c (revision 595352)
+++ src/modules/perl/modperl_callback.c (working copy)
@@ -253,6 +253,7 @@
};
modperl_callback_current_callback_set(desc);
+ if (rcfg) { rcfg->current_callback = idx; }
MP_TRACE_h(MP_FUNC, "[%s] running %d %s handlers\n",
modperl_pid_tid(p), av->nelts, desc);
Index: src/modules/perl/modperl_types.h
===================================================================
--- src/modules/perl/modperl_types.h (revision 595352)
+++ src/modules/perl/modperl_types.h (working copy)
@@ -249,6 +249,7 @@
U8 flags;
int status;
modperl_wbucket_t *wbucket;
+ int current_callback;
MpAV *handlers_per_dir[MP_HANDLER_NUM_PER_DIR];
MpAV *handlers_per_srv[MP_HANDLER_NUM_PER_SRV];
modperl_perl_globals_t perl_globals;
Index: t/hooks/stacked_handlers2.t
===================================================================
--- t/hooks/stacked_handlers2.t (revision 595352)
+++ t/hooks/stacked_handlers2.t (working copy)
@@ -18,7 +18,7 @@
plan tests => 1;
-my $expected = q!ran 2 PerlPostReadRequestHandler handlers
+my $expected = q!ran 3 PerlPostReadRequestHandler handlers
ran 1 PerlTransHandler handlers
ran 1 PerlMapToStorageHandler handlers
ran 4 PerlHeaderParserHandler handlers
Index: t/hooks/TestHooks/stacked_handlers2.pm
===================================================================
--- t/hooks/TestHooks/stacked_handlers2.pm (revision 595352)
+++ t/hooks/TestHooks/stacked_handlers2.pm (working copy)
@@ -31,6 +31,17 @@
return Apache2::Const::DECLINED;
}
+sub push_fixup_handler {
+
+ my $r = shift;
+
+ $r->push_handlers(PerlFixupHandler => \&ok);
+
+ callback($r);
+
+ return Apache2::Const::OK;
+}
+
sub auth_required {
callback(shift);
@@ -150,7 +161,8 @@
PerlModule TestHooks::stacked_handlers2
- # all 2 run
+ # all 3 run
+ PerlPostReadRequestHandler TestHooks::stacked_handlers2::push_fixup_handler
PerlPostReadRequestHandler TestHooks::stacked_handlers2::ok
TestHooks::stacked_handlers2::ok
# 1 run, 1 left behind
signature.asc
Description: OpenPGP digital signature
