You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@perl.apache.org by Stas Bekman <st...@stason.org> on 2003/08/23 03:55:31 UTC
Re: [patch] fixing the %ENV setting logic, breaking 'SetHandler modperl'
a bit
Anybody? This was first posted 6 months ago. I'm reposting with an updated patch.
> The current modperl-2.0 cvs has two problems with setting %ENV:
>
> 1. If PerlOptions +SetupEnv is not set, it sets the env too late (only
> for the 'perl-script' response handlers). pre-response handlers don't
> get the environment. So things like cookie based auth don't work out of
> box (need mangling with $r->subprocess_env).
>
> 2. If 'PerlOptions +SetupEnv' is set, the %ENV setting happens too
> early, without giving header_parser handlers to modify the headers.
>
> So I've tried to ammend these two problems, by letting the request a
> chance to set %ENV as the very last header_parser callback (may be
> better to move it as the first callback of the access stage). Though it
> introduces the following problem: 'SetHandler modperl' automatically
> gets the default 'PerlOptions +SetupEnv', because there is no way to
> check what kind of 'SetHandler' we have before the response stage and
> when we get it, it's too late, %ENV has been set already. So the
> solution is to explicitly turn it off with 'PerlOptions -SetupEnv' for
> modperl.
>
> Handlers before the 'access' stage should explicitly call
> $r->subprocess_env, if they want %ENV before the correct time.
>
> If you have better ideas, speak up.
>
> Here is the patch:
Index: src/modules/perl/mod_perl.c
===================================================================
RCS file: /home/cvs/modperl-2.0/src/modules/perl/mod_perl.c,v
retrieving revision 1.180
diff -u -r1.180 mod_perl.c
--- src/modules/perl/mod_perl.c 22 Aug 2003 19:15:09 -0000 1.180
+++ src/modules/perl/mod_perl.c 23 Aug 2003 01:33:22 -0000
@@ -650,6 +650,33 @@
return OK;
}
+static int modperl_hook_header_parser_last(request_rec *r)
+{
+
+ MP_dDCFG;
+
+ /* XXX: modperl_env_request_populate() is called only once per
+ * request. It should be really called only after the
+ * header_parser phase is completed, to give a chance to other
+ * handlers to modify the headers. If handlers need to access %ENV
+ * before that, they should explicitly call $r->subprocess_env
+ */
+ /* XXX: sections with 'SetHandler modperl', must set
+ * 'PerlOption -SetupEnv' to skip the env setting.
+
+ /* 'default: PerlOption +SetupEnv */
+ if (MpDirSETUP_ENV(dcfg) || !MpDirSeenSETUP_ENV(dcfg)) {
+#ifdef USE_ITHREADS
+ modperl_interp_t *interp =
+ modperl_interp_select(r, r->connection, r->server);
+ dTHXa(interp->perl);
+#endif
+ modperl_env_request_populate(aTHX_ r);
+ }
+
+ return DECLINED;
+}
+
static int modperl_destruct_level = 2; /* default is full tear down */
int modperl_perl_destruct_level(void)
@@ -746,6 +773,9 @@
ap_hook_header_parser(modperl_hook_header_parser,
NULL, NULL, APR_HOOK_FIRST);
+ ap_hook_header_parser(modperl_hook_header_parser_last,
+ NULL, NULL, APR_HOOK_LAST);
+
ap_hook_child_init(modperl_hook_child_init,
NULL, NULL, APR_HOOK_FIRST);
@@ -895,11 +925,6 @@
#endif
modperl_perl_global_request_save(aTHX_ r);
-
- /* default is +SetupEnv, skip if PerlOption -SetupEnv */
- if (MpDirSETUP_ENV(dcfg) || !MpDirSeenSETUP_ENV(dcfg)) {
- modperl_env_request_populate(aTHX_ r);
- }
/* default is +GlobalRequest, skip if PerlOption -GlobalRequest */
if (MpDirGLOBAL_REQUEST(dcfg) || !MpDirSeenGLOBAL_REQUEST(dcfg)) {
Index: src/modules/perl/modperl_callback.c
===================================================================
RCS file: /home/cvs/modperl-2.0/src/modules/perl/modperl_callback.c,v
retrieving revision 1.56
diff -u -r1.56 modperl_callback.c
--- src/modules/perl/modperl_callback.c 19 Aug 2003 05:01:22 -0000 1.56
+++ src/modules/perl/modperl_callback.c 23 Aug 2003 01:33:22 -0000
@@ -149,10 +149,6 @@
modperl_handler_make_args(aTHX_ &av_args,
"Apache::RequestRec", r, NULL);
- /* only happens once per-request */
- if (MpDirSETUP_ENV(dcfg)) {
- modperl_env_request_populate(aTHX_ r);
- }
break;
case MP_HANDLER_TYPE_PRE_CONNECTION:
case MP_HANDLER_TYPE_CONNECTION:
Index: t/hooks/TestHooks/headerparser.pm
===================================================================
RCS file: /home/cvs/modperl-2.0/t/hooks/TestHooks/headerparser.pm,v
retrieving revision 1.2
diff -u -r1.2 headerparser.pm
--- t/hooks/TestHooks/headerparser.pm 11 Apr 2002 11:08:43 -0000 1.2
+++ t/hooks/TestHooks/headerparser.pm 23 Aug 2003 01:33:22 -0000
@@ -13,6 +13,7 @@
sub handler {
my $r = shift;
+ $r->subprocess_env; # %ENV is not set yet at this stage
$r->notes->set(url => $ENV{REQUEST_URI});
Apache::OK;
__________________________________________________________________
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
Re: [patch] fixing the %ENV setting logic, breaking 'SetHandler modperl'
a bit
Posted by Stas Bekman <st...@stason.org>.
Geoffrey Young wrote:
>
>>> IIRC, this is the same as mp1 behavior. mp/apache 1 require a call
>>> to set_cgi_env_vars to do this, which both mod_perl and mod_cgi do.
>>> calling $r->subprocess_env early was the solution, which made
>>> standard variables available early. just going by memory at this
>>> point, though :)
>>
>>
>>
>> I think the real problem is that:
>>
>> SetHandler perl-script
>>
>> and:
>>
>> SetHandler perl-script
>> PerlOptions +SetupEnv
>>
>> aren't the same, even though the docs advertise them as such. We
>> should probably ammend to doc to say, that:
>>
>> SetHandler perl-script
>>
>> sets the env vars, but only before the response phase. Adding an explicit
>>
>> PerlOptions +SetupEnv
>>
>> will set them during the very first mod_perl phase whatever it is. So
>> only if there is only a response phase handled by mod_perl, the two
>> are equivalent.
>
>
> sounds good to me, if by whatever it is you mean header-parser or
> post-read-request, depending on whether +SetupEnv is inside or outside
> of a container.
right. So should we talk about the init phase, and let the reader figure out
the rest? Or should we be explicit about it?
>> (the next item deals with the fine definition of 'during').
>>
>>>>> 2. If 'PerlOptions +SetupEnv' is set, the %ENV setting happens too
>>>>> early, without giving header_parser handlers to modify the headers.
>>>
>>>
>>>
>>>
>>> header-parser was a misnomer in apache 1.3 - the headers are already
>>> parsed before the phase runs. has that changed in 2.0? I know you
>>> can get to everything early now, especially with input filters, but I
>>> thought the request phases stayed the same.
>>
>>
>>
>> Yup, headers are parsed in one of the core filters. Though this phase
>> is normally where you manipulate $r->headers_in, no?
>>
>> So we should probably nominate this phase as special with regards to
>> +SetupEnv. For example:
>>
>> PerlOptions +SetupEnv
>>
>> sets the env variables at the end of the headerparser phase, so they
>> become available only during the access phase and up.
>
>
> I'm thinking this is the way to go. except that it has to be the
> post-read-request phase if +SetupEnv is seen within a container directive.
ok, so it's a bit more complex, but the logic is the same.
So we want to tell users to use PerlInitHandler to adjust headers ;)
>> Though it will always add a phase to the mod_perl cycle, even if
>> mod_perl is not used at all.
>
>
> I thought there was some optimizations that kept phases from running
> where no perl hook was registered. couldn't we do the same - don't run
> the new hook unless +SetupEnv is found?
Not in this case. The hooks registration is done globally, of course we
immediately return if there is nothing to do. So there is little overhead.
>> Currently the trick is done by doing this one from the modperl_callback:
>>
>> case MP_HANDLER_TYPE_PER_DIR:
>> case MP_HANDLER_TYPE_PER_SRV:
>> modperl_handler_make_args(aTHX_ &av_args,
>> "Apache::RequestRec", r, NULL);
>>
>> /* only happens once per-request */
>> if (MpDirSETUP_ENV(dcfg)) {
>> modperl_env_request_populate(aTHX_ r);
>> }
>> break;
>>
>> So it always run before the very first mod_perl handler whatever it's.
>>
>> If my suggestion above applies, we could rewrite this code to do the
>> same thing if the phase is access or higher. But add the same code
>> after the handler is executed, and only if the phase is 'headerparser'
>> run this code. In pseudo code this will look as:
>>
>> set_env if phase > header_parser
>> run_handler
>> set_env if phase == header_parser
>>
>> So a custom header_parser will be able to adjust the headers before
>> they are converted into %ENV.
>
>
> I think we need separate per-dir and per-server logic to cover whether
> the directive is inside or outside of a container, but yeah, something
> like that.
sounds good. I need to write some tests that fail first ;)
__________________________________________________________________
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
Re: [patch] fixing the %ENV setting logic, breaking 'SetHandler modperl'
a bit
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
>> IIRC, this is the same as mp1 behavior. mp/apache 1 require a call to
>> set_cgi_env_vars to do this, which both mod_perl and mod_cgi do.
>> calling $r->subprocess_env early was the solution, which made standard
>> variables available early. just going by memory at this point, though :)
>
>
> I think the real problem is that:
>
> SetHandler perl-script
>
> and:
>
> SetHandler perl-script
> PerlOptions +SetupEnv
>
> aren't the same, even though the docs advertise them as such. We should
> probably ammend to doc to say, that:
>
> SetHandler perl-script
>
> sets the env vars, but only before the response phase. Adding an explicit
>
> PerlOptions +SetupEnv
>
> will set them during the very first mod_perl phase whatever it is. So
> only if there is only a response phase handled by mod_perl, the two are
> equivalent.
sounds good to me, if by whatever it is you mean header-parser or
post-read-request, depending on whether +SetupEnv is inside or outside of a
container.
>
> (the next item deals with the fine definition of 'during').
>
>>>> 2. If 'PerlOptions +SetupEnv' is set, the %ENV setting happens too
>>>> early, without giving header_parser handlers to modify the headers.
>>
>>
>>
>> header-parser was a misnomer in apache 1.3 - the headers are already
>> parsed before the phase runs. has that changed in 2.0? I know you
>> can get to everything early now, especially with input filters, but I
>> thought the request phases stayed the same.
>
>
> Yup, headers are parsed in one of the core filters. Though this phase is
> normally where you manipulate $r->headers_in, no?
>
> So we should probably nominate this phase as special with regards to
> +SetupEnv. For example:
>
> PerlOptions +SetupEnv
>
> sets the env variables at the end of the headerparser phase, so they
> become available only during the access phase and up.
I'm thinking this is the way to go. except that it has to be the
post-read-request phase if +SetupEnv is seen within a container directive.
> If env vars are
> wanted earlier $r->subprocess_env is to be called explicitly.
yup.
>
>> but what exactly is the complaint? you want to manipulate the Cookie
>> header before it gets to %ENV via a header-parser handler? instead of
>> HOOK_LAST, we could always register two hooks - the
>> PerlHeaderParserHandler hook as HOOK_REALLY_FIRST and then add
>> modperl_env_request_populate logic as a separate HOOK_FIRST handler :)
>
>
> Since PerlHeaderParserHandler is RUN_ALL, this should work, but why
> HOOK_FIRST and not HOOK_REALLY_LAST for modperl_env_request_populate logic?
well, HOOK_REALLY_FIRST and HOOK_FIRST was just a thought (and it was late
:). keeping header-parser/post-read-request as HOOK_FIRST and making
SetupEnv HOOK_LAST is probably better.
>
> Though it will always add a phase to the mod_perl cycle, even if
> mod_perl is not used at all.
I thought there was some optimizations that kept phases from running where
no perl hook was registered. couldn't we do the same - don't run the new
hook unless +SetupEnv is found?
>
> Currently the trick is done by doing this one from the modperl_callback:
>
> case MP_HANDLER_TYPE_PER_DIR:
> case MP_HANDLER_TYPE_PER_SRV:
> modperl_handler_make_args(aTHX_ &av_args,
> "Apache::RequestRec", r, NULL);
>
> /* only happens once per-request */
> if (MpDirSETUP_ENV(dcfg)) {
> modperl_env_request_populate(aTHX_ r);
> }
> break;
>
> So it always run before the very first mod_perl handler whatever it's.
>
> If my suggestion above applies, we could rewrite this code to do the
> same thing if the phase is access or higher. But add the same code after
> the handler is executed, and only if the phase is 'headerparser' run
> this code. In pseudo code this will look as:
>
> set_env if phase > header_parser
> run_handler
> set_env if phase == header_parser
>
> So a custom header_parser will be able to adjust the headers before they
> are converted into %ENV.
I think we need separate per-dir and per-server logic to cover whether the
directive is inside or outside of a container, but yeah, something like that.
--Geoff
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [patch] fixing the %ENV setting logic, breaking 'SetHandler modperl'
a bit
Posted by Stas Bekman <st...@stason.org>.
Geoffrey Young wrote:
>
>
> Stas Bekman wrote:
>
>> Anybody? This was first posted 6 months ago. I'm reposting with an
>> updated patch.
>>
>>> The current modperl-2.0 cvs has two problems with setting %ENV:
>>>
>>> 1. If PerlOptions +SetupEnv is not set, it sets the env too late
>>> (only for the 'perl-script' response handlers). pre-response handlers
>>> don't get the environment. So things like cookie based auth don't
>>> work out of box (need mangling with $r->subprocess_env).
>
>
> IIRC, this is the same as mp1 behavior. mp/apache 1 require a call to
> set_cgi_env_vars to do this, which both mod_perl and mod_cgi do.
> calling $r->subprocess_env early was the solution, which made standard
> variables available early. just going by memory at this point, though :)
I think the real problem is that:
SetHandler perl-script
and:
SetHandler perl-script
PerlOptions +SetupEnv
aren't the same, even though the docs advertise them as such. We should
probably ammend to doc to say, that:
SetHandler perl-script
sets the env vars, but only before the response phase. Adding an explicit
PerlOptions +SetupEnv
will set them during the very first mod_perl phase whatever it is. So only if
there is only a response phase handled by mod_perl, the two are equivalent.
(the next item deals with the fine definition of 'during').
>>> 2. If 'PerlOptions +SetupEnv' is set, the %ENV setting happens too
>>> early, without giving header_parser handlers to modify the headers.
>
>
> header-parser was a misnomer in apache 1.3 - the headers are already
> parsed before the phase runs. has that changed in 2.0? I know you can
> get to everything early now, especially with input filters, but I
> thought the request phases stayed the same.
Yup, headers are parsed in one of the core filters. Though this phase is
normally where you manipulate $r->headers_in, no?
So we should probably nominate this phase as special with regards to
+SetupEnv. For example:
PerlOptions +SetupEnv
sets the env variables at the end of the headerparser phase, so they become
available only during the access phase and up. If env vars are wanted earlier
$r->subprocess_env is to be called explicitly.
> but what exactly is the complaint? you want to manipulate the Cookie
> header before it gets to %ENV via a header-parser handler? instead of
> HOOK_LAST, we could always register two hooks - the
> PerlHeaderParserHandler hook as HOOK_REALLY_FIRST and then add
> modperl_env_request_populate logic as a separate HOOK_FIRST handler :)
Since PerlHeaderParserHandler is RUN_ALL, this should work, but why HOOK_FIRST
and not HOOK_REALLY_LAST for modperl_env_request_populate logic?
Though it will always add a phase to the mod_perl cycle, even if mod_perl is
not used at all.
Currently the trick is done by doing this one from the modperl_callback:
case MP_HANDLER_TYPE_PER_DIR:
case MP_HANDLER_TYPE_PER_SRV:
modperl_handler_make_args(aTHX_ &av_args,
"Apache::RequestRec", r, NULL);
/* only happens once per-request */
if (MpDirSETUP_ENV(dcfg)) {
modperl_env_request_populate(aTHX_ r);
}
break;
So it always run before the very first mod_perl handler whatever it's.
If my suggestion above applies, we could rewrite this code to do the same
thing if the phase is access or higher. But add the same code after the
handler is executed, and only if the phase is 'headerparser' run this code. In
pseudo code this will look as:
set_env if phase > header_parser
run_handler
set_env if phase == header_parser
So a custom header_parser will be able to adjust the headers before they are
converted into %ENV.
__________________________________________________________________
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
Re: [patch] fixing the %ENV setting logic, breaking 'SetHandler modperl'
a bit
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
Stas Bekman wrote:
> Anybody? This was first posted 6 months ago. I'm reposting with an
> updated patch.
>
>> The current modperl-2.0 cvs has two problems with setting %ENV:
>>
>> 1. If PerlOptions +SetupEnv is not set, it sets the env too late (only
>> for the 'perl-script' response handlers). pre-response handlers don't
>> get the environment. So things like cookie based auth don't work out
>> of box (need mangling with $r->subprocess_env).
IIRC, this is the same as mp1 behavior. mp/apache 1 require a call to
set_cgi_env_vars to do this, which both mod_perl and mod_cgi do. calling
$r->subprocess_env early was the solution, which made standard variables
available early. just going by memory at this point, though :)
>>
>> 2. If 'PerlOptions +SetupEnv' is set, the %ENV setting happens too
>> early, without giving header_parser handlers to modify the headers.
header-parser was a misnomer in apache 1.3 - the headers are already parsed
before the phase runs. has that changed in 2.0? I know you can get to
everything early now, especially with input filters, but I thought the
request phases stayed the same.
but what exactly is the complaint? you want to manipulate the Cookie header
before it gets to %ENV via a header-parser handler? we could always
register the PerlHeaderParserHandler hook as HOOK_REALLY_FIRST and then add
SetupEnv as a separate HOOK_FIRST handler :)
>>
>> So I've tried to ammend these two problems, by letting the request a
>> chance to set %ENV as the very last header_parser callback
but what exactly is the complaint? you want to manipulate the Cookie header
before it gets to %ENV via a header-parser handler? instead of HOOK_LAST,
we could always register two hooks - the PerlHeaderParserHandler hook as
HOOK_REALLY_FIRST and then add modperl_env_request_populate logic as a
separate HOOK_FIRST handler :)
>> (may be
>> better to move it as the first callback of the access stage).
I definitely wouldn't do that - let's keep our use of the phases (in core,
at least ;) to their official meanings as much as possible
>> Though
>> it introduces the following problem: 'SetHandler modperl'
>> automatically gets the default 'PerlOptions +SetupEnv', because there
>> is no way to check what kind of 'SetHandler' we have before the
>> response stage and when we get it, it's too late, %ENV has been set
>> already. So the solution is to explicitly turn it off with
>> 'PerlOptions -SetupEnv' for modperl.
>>
>> Handlers before the 'access' stage should explicitly call
>> $r->subprocess_env, if they want %ENV before the correct time.
>>
>> If you have better ideas, speak up.
it's late, but I'm not sure this is an issue. $r->subprocess_env is the old
way, so I don't see a problem carrying it over for early access. I
definitely would't stick anything into the access phase
as for using the header-parser before %ENV is setup, I don't think you could
do it before, so I don't see it as a major issue. of course, if we can get
it right it would be a nice bonus :)
--Geoff
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org