hi all...
I'd like to talk about the behavior of the stacked handler mechanism in mp2.
as we all know, handler return codes mean different things for different phases. for instance, returning OK from a fixup handler in Apache (C modules, this is) lets the next fixup run, while returning OK from a trans handler terminates the translation phase. in Apache 2.0, the following "programmable" request phases allow handlers returning OK to end the phase (taken from server/request.c)
AP_IMPLEMENT_HOOK_RUN_FIRST(int,translate_name, (request_rec *r), (r), DECLINED) AP_IMPLEMENT_HOOK_RUN_FIRST(int,check_user_id, (request_rec *r), (r), DECLINED) AP_IMPLEMENT_HOOK_RUN_FIRST(int,type_checker, (request_rec *r), (r), DECLINED) AP_IMPLEMENT_HOOK_RUN_FIRST(int,auth_checker, (request_rec *r), (r), DECLINED)
the content handler (not listed here) also behaves this way.
anyway, the current (mp1 and mp2) stacked handler mechanism does _not_ follow this idiom - any and all configured stacked handlers are run for each phase, regardless of the meaning of that phase to Apache. I think this behavior is wrong and should be fixed in 2.0.
why is it wrong? consider this
PerlAuthenHandler My::Returns::OK My::Returns::AUTH_REQUIRED
in this situation, the first authen handler returns OK, which _should_ mean the user was authenticated and can proceed. indeed, that is what it means to Apache when mod_perl returns OK for the phase. however, with mod_perl handlers the second handler is run and mod_perl returns AUTH_REQUIRED for the entire phase. this is very unDWIMmy.
basically, all the stuff we tell people about "the first trans handler to return OK ends the translation phase" is true, but only if there are no stacked Perl handlers - if there are, the typical advice becomes bogus. this confusioon actually has bitten a few people and has come up on the list several times. Apache::AuthenCache in particular comes to mind, as it needs to jump through lots of hoops in order to Do The Right Thing.
at any rate, I think that blindly running all stacked Perl handlers for each phase is wrong. it does not keep with the nature of what Perl handler writers want to do, namely interact transparently with Apache as though they were writing C handlers. having stacked handlers behave differently in C and Perl is confusing and, as it turns out, unnecessary - while for the first four stages it will probably impact people very little (and only in a good way), the only real value in mp1 in having handlers return OK _and_ running the next handler was with content handlers, but in 2.0 this is what output filters are for :)
+1, good stuff, but see more discussion below
so, what I'm suggesting is that we make the stacked handler mechanism more intelligent and Apache-like, and let the phases behave the same way for Perl handlers as they do for Apache. that is, let handlers returning OK end the phase for translation, authen, authz, type-checking, and content.
Check: http://perl.apache.org/docs/2.0/user/handlers/intro.html#Single_Phase_s_Multiple_Handlers_Behavior there is at least process handler that is of the same kind.
What about http://perl.apache.org/docs/2.0/user/handlers/intro.html#item_VOID
Now we say:
"Handlers of the type VOID will be all executed in the order they have been registered disregarding their return values. Though in mod_perl they are expected to return Apache::OK."
Should we simply call them RUN_ALL?
keep in mind that in doing this we're not really taking away any functionality - handlers (in the 5 phases in question) can still return DECLINED if they want other handlers within the phase to run. all that we would be changing is the behavior of OK in phases that Apache already treats differently.
So what happens to response handlers? Do we just break the mp1 compatibility here, with an excuse that it was broken anyways? I suggest that if we change other phases we change response handlers as well.
anyway, here is a patch (attached due to long lines) to implement what I'm talking about. all tests pass (after some tweaking :) except for the mod_include tests, which use stacked response handlers in a way that we should really be advocating filters for - so, you can either buy into my argument or we can keep response handlers stackable with OK (it only means changing a FALSE to TRUE once the rest of the infrastructure is applied :).
regarding the patch:
Index: lib/ModPerl/Code.pm
===================================================================
RCS file: /home/cvspublic/modperl-2.0/lib/ModPerl/Code.pm,v
retrieving revision 1.100
diff -u -r1.100 Code.pm
--- lib/ModPerl/Code.pm 20 May 2003 01:31:49 -0000 1.100
+++ lib/ModPerl/Code.pm 30 May 2003 17:46:58 -0000
@@ -211,6 +211,15 @@
my($protostr, $pass) = canon_proto($prototype, $name);
my $ix = $self->{handler_index}->{$class}->[$i];
+ if ($callback =~ m/modperl_callback_per_(dir|srv)/) {
+ if ($ix =~ m/AUTH|TYPE|TRANS/) {
what about PerlProcessConnectionHandler? See: http://perl.apache.org/docs/2.0/user/handlers/intro.html#Single_Phase_s_Multiple_Handlers_Behavior
Index: src/modules/perl/mod_perl.c
===================================================================
RCS file: /home/cvspublic/modperl-2.0/src/modules/perl/mod_perl.c,v
retrieving revision 1.172
diff -u -r1.172 mod_perl.c
--- src/modules/perl/mod_perl.c 20 May 2003 06:53:47 -0000 1.172
+++ src/modules/perl/mod_perl.c 30 May 2003 17:46:58 -0000
@@ -774,7 +774,7 @@
modperl_response_init(r);
- retval = modperl_callback_per_dir(MP_RESPONSE_HANDLER, r);
+ retval = modperl_callback_per_dir(MP_RESPONSE_HANDLER, r, FALSE);
I suggest to use enum instead of int for the new argument: run_all like the modperl_handler_action_e type. (enum values being RUN_FIRST/RUN_ALL/VOID to match Apache, we can probably even re-use httpd enum for this purpose if we can).
Even better don't pass it as an argument, but pick it up via
modperl_handler_lookup_handlers, so not to clutter the API with too many arguments. The benefit is that you can encode the RUN_FIRST/RUN_ALL/VOID per handler and not passing them around.
Index: t/hooks/TestHooks/init.pm
===================================================================
RCS file: /home/cvspublic/modperl-2.0/t/hooks/TestHooks/init.pm,v
retrieving revision 1.4
diff -u -r1.4 init.pm
--- t/hooks/TestHooks/init.pm 31 Mar 2003 01:50:52 -0000 1.4
+++ t/hooks/TestHooks/init.pm 30 May 2003 17:46:58 -0000
@@ -35,7 +35,7 @@
$r->notes->set(ok3 => $ok + 1);
- Apache::OK;
+ Apache::DECLINED;
}
sub response {
@@ -59,6 +59,5 @@
PerlModule TestHooks::init
PerlInitHandler TestHooks::init::first
</Base>
-PerlResponseHandler TestHooks::init
-PerlResponseHandler TestHooks::init::response
+PerlResponseHandler TestHooks::init TestHooks::init::response
SetHandler modperl
why changing the config? It was written to test the insertion of two handlers with two directives. Or did you have something special in mind?
Index: t/hooks/TestHooks/stacked_handlers.pm
===================================================================
RCS file: /home/cvspublic/modperl-2.0/t/hooks/TestHooks/stacked_handlers.pm,v
retrieving revision 1.5
diff -u -r1.5 stacked_handlers.pm
--- t/hooks/TestHooks/stacked_handlers.pm 18 Apr 2003 06:18:57 -0000 1.5
+++ t/hooks/TestHooks/stacked_handlers.pm 30 May 2003 17:46:58 -0000
@@ -11,13 +11,13 @@
use Apache::RequestIO ();
use Apache::RequestUtil ();
-use Apache::Const -compile => qw(OK DECLINED DONE);
+use Apache::Const -compile => qw(OK DECLINED DONE AUTH_REQUIRED SERVER_ERROR);
sub handler {
my $r = shift;
- $r->handler("modperl");
- $r->push_handlers(PerlResponseHandler => [\&one, \&two, \&three, \&four]);
+ $r->content_type('text/plain');
+ $r->print(join "\n", @{$r->pnotes('STACKED')});
return Apache::OK;
}
@@ -25,8 +25,7 @@
sub one {
my $r = shift;
- $r->content_type('text/plain');
- $r->print("one\n");
+ $r->pnotes(STACKED => ['one']);
return Apache::OK;
}
@@ -34,36 +33,48 @@
sub two {
my $r = shift;
- $r->print("two\n");
+ push @{$r->pnotes('STACKED')}, 'two';
- return Apache::OK;
+ return Apache::DECLINED;
}
sub three {
my $r = shift;
- $r->print("three\n");
+ push @{$r->pnotes('STACKED')}, 'three';
- return Apache::DONE;
+ return Apache::OK;
}
-# this one shouldn't get called, because the handler 'three' has
-# returned DONE
sub four {
my $r = shift;
- $r->print("four\n");
+ $r->handler('modperl');
+ $r->push_handlers(PerlResponseHandler => [\&handler, \&error]);
return Apache::OK;
}
+sub ok { return Apache::OK }
+sub declined { return Apache::DECLINED }
+sub auth_required { return Apache::AUTH_REQUIRED }
+sub error { return Apache::SERVER_ERROR };
1;
__DATA__
<NoAutoConfig>
+ PerlModule TestHooks::stacked_handlers
+
+ # can't test PerlTransHandler without messing up other tests
+ PerlTypeHandler TestHooks::stacked_handlers::ok TestHooks::stacked_handlers::error
+
<Location /TestHooks__stacked_handlers>
- SetHandler modperl
- PerlHeaderParserHandler TestHooks::stacked_handlers
+ PerlHeaderParserHandler TestHooks::stacked_handlers::one TestHooks::stacked_handlers::two + PerlHeaderParserHandler TestHooks::stacked_handlers::three TestHooks::stacked_handlers::four
+
+ PerlAuthenHandler TestHooks::stacked_handlers::declined TestHooks::stacked_handlers::ok TestHooks::stacked_handlers::auth_required
+ PerlAuthzHandler TestHooks::stacked_handlers::ok TestHooks::stacked_handlers::auth_required
+ require valid-user
</Location>
</NoAutoConfig>
Ahm, perhaps add a new test if you want to put so many new functionality tests inside of it? e.g. you have killed the test for Apache::DONE on the way. I'd rather leave this test alone (with only adjustements fro the new "paradigm") and add a new one to test other things.
__________________________________________________________________ Stas Bekman JAm_pH ------> Just Another mod_perl Hacker http://stason.org/ mod_perl Guide ---> http://perl.apache.org mailto:[EMAIL PROTECTED] http://use.perl.org http://apacheweek.com http://modperlbook.org http://apache.org http://ticketmaster.com
--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
