perl-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stas Bekman <s...@stason.org>
Subject Re: a new stacked handlers paradigm
Date Mon, 02 Jun 2003 00:05:57 GMT
Geoffrey Young wrote:
> 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:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Mime
View raw message