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@xxxxxxxxxx http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com