Return-Path: X-Original-To: apmail-perl-dev-archive@www.apache.org Delivered-To: apmail-perl-dev-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 2568810457 for ; Wed, 6 Nov 2013 19:07:11 +0000 (UTC) Received: (qmail 16545 invoked by uid 500); 6 Nov 2013 19:07:11 -0000 Delivered-To: apmail-perl-dev-archive@perl.apache.org Received: (qmail 16506 invoked by uid 500); 6 Nov 2013 19:07:11 -0000 Mailing-List: contact dev-help@perl.apache.org; run by ezmlm Precedence: bulk list-help: list-unsubscribe: List-Post: List-Id: Delivered-To: mailing list dev@perl.apache.org Received: (qmail 16498 invoked by uid 99); 6 Nov 2013 19:07:11 -0000 Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 06 Nov 2013 19:07:11 +0000 X-ASF-Spam-Status: No, hits=2.7 required=5.0 tests=HTML_MESSAGE,MISSING_HEADERS,RCVD_IN_DNSWL_LOW,SPF_PASS X-Spam-Check-By: apache.org Received-SPF: pass (athena.apache.org: domain of trawick@gmail.com designates 209.85.217.177 as permitted sender) Received: from [209.85.217.177] (HELO mail-lb0-f177.google.com) (209.85.217.177) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 06 Nov 2013 19:07:07 +0000 Received: by mail-lb0-f177.google.com with SMTP id u14so72265lbd.36 for ; Wed, 06 Nov 2013 11:06:45 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:cc :content-type; bh=ZheXPLXzUjCBzXp7+mxJlzLnF03xRXbgrqbnKcrCXgU=; b=nNCgCi8ionmHU4GRVpuFpv4FisOFGgZonX2bmM1kX8mF8vkftkjjU/RdLJ7ID36zHu XowQK/qUj7Pn537IZG0plzUFDiTXDh4aSfEy0hJI0oA3QCh1C6qsg4dRxhOJcgbWVW3t ibsnERq5Ghc6vGa5NtcYBNVRDZ1EPF/qgNe55pJ2wxd3ZOBSPFtLmKeiPtuLbi81qRQf gPjH3D+klw1tx5k3QEwfBXrr+yt6N9r2dC7U4FXwoXTjnkS8NlY65lEk90kXIFkjwi5u vm6GS9aAn2GffCKuG8TbPjzwsvTAESxxaX7bn1xAEo/BVf8KyvKxT/Z6S8gJV4F2JYqR g61g== MIME-Version: 1.0 X-Received: by 10.112.13.72 with SMTP id f8mr2734986lbc.40.1383764805202; Wed, 06 Nov 2013 11:06:45 -0800 (PST) Received: by 10.114.187.71 with HTTP; Wed, 6 Nov 2013 11:06:45 -0800 (PST) In-Reply-To: References: Date: Wed, 6 Nov 2013 14:06:45 -0500 Message-ID: Subject: Re: need sanity check on my mod_perl + 2.4.6 on Windows attempt From: Jeff Trawick Cc: dev@perl.apache.org Content-Type: multipart/alternative; boundary=001a11c3ee6aaab69b04ea86d909 X-Virus-Checked: Checked by ClamAV on apache.org --001a11c3ee6aaab69b04ea86d909 Content-Type: text/plain; charset=ISO-8859-1 On Wed, Nov 6, 2013 at 1:20 PM, Steve Hay wrote: > On 6 November 2013 14:27, Jeff Trawick wrote: > > On Wed, Nov 6, 2013 at 8:50 AM, Steve Hay > > wrote: > >> > >> On 6 November 2013 13:50, Steve Hay wrote: > >> > On 6 November 2013 12:27, Jeff Trawick wrote: > >> >> On Wed, Nov 6, 2013 at 4:02 AM, Steve Hay < > steve.m.hay@googlemail.com> > >> >> wrote: > >> >>> > >> >>> On 6 November 2013 00:48, Jeff Trawick wrote: > >> >>> > Back to the httpd24threading branch: > >> >>> > > >> >>> > * modperl_interp_pool_select() has this notion of phase, which > must > >> >>> > either > >> >>> > be startup or request context. > >> >>> > * It thinks it is startup only if the pool passed in is > >> >>> > s->process->pconf. > >> >>> > * Sometimes it is passed s->process->pool (parent of pconf), such > as > >> >>> > from > >> >>> > perl_parse_require_line(). > >> >>> > * perl_parse_require_line() can sometimes be called from request > >> >>> > context. > >> >>> > * When perl_parse_require_line() calls > modperl_interp_pool_select(), > >> >>> > request > >> >>> > context can never be identified because perl_parse_require_line() > >> >>> > never > >> >>> > passes in r->pool (which I guess would be cmd->pool). > >> >>> > * etc. > >> >>> > > >> >>> > This would seem to be the way to get the right pool to > >> >>> > modperl_interp_pool_select(). > >> >>> > > >> >>> > Index: src/modules/perl/modperl_util.c > >> >>> > > =================================================================== > >> >>> > --- src/modules/perl/modperl_util.c (revision 1539040) > >> >>> > +++ src/modules/perl/modperl_util.c (working copy) > >> >>> > @@ -989,7 +989,7 @@ > >> >>> > int count; > >> >>> > void *key; > >> >>> > auth_callback *ab; > >> >>> > - MP_dINTERP_POOLa(cmd->server->process->pool, cmd->server); > >> >>> > + MP_dINTERP_POOLa(cmd->pool, cmd->server); > >> >>> > > >> >>> > if (global_authz_providers == NULL) { > >> >>> > MP_INTERP_PUTBACK(interp, aTHX); > >> >>> > > >> >>> > That still doesn't bring happiness (no interpreter returned, > >> >>> > resulting > >> >>> > in a > >> >>> > crash trying to dereference interp). > >> >>> > > >> >>> > >> >>> I'm getting the same crash-on-startup behaviour now myself after a > >> >>> fresh rebuild of everything (now using httpd-2.4.6 and > perl-5.19.5). I > >> >>> will look back over the changes made on the threading branch and/or > my > >> >>> merges of them into the httpd24 branch. Hopefully the answer lies > >> >>> there somewhere. I'll be very grateful for any help I can get with > >> >>> this though -- I didn't do the original work on either of those > >> >>> branches... > >> >> > >> >> > >> >> With the "fix" above in place, modperl_init_vhost() seems to be the > >> >> next > >> >> crucial code. We go down this path: > >> >> > >> >> if (base_server == s) { > >> >> MP_TRACE_i(MP_FUNC, "base server is not vhost, skipping %s", > >> >> vhost); > >> >> return OK; > >> >> } > >> >> > >> >> and fall through this FIXME in modperl_interp_pool_select(): > >> >> > >> >> if (!scfg->mip) { > >> >> /* FIXME: We get here if global "server_rec" == > s, > >> >> scfg->mip > >> >> * is not created then. I'm not sure if that's > bug > >> >> or > >> >> * bad/good design decicision. For now just > return > >> >> NULL. > >> >> */ > >> >> return NULL; > >> >> } > >> >> > >> >> (Note: disabling the base_server == s check in modperl_init_vhost() > >> >> brings > >> >> no happiness either, though perhaps it is a step in the right > >> >> direction.) > >> >> > >> >> This path is new with httpd 2.4; 2.2 didn't have authz_providers. > >> >> > >> >> This seems to be a whack-a-mole issue. I'd expect that there is some > >> >> easy > >> >> way to grab the interpreter for any arbitrary startup path, but I > don't > >> >> see > >> >> it. Maybe it is worthwhile seeing if we already went through some > >> >> paths > >> >> where we were able to grab an interpreter. > >> >> > >> > > >> > The last change on the httpd24 branch (r1503193) is what added the > >> > FIXME above, but it also made a change in perl_parse_require_line() > >> > which I've lost in the course of merging the threading branch in: it > >> > made that function tolerant of modperl_interp_pool_select() returning > >> > NULL (which is exactly what happens in the FIXME case). > >> > > >> > If modperl_interp_pool_select() returns NULL then > >> > perl_parse_require_line() just returns NULL itself in r1503193, but in > >> > httpd24threading I've hidden the use of modperl_interp_pool_select() > >> > within the MP_dINTERP_POOLa() macro (as per the general style of > >> > changes in the threading branch), but that macro crashes if > >> > modperl_interp_pool_select() has returned NULL. > >> > > >> > The diff below makes that macro tolerant of > >> > modperl_interp_pool_select() returning NULL, and makes > >> > perl_parse_require_line() tolerant of interp ending up NULL, like it > >> > used to be in r1503193. > >> > > >> > With this diff in place (which includes your earlier change), the > >> > server now starts up for me and tests appear to be running as normal. > >> > >> Oops! In my excitement I forgot the diff!: > >> > >> Index: src/modules/perl/modperl_interp.h > >> =================================================================== > >> --- src/modules/perl/modperl_interp.h (revision 1539262) > >> +++ src/modules/perl/modperl_interp.h (working copy) > >> @@ -68,9 +68,12 @@ > >> #define MP_INTERP_POOLa(p, s) > \ > >> MP_TRACE_i(MP_FUNC, "selecting interp: p=%pp, s=%pp", (p), (s)); > \ > >> interp = modperl_interp_pool_select((p), (s)); > \ > >> - MP_TRACE_i(MP_FUNC, " --> got (0x%pp)->refcnt=%d", > \ > >> - interp, interp->refcnt); > \ > >> - aTHX = interp->perl > >> + if (interp) { > \ > >> + MP_TRACE_i(MP_FUNC, " --> got (0x%pp)->refcnt=%d", > \ > >> + interp, interp->refcnt); > \ > >> + aTHX = interp->perl; > \ > >> + } > \ > >> + NOOP > >> > >> #define MP_dINTERP_POOLa(p, s) > \ > >> MP_dINTERP; > \ > >> Index: src/modules/perl/modperl_util.c > >> =================================================================== > >> --- src/modules/perl/modperl_util.c (revision 1539262) > >> +++ src/modules/perl/modperl_util.c (working copy) > >> @@ -989,8 +989,11 @@ > >> int count; > >> void *key; > >> auth_callback *ab; > >> - MP_dINTERP_POOLa(cmd->server->process->pool, cmd->server); > >> + MP_dINTERP_POOLa(cmd->pool, cmd->server); > >> > >> + if (!interp) > >> + return ret; > >> + > >> if (global_authz_providers == NULL) { > >> MP_INTERP_PUTBACK(interp, aTHX); > >> return ret; > > > > > > I don't think it will ever have an interpreter except when processing a > > require from htaccess. Still, it doesn't crash, and I get the same test > > results as in my prior post. > > > > I think this should trigger an error if we get past these checks with no > > interpreter: > > > > if (global_authz_providers == NULL || > apr_hash_count(global_authz_providers) > > == 0) { > > /* put back an interpreter if we got one */ > > return ret; /* still NULL; why not make it obvious? */ > > } > > > > apr_pool_userdata_get(&key, AUTHZ_PROVIDER_NAME_NOTE, cmd->temp_pool); > > ab = apr_hash_get(global_authz_providers, (char *) key, > > APR_HASH_KEY_STRING); > > if (ab == NULL || ab->cb2 == NULL) { > > /* put back an interpreter if we got one */ > > return ret; /* still NULL; why not make it obvious? */ > > } > > > > if (!interp) { > > return "Require handler is not currently supported in this context." > > } > > > > And the ordering issue (create interpreter vs. handle Require parsing) > still > > needs to be resolved. > > I've committed my change to make the macro more robust, plus your > change to delay trying to fetch an interpreter -- with an early return > if it still fails -- in revisions 1539412 and 1539414. Thanks for your > help with this! > Cool! BTW, to return an error from a require line parser, you actually return the error string (like a directive parser). Here's an example of one from mod_ssl that just ensures there are no arguments: static const char *ssl_authz_require_ssl_parse(cmd_parms *cmd, const char *require_line, const void **parsed) { if (require_line && require_line[0]) return "'Require ssl' does not take arguments"; return NULL; } So returning the error string in the unhandled case (instead of NULL) would probably be good. BUT: > I don't know what to do about trying to fix it for real (i.e. the > ordering problem that you refer to). I'm hoping someone else on the > list might be able to help? > I'm trying to follow this through from a directive like in the testcase: PerlAddAuthzProvider my-user TestAPI::access2_24->authz_handler mod_perl.c has MP_CMD_SRV_TAKE2("PerlAddAuthzProvider", authz_provider, "PerlAddAuthzProvider"), authz_provider takes exactly 2 arguments (or httpd will trigger an error). The first argument (like "my-user") is name and the second argument (like "TestAPI::access2_24->authz_handler") is cb in the following call: modperl_register_auth_provider_name(p, AUTHZ_PROVIDER_GROUP, name, AUTHZ_PROVIDER_VERSION, cb, NULL, AP_AUTH_INTERNAL_PER_CONF); in modperl_register_auth_provider(), cb and the following NULL are callback1 and callback2, and we have ab->cb1 = callback1; ab->cb2 = callback2; return register_auth_provider(pool, provider_group, provider_name_dup, provider_version, ab, type); (callback2 always NULL for an authz provider) register_auth_provider(), for an authz provider like this, stores ab (the two callbacks, one always NULL), in the global_authz_providers hash then calls the httpd API to point to authz_perl_provider for the authz provider with this name (e.g., "my-user"). authz_perl_provider is the thing that says call perl_parse_require_line; perl_parse_require_line is our new friend that tries real hard to get an interpreter in case cb2 is non-NULL, which it never will be, until PerlAddAuthzProvider is updated to allow an optional 2nd handler which would be called at init to check a require line for errors. Similarly it would seem that the authn variant, for which a second handler would have the task of obtaining a password hash for the realm, also cannot be configured. >From the application/script standpoint, I think the drawback of not being able to provide a require line parser is that I'd be required to look at it a steady state and possibly encounter configuration errors that could have been reported at startup (a.k.a. not the end of the world; nothing really to parse for some authz providers). > > > > > > I guess TestAPI::access2_24.pm needs more code to declare a require > parser. > > Any idea how to do that, just to see something found in the hash? > > Sorry, I don't know that right now either, but I will look into it too. > After looking, I think there's no way (see above). -- Born in Roswell... married an alien... http://emptyhammock.com/ --001a11c3ee6aaab69b04ea86d909 Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable
On Wed, Nov 6, 2013 at 1:20 PM, Steve Hay <steve= .m.hay@googlemail.com> wrote:
<= div class=3D"gmail_quote">
On 6 November 2013 14:27= , Jeff Trawick <trawick@gmail.com> wrote:
> On Wed, Nov 6, 2013 at 8:50 AM, Steve Hay <
steve.m.hay@googlemail.com>
> wrote:
>>
>> On 6 November 2013 13:50, Steve Hay <steve.m.hay@googlemail.com> wrote:
>> > On 6 November 2013 12:27, Jeff Trawick <trawick@gmail.com> wrote:
>> >> On Wed, Nov 6, 2013 at 4:02 AM, Steve Hay <steve.m.hay@googlemail.com>
>> >> wrote:
>> >>>
>> >>> On 6 November 2013 00:48, Jeff Trawick <trawick@gmail.com> wrote:
>> >>> > Back to the httpd24threading branch:
>> >>> >
>> >>> > * modperl_interp_pool_select() has this notion o= f phase, which must
>> >>> > either
>> >>> > be startup or request context.
>> >>> > * It thinks it is startup only if the pool passe= d in is
>> >>> > s->process->pconf.
>> >>> > * Sometimes it is passed s->process->pool = (parent of pconf), such as
>> >>> > from
>> >>> > perl_parse_require_line().
>> >>> > * perl_parse_require_line() can sometimes be cal= led from request
>> >>> > context.
>> >>> > * When perl_parse_require_line() calls modperl_i= nterp_pool_select(),
>> >>> > request
>> >>> > context can never be identified because perl_par= se_require_line()
>> >>> > never
>> >>> > passes in r->pool (which I guess would be cmd= ->pool).
>> >>> > * etc.
>> >>> >
>> >>> > This would seem to be the way to get the right p= ool to
>> >>> > modperl_interp_pool_select().
>> >>> >
>> >>> > Index: src/modules/perl/modperl_util.c
>> >>> > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D
>> >>> > --- src/modules/perl/modperl_util.c =A0 =A0 (rev= ision 1539040)
>> >>> > +++ src/modules/perl/modperl_util.c =A0 =A0 (wor= king copy)
>> >>> > @@ -989,7 +989,7 @@
>> >>> > =A0 =A0 =A0int count;
>> >>> > =A0 =A0 =A0void *key;
>> >>> > =A0 =A0 =A0auth_callback *ab;
>> >>> > - =A0 =A0MP_dINTERP_POOLa(cmd->server->pro= cess->pool, cmd->server);
>> >>> > + =A0 =A0MP_dINTERP_POOLa(cmd->pool, cmd->= server);
>> >>> >
>> >>> > =A0 =A0 =A0if (global_authz_providers =3D=3D NUL= L) {
>> >>> > =A0 =A0 =A0 =A0 =A0MP_INTERP_PUTBACK(interp, aTH= X);
>> >>> >
>> >>> > That still doesn't bring happiness (no inter= preter returned,
>> >>> > resulting
>> >>> > in a
>> >>> > crash trying to dereference interp).
>> >>> >
>> >>>
>> >>> I'm getting the same crash-on-startup behaviour n= ow myself after a
>> >>> fresh rebuild of everything (now using httpd-2.4.6 an= d perl-5.19.5). I
>> >>> will look back over the changes made on the threading= branch and/or my
>> >>> merges of them into the httpd24 branch. Hopefully the= answer lies
>> >>> there somewhere. I'll be very grateful for any he= lp I can get with
>> >>> this though -- I didn't do the original work on e= ither of those
>> >>> branches...
>> >>
>> >>
>> >> With the "fix" above in place, modperl_init_vho= st() seems to be the
>> >> next
>> >> crucial code. =A0We go down this path:
>> >>
>> >> =A0 =A0 if (base_server =3D=3D s) {
>> >> =A0 =A0 =A0 =A0 MP_TRACE_i(MP_FUNC, "base server is = not vhost, skipping %s",
>> >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0vhost);
>> >> =A0 =A0 =A0 =A0 return OK;
>> >> =A0 =A0 }
>> >>
>> >> and fall through this FIXME in modperl_interp_pool_select= ():
>> >>
>> >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (!scfg->mip) {
>> >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* FIXME: We get = here if global "server_rec" =3D=3D s,
>> >> scfg->mip
>> >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* is not creat= ed then. I'm not sure if that's bug
>> >> or
>> >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* bad/good des= ign decicision. For now just return
>> >> NULL.
>> >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0*/
>> >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 return NULL;
>> >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 }
>> >>
>> >> (Note: disabling the base_server =3D=3D s check in modper= l_init_vhost()
>> >> brings
>> >> no happiness either, though perhaps it is a step in the r= ight
>> >> direction.)
>> >>
>> >> This path is new with httpd 2.4; 2.2 didn't have auth= z_providers.
>> >>
>> >> This seems to be a whack-a-mole issue. =A0I'd expect = that there is some
>> >> easy
>> >> way to grab the interpreter for any arbitrary startup pat= h, but I don't
>> >> see
>> >> it. =A0Maybe it is worthwhile seeing if we already went t= hrough some
>> >> paths
>> >> where we were able to grab an interpreter.
>> >>
>> >
>> > The last change on the httpd24 branch (r1503193) is what adde= d the
>> > FIXME above, but it also made a change in perl_parse_require_= line()
>> > which I've lost in the course of merging the threading br= anch in: it
>> > made that function tolerant of modperl_interp_pool_select() r= eturning
>> > NULL (which is exactly what happens in the FIXME case).
>> >
>> > If modperl_interp_pool_select() returns NULL then
>> > perl_parse_require_line() just returns NULL itself in r150319= 3, but in
>> > httpd24threading I've hidden the use of modperl_interp_po= ol_select()
>> > within the MP_dINTERP_POOLa() macro (as per the general style= of
>> > changes in the threading branch), but that macro crashes if >> > modperl_interp_pool_select() has returned NULL.
>> >
>> > The diff below makes that macro tolerant of
>> > modperl_interp_pool_select() returning NULL, and makes
>> > perl_parse_require_line() tolerant of interp ending up NULL, = like it
>> > used to be in r1503193.
>> >
>> > With this diff in place (which includes your earlier change),= the
>> > server now starts up for me and tests appear to be running as= normal.
>>
>> Oops! In my excitement I forgot the diff!:
>>
>> Index: src/modules/perl/modperl_interp.h
>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>> --- src/modules/perl/modperl_interp.h =A0 (revision 1539262)
>> +++ src/modules/perl/modperl_interp.h =A0 (working copy)
>> @@ -68,9 +68,12 @@
>> =A0#define MP_INTERP_POOLa(p, s) =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 \
>> =A0 =A0 =A0MP_TRACE_i(MP_FUNC, "selecting interp: p=3D%pp, s= =3D%pp", (p), (s)); =A0 =A0\
>> =A0 =A0 =A0interp =3D modperl_interp_pool_select((p), (s)); =A0 = =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0\
>> - =A0 =A0MP_TRACE_i(MP_FUNC, " =A0--> got (0x%pp)->refc= nt=3D%d", =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 \
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 interp, interp->refcnt); =A0 =A0 = =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 \
>> - =A0 =A0aTHX =3D interp->perl
>> + =A0 =A0if (interp) { =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 \
>> + =A0 =A0 =A0 =A0MP_TRACE_i(MP_FUNC, " =A0--> got (0x%pp)-= >refcnt=3D%d", =A0 =A0 =A0 =A0 =A0 =A0 \
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 interp, interp->refcnt); = =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 \
>> + =A0 =A0 =A0 =A0aTHX =3D interp->perl; =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0\
>> + =A0 =A0} =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 \
>> + =A0 =A0NOOP
>>
>> =A0#define MP_dINTERP_POOLa(p, s) =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0\
>> =A0 =A0 =A0MP_dINTERP; =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 \
>> Index: src/modules/perl/modperl_util.c
>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>> --- src/modules/perl/modperl_util.c =A0 =A0 (revision 1539262)
>> +++ src/modules/perl/modperl_util.c =A0 =A0 (working copy)
>> @@ -989,8 +989,11 @@
>> =A0 =A0 =A0int count;
>> =A0 =A0 =A0void *key;
>> =A0 =A0 =A0auth_callback *ab;
>> - =A0 =A0MP_dINTERP_POOLa(cmd->server->process->pool, cmd= ->server);
>> + =A0 =A0MP_dINTERP_POOLa(cmd->pool, cmd->server);
>>
>> + =A0 =A0if (!interp)
>> + =A0 =A0 =A0 return ret;
>> +
>> =A0 =A0 =A0if (global_authz_providers =3D=3D NULL) {
>> =A0 =A0 =A0 =A0 =A0MP_INTERP_PUTBACK(interp, aTHX);
>> =A0 =A0 =A0 =A0 =A0return ret;
>
>
> I don't think it will ever have an interpreter except when process= ing a
> require from htaccess. =A0Still, it doesn't crash, and I get the s= ame test
> results as in my prior post.
>
> I think this should trigger an error if we get past these checks with = no
> interpreter:
>
> if (global_authz_providers =3D=3D NULL || apr_hash_count(global_authz_= providers)
> =3D=3D 0) {
> =A0 =A0 /* put back an interpreter if we got one */
> =A0 =A0 return ret; /* still NULL; why not make it obvious? */
> }
>
> apr_pool_userdata_get(&key, AUTHZ_PROVIDER_NAME_NOTE, cmd->temp= _pool);
> ab =3D apr_hash_get(global_authz_providers, (char *) key,
> APR_HASH_KEY_STRING);
> if (ab =3D=3D NULL || ab->cb2 =3D=3D NULL) {
> =A0 =A0 /* put back an interpreter if we got one */
> =A0 =A0 return ret; /* still NULL; why not make it obvious? */
> }
>
> if (!interp) {
> =A0 =A0 return "Require handler is not currently supported in thi= s context."
> }
>
> And the ordering issue (create interpreter vs. handle Require parsing)= still
> needs to be resolved.

I've committed my change to make the macro more robust, plu= s your
change to delay trying to fetch an interpreter -- with an early return
if it still fails -- in revisions 1539412 and 1539414. Thanks for your
help with this!

Cool!

BTW, to return an error from a require line parser, you actually re= turn the error string (like a directive parser). =A0Here's an example o= f one from mod_ssl that just ensures there are no arguments:

static const char *ssl_authz_require_ssl_parse(cmd_parm= s *cmd,
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0const char *require_line,
= =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 =A0 =A0const void **parsed)
{
=A0 =A0 if (require_line && require_line[0])
=
=A0 =A0 =A0 =A0 return "'Require ssl' does not take argum= ents";

=A0 =A0 return NULL;
}
=

So returning the error string in the unhandled case (instead of NULL) = would probably be good.

BUT:

<= blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-l= eft-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;pa= dding-left:1ex">
I don't know what to do about trying to fix it for real (i.e. the
ordering problem that you refer to). I'm hoping someone else on the
list might be able to help?

I'm try= ing to follow this through from a directive like in the testcase:=A0
<= div>=A0
PerlAddAuthzProvider my-user TestAPI::access2_24->auth= z_handler

mod_perl.c has

MP_CMD_SR= V_TAKE2("PerlAddAuthzProvider", authz_provider, "PerlAddAuth= zProvider"),

authz_provider takes exactly= 2 arguments (or httpd will trigger an error). =A0The first argument (like = "my-user") is name and the second argument (like "TestAPI::a= ccess2_24->authz_handler") is cb in the following call:

modperl_register_auth_provider_name(p, AUTHZ_PROVI= DER_GROUP, name,
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 AUTHZ_PROVIDER_VERSION, cb, NULL,
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 AP_AUTH_INTERNAL_PER_CONF);

in modperl_register_auth_provider(), cb and the f= ollowing NULL are callback1 and callback2, and we have

=
=A0 =A0 ab->cb1 =3D callback1;
=A0 =A0 ab->cb2 = =3D callback2;

=A0 =A0 return register_auth_provider(pool, provider_gr= oup, provider_name_dup,
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0 =A0 =A0 =A0 provider_version, ab, type);
(callback2 always NULL for an authz provider)

register_auth_provider(), for an authz provider like th= is, stores ab (the two callbacks, one always NULL), in the global_authz_pro= viders hash then calls the httpd API to point to authz_perl_provider for th= e authz provider with this name (e.g., "my-user").

authz_perl_provider is the thing that says call perl_pa= rse_require_line; perl_parse_require_line is our new friend that tries real= hard to get an interpreter in case cb2 is non-NULL, which it never will be= , until PerlAddAuthzProvider is updated to allow an optional 2nd handler wh= ich would be called at init to check a require line for errors.

Similarly it would seem that the authn variant, for whi= ch a second handler would have the task of obtaining a password hash for th= e realm, also cannot be configured.

From the appli= cation/script standpoint, I think the drawback of not being able to provide= a require line parser is that I'd be required to look at it a steady s= tate and possibly encounter configuration errors that could have been repor= ted at startup =A0(a.k.a. not the end of the world; nothing really to parse= for some authz providers).





>
> I guess TestAPI::ac= cess2_24.pm needs more code to declare a require parser.
> Any idea how to do that, just to see something found in the hash?

Sorry, I don't know that right now either, but I will look into i= t too.

After looking, I think there's no way (see above= ).

--
Born in Roswell... married an ali= en...
http://empt= yhammock.com/
--001a11c3ee6aaab69b04ea86d909--