Return-Path: Delivered-To: apmail-perl-dev-archive@perl.apache.org Received: (qmail 82424 invoked by uid 500); 30 May 2003 17:55:09 -0000 Mailing-List: contact dev-help@perl.apache.org; run by ezmlm Precedence: bulk list-help: list-unsubscribe: list-post: Delivered-To: mailing list dev@perl.apache.org Received: (qmail 82276 invoked from network); 30 May 2003 17:55:08 -0000 Received: from unknown (HELO secure.exclamationlabs.net) (66.77.29.176) by daedalus.apache.org with SMTP; 30 May 2003 17:55:08 -0000 Received: from modperlcookbook.org (ham-nat.covad.net [68.165.151.221]) (authenticated (0 bits)) by secure.exclamationlabs.net (8.11.6/8.11.6) with ESMTP id h4UHtB413458 for ; Fri, 30 May 2003 12:55:11 -0500 Message-ID: <3ED79AFE.4000900@modperlcookbook.org> Date: Fri, 30 May 2003 13:55:10 -0400 From: Geoffrey Young User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.0.1) Gecko/20020823 Netscape/7.0 X-Accept-Language: en-us, en MIME-Version: 1.0 To: dev@perl.apache.org Subject: a new stacked handlers paradigm Content-Type: multipart/mixed; boundary="------------070100090004060304070206" X-Spam-Rating: daedalus.apache.org 1.6.2 0/1000/N --------------070100090004060304070206 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit 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 :) 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. 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. 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 :). discussion welcome. --Geoff --------------070100090004060304070206 Content-Type: text/plain; name="stacked.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="stacked.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/) { + $pass .= ', FALSE'; + } + else { + $pass .= ', TRUE'; + } + } + print $h_fh "\n$protostr;\n"; print $c_fh <content_type) { r->handler = r->content_type; /* let http_core or whatever try */ Index: src/modules/perl/modperl_callback.c =================================================================== RCS file: /home/cvspublic/modperl-2.0/src/modules/perl/modperl_callback.c,v retrieving revision 1.54 diff -u -r1.54 modperl_callback.c --- src/modules/perl/modperl_callback.c 17 Feb 2003 09:03:16 -0000 1.54 +++ src/modules/perl/modperl_callback.c 30 May 2003 17:46:58 -0000 @@ -81,7 +81,8 @@ request_rec *r, conn_rec *c, server_rec *s, apr_pool_t *pconf, apr_pool_t *plog, - apr_pool_t *ptemp) + apr_pool_t *ptemp, + int run_all) { #ifdef USE_ITHREADS pTHX; @@ -192,8 +193,21 @@ status = modperl_errsv(aTHX_ status, r, s); #ifdef MP_TRACE if (i+1 != nelts) { - MP_TRACE_h(MP_FUNC, "there were %d uncalled handlers\n", - nelts-i-1); + MP_TRACE_h(MP_FUNC, "error status %d leaves %d uncalled handlers\n", + status, desc, nelts-i-1); + } +#endif + break; + } + + /* follow Apache's lead and let OK terminate the phase for + * translation, authen, authz, type-checking, and content + */ + if (status == OK && run_all == FALSE) { +#ifdef MP_TRACE + if (i+1 != nelts) { + MP_TRACE_h(MP_FUNC, "OK ends the %s stack, leaving %d uncalled handlers\n", + desc, nelts-i-1); } #endif break; @@ -213,39 +227,39 @@ return status; } -int modperl_callback_per_dir(int idx, request_rec *r) +int modperl_callback_per_dir(int idx, request_rec *r, int run_all) { return modperl_callback_run_handlers(idx, MP_HANDLER_TYPE_PER_DIR, r, NULL, r->server, - NULL, NULL, NULL); + NULL, NULL, NULL, run_all); } -int modperl_callback_per_srv(int idx, request_rec *r) +int modperl_callback_per_srv(int idx, request_rec *r, int run_all) { return modperl_callback_run_handlers(idx, MP_HANDLER_TYPE_PER_SRV, r, NULL, r->server, - NULL, NULL, NULL); + NULL, NULL, NULL, run_all); } int modperl_callback_connection(int idx, conn_rec *c) { return modperl_callback_run_handlers(idx, MP_HANDLER_TYPE_CONNECTION, NULL, c, c->base_server, - NULL, NULL, NULL); + NULL, NULL, NULL, TRUE); } int modperl_callback_pre_connection(int idx, conn_rec *c, void *csd) { return modperl_callback_run_handlers(idx, MP_HANDLER_TYPE_PRE_CONNECTION, NULL, c, c->base_server, - NULL, NULL, NULL); + NULL, NULL, NULL, TRUE); } void modperl_callback_process(int idx, apr_pool_t *p, server_rec *s) { modperl_callback_run_handlers(idx, MP_HANDLER_TYPE_PROCESS, NULL, NULL, s, - p, NULL, NULL); + p, NULL, NULL, TRUE); } int modperl_callback_files(int idx, @@ -254,5 +268,5 @@ { return modperl_callback_run_handlers(idx, MP_HANDLER_TYPE_FILES, NULL, NULL, s, - pconf, plog, ptemp); + pconf, plog, ptemp, TRUE); } Index: src/modules/perl/modperl_callback.h =================================================================== RCS file: /home/cvspublic/modperl-2.0/src/modules/perl/modperl_callback.h,v retrieving revision 1.22 diff -u -r1.22 modperl_callback.h --- src/modules/perl/modperl_callback.h 17 Feb 2003 09:03:16 -0000 1.22 +++ src/modules/perl/modperl_callback.h 30 May 2003 17:46:58 -0000 @@ -26,11 +26,12 @@ request_rec *r, conn_rec *c, server_rec *s, apr_pool_t *pconf, apr_pool_t *plog, - apr_pool_t *ptemp); + apr_pool_t *ptemp, + int run_all); -int modperl_callback_per_dir(int idx, request_rec *r); +int modperl_callback_per_dir(int idx, request_rec *r, int run_all); -int modperl_callback_per_srv(int idx, request_rec *r); +int modperl_callback_per_srv(int idx, request_rec *r, int run_all); int modperl_callback_connection(int idx, conn_rec *c); Index: src/modules/perl/modperl_config.c =================================================================== RCS file: /home/cvspublic/modperl-2.0/src/modules/perl/modperl_config.c,v retrieving revision 1.63 diff -u -r1.63 modperl_config.c --- src/modules/perl/modperl_config.c 20 May 2003 06:02:47 -0000 1.63 +++ src/modules/perl/modperl_config.c 30 May 2003 17:46:58 -0000 @@ -290,7 +290,7 @@ rcfg->pnotes = Nullhv; } - retval = modperl_callback_per_dir(MP_CLEANUP_HANDLER, r); + retval = modperl_callback_per_dir(MP_CLEANUP_HANDLER, r, TRUE); return retval; } Index: t/hooks/stacked_handlers.t =================================================================== RCS file: /home/cvspublic/modperl-2.0/t/hooks/stacked_handlers.t,v retrieving revision 1.2 diff -u -r1.2 stacked_handlers.t --- t/hooks/stacked_handlers.t 18 Apr 2003 06:18:57 -0000 1.2 +++ t/hooks/stacked_handlers.t 30 May 2003 17:46:58 -0000 @@ -8,7 +8,7 @@ plan tests => 1; my $location = "/TestHooks__stacked_handlers"; -my $expected = join "\n", qw(one two three), ''; +my $expected = join "\n", qw(one two three); my $received = GET_BODY $location; ok t_cmp($expected, $received, "stacked_handlers"); 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 -PerlResponseHandler TestHooks::init -PerlResponseHandler TestHooks::init::response +PerlResponseHandler TestHooks::init TestHooks::init::response SetHandler modperl Index: t/hooks/TestHooks/push_handlers.pm =================================================================== RCS file: /home/cvspublic/modperl-2.0/t/hooks/TestHooks/push_handlers.pm,v retrieving revision 1.5 diff -u -r1.5 push_handlers.pm --- t/hooks/TestHooks/push_handlers.pm 18 Apr 2003 06:18:57 -0000 1.5 +++ t/hooks/TestHooks/push_handlers.pm 30 May 2003 17:46:58 -0000 @@ -39,7 +39,7 @@ } sub end { return Apache::DONE } -sub say { shift->print(shift,"\n"); return Apache::OK } +sub say { shift->print(shift,"\n"); return Apache::DECLINED } sub conf { # this one is configured from httpd.conf 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__ + PerlModule TestHooks::stacked_handlers + + # can't test PerlTransHandler without messing up other tests + PerlTypeHandler TestHooks::stacked_handlers::ok TestHooks::stacked_handlers::error + - 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 Index: xs/tables/current/ModPerl/FunctionTable.pm =================================================================== RCS file: /home/cvspublic/modperl-2.0/xs/tables/current/ModPerl/FunctionTable.pm,v retrieving revision 1.115 diff -u -r1.115 FunctionTable.pm --- xs/tables/current/ModPerl/FunctionTable.pm 30 May 2003 12:55:15 -0000 1.115 +++ xs/tables/current/ModPerl/FunctionTable.pm 30 May 2003 17:47:01 -0000 @@ -190,6 +190,10 @@ { 'type' => 'request_rec *', 'name' => 'r' + }, + { + 'type' => 'int', + 'name' => 'run_all' } ] }, @@ -204,6 +208,10 @@ { 'type' => 'request_rec *', 'name' => 'r' + }, + { + 'type' => 'int', + 'name' => 'run_all' } ] }, @@ -278,6 +286,10 @@ { 'type' => 'apr_pool_t *', 'name' => 'ptemp' + }, + { + 'type' => 'int', + 'name' => 'run_all' } ] }, --------------070100090004060304070206 Content-Type: text/plain; charset=us-ascii --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org For additional commands, e-mail: dev-help@perl.apache.org --------------070100090004060304070206--