You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@perl.apache.org by "Philippe M. Chiasson" <go...@cpan.org> on 2003/05/27 06:40:05 UTC

Re: [MP2 Patch] Bug ? Recursion in parsing of PerlSection

On Tue, 2003-05-27 at 12:00, Stas Bekman wrote:
> Philippe M. Chiasson wrote:
> [...]
> > Is evaluated in Apache::ReadConfig's namespace, even if it does nothing.
> > 
> > Once again Apache::PerlSections is called in with Apache::ReadConfig as
> > the namespace
> > 
> > This handler will once again see
> > 
> > @Apache::ReadConfig::include = "foo.conf";
> > 
> > and here starts the recursion.
> > 
> > 
> > This is a hairy problem indeed. I realized this partially in the past,
> > when re-introducing PerlSaveConfig but didn't think of this particular
> > recursive possibility.
> > 
> > The way I see it, there is only a few ways to proprely fix this.
> > 
> > 1. To use one _extra_ namespace to save the config into, if
> > PerlSaveConfig is specified, and delete or move elements from
> > Apache::ReadConfig as it is being processed.
> > 
> > 2. To use automatically-generated namespaces for each <Perl > sections,
> > like ModPerl::Registry, like Apache::ReadConfig::blocknnn or something.
> > 
> > I personally prefer solution #2.
> > 
> > Ideas/suggestions ? If not, I'll probably give a shot at implementing #2
> > sometime this week.
> 
> Do you have to process again the include directive? 

Well, if I PerlSection was smart enough it could treat @Include
directives as a special case, but that wouldn't solve the whole problem.

> If not, why not use the 
> same solution as used by require, which stores name=>path of loaded files in 
> %INC. I don't know where you currently store the processed result of include, 
> but if you lose it, why not store it in %Apache::ReadConfig::INC, with key 
> being the included filename and value the result of the include? Though need 
> to ensure that we have no duplication when two files with indentical names 
> (but located at different paths) are included. Should probably resolve these 
> to a full path? or append the parent's path?

Yes, I thought along those lines, but the simplest problem with this
approach is this (unrelated to first problem, it's a second problem)

<Perl >
push @Alias = [ '/foo', '/bar' ];
</Perl>

And later on

<Perl >
1;
</Perl>

Will cause a warning about one Alias shadowing another one.

The core reason why <Perl > blocks are a bit broken right now is
because multiple <Perl > sections add to the same namespace (if
SaveSections is on, or if @include is used), and when the PerlSection
handler kicks in , it has no way to differentiate between the content it
has processed before vs. the new stuff.

I thougth about many ways of moving stuff as it's being processed, but
it would be quite complex, as it's all recursive and all.

The following patch does work nicely for me though.

It puts each <Perl > block in it's own namespace, and keeps feeding them
to the handler as usual. It all works fine. When it's time to have
Apache::PerlSections->Dump(), it'll be just a matter of iterating over
all those namespaces, striping them out before printing them.

And since Dump() has not been ported yet (mea culpa), I figured this
change wouldn't break anything.

Thoughts ?

# $Id: Apache_PerlSections_ns.patch,v 1.1 2003/05/27 04:39:40 gozer Exp $

Index: src/modules/perl/modperl_cmd.c
===================================================================
RCS file: /home/cvs/modperl-2.0/src/modules/perl/modperl_cmd.c,v
retrieving revision 1.47
diff -u -I$Id -r1.47 modperl_cmd.c
--- src/modules/perl/modperl_cmd.c	16 Apr 2003 03:03:35 -0000	1.47
+++ src/modules/perl/modperl_cmd.c	27 May 2003 04:38:32 -0000
@@ -360,6 +360,7 @@
     const char *package_name = NULL;
     int status = OK;
     AV *args = Nullav;
+    static int ns_counter = 0;
 #ifdef USE_ITHREADS
     MP_dSCFG(s);
     pTHX;
@@ -392,8 +393,11 @@
             
         if (!(package_name = apr_table_get(options, "package"))) {
             package_name = apr_pstrdup(p, MP_DEFAULT_PERLSECTION_PACKAGE);
-            apr_table_set(options, "package", package_name);
         }
+
+        package_name = apr_pstrcat(p, package_name,"::", 
+                                   apr_itoa(p, ns_counter++), NULL);
+        apr_table_set(options, "package", package_name);
 
         /* put the code about to be executed in the configured package */
         arg = apr_pstrcat(p, "package ", package_name, ";", arg, NULL);
Index: t/response/TestDirective/perldo.pm
===================================================================
RCS file: /home/cvs/modperl-2.0/t/response/TestDirective/perldo.pm,v
retrieving revision 1.3
diff -u -I$Id -r1.3 perldo.pm
--- t/response/TestDirective/perldo.pm	17 Mar 2003 06:46:56 -0000	1.3
+++ t/response/TestDirective/perldo.pm	27 May 2003 04:38:33 -0000
@@ -14,11 +14,11 @@
 
     ok t_cmp('yes', $TestDirective::perl::worked);
     
-    ok not exists $Apache::ReadConfig::Location{'/perl_sections'};
+    ok not exists $Apache::ReadConfig::1::Location{'/perl_sections'};
     
-    ok exists $Apache::ReadConfig::Location{'/perl_sections_saved'};
+    ok exists $Apache::ReadConfig::2::Location{'/perl_sections_saved'};
   
-    ok t_cmp('PerlSection', $Apache::ReadConfig::Location{'/perl_sections_saved'}{'AuthName'});
+    ok t_cmp('PerlSection', $Apache::ReadConfig::2::Location{'/perl_sections_saved'}{'AuthName'});
 
     ok t_cmp('yes', $TestDirective::perl::comments);
 



> 
> __________________________________________________________________
> 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
-- 
-- -----------------------------------------------------------------------------
Philippe M. Chiasson /gozer\@(cpan|ectoplasm)\.org/ 88C3A5A5 (122FF51B/C634E37B)
http://gozer.ectoplasm.org/    F9BF E0C2 480E 7680 1AE5 3631 CB32 A107 88C3 A5A5
Q: It is impossible to make anything foolproof because fools are so ingenious.
perl -e'$$=\${gozer};{$_=unpack(P7,pack(L,$$));/^JAm_pH\n$/&&print||$$++&&redo}'

Re: [MP2 Patch] Bug ? Recursion in parsing of PerlSection

Posted by Stas Bekman <st...@stason.org>.
Philippe M. Chiasson wrote:
> On Tue, 2003-05-27 at 13:01, Stas Bekman wrote:
> 
>>Philippe M. Chiasson wrote:
>>
>>>On Tue, 2003-05-27 at 12:00, Stas Bekman wrote:
>>>
>>>
>>>>Philippe M. Chiasson wrote:
>>>>[...]
>>>>
>>>>
>>>>>Is evaluated in Apache::ReadConfig's namespace, even if it does nothing.
>>>>>
>>>>>Once again Apache::PerlSections is called in with Apache::ReadConfig as
>>>>>the namespace
>>>>>
>>>>>This handler will once again see
>>>>>
>>>>>@Apache::ReadConfig::include = "foo.conf";
>>>>>
>>>>>and here starts the recursion.
>>>>>
>>>>>
>>>>>This is a hairy problem indeed. I realized this partially in the past,
>>>>>when re-introducing PerlSaveConfig but didn't think of this particular
>>>>>recursive possibility.
>>>>>
>>>>>The way I see it, there is only a few ways to proprely fix this.
>>>>>
>>>>>1. To use one _extra_ namespace to save the config into, if
>>>>>PerlSaveConfig is specified, and delete or move elements from
>>>>>Apache::ReadConfig as it is being processed.
>>>>>
>>>>>2. To use automatically-generated namespaces for each <Perl > sections,
>>>>>like ModPerl::Registry, like Apache::ReadConfig::blocknnn or something.
>>>>>
>>>>>I personally prefer solution #2.
>>>>>
>>>>>Ideas/suggestions ? If not, I'll probably give a shot at implementing #2
>>>>>sometime this week.
>>>>
>>>>Do you have to process again the include directive? 
>>>
>>>
>>>Well, if I PerlSection was smart enough it could treat @Include
>>>directives as a special case, but that wouldn't solve the whole problem.
>>>
>>>
>>>
>>>>If not, why not use the 
>>>>same solution as used by require, which stores name=>path of loaded files in 
>>>>%INC. I don't know where you currently store the processed result of include, 
>>>>but if you lose it, why not store it in %Apache::ReadConfig::INC, with key 
>>>>being the included filename and value the result of the include? Though need 
>>>>to ensure that we have no duplication when two files with indentical names 
>>>>(but located at different paths) are included. Should probably resolve these 
>>>>to a full path? or append the parent's path?
>>>
>>>
>>>Yes, I thought along those lines, but the simplest problem with this
>>>approach is this (unrelated to first problem, it's a second problem)
>>>
>>><Perl >
>>>push @Alias = [ '/foo', '/bar' ];
>>></Perl>
>>>
>>>And later on
>>>
>>><Perl >
>>>1;
>>></Perl>
>>>
>>>Will cause a warning about one Alias shadowing another one.
>>>
>>>The core reason why <Perl > blocks are a bit broken right now is
>>>because multiple <Perl > sections add to the same namespace (if
>>>SaveSections is on, or if @include is used), and when the PerlSection
>>>handler kicks in , it has no way to differentiate between the content it
>>>has processed before vs. the new stuff.
>>>
>>>I thougth about many ways of moving stuff as it's being processed, but
>>>it would be quite complex, as it's all recursive and all.
>>>
>>>The following patch does work nicely for me though.
>>>
>>>It puts each <Perl > block in it's own namespace, and keeps feeding them
>>>to the handler as usual. It all works fine. When it's time to have
>>>Apache::PerlSections->Dump(), it'll be just a matter of iterating over
>>>all those namespaces, striping them out before printing them.
>>>
>>>And since Dump() has not been ported yet (mea culpa), I figured this
>>>change wouldn't break anything.
>>>
>>>Thoughts ?
>>
>>I see at least two drawbacks with this approach:
>>
>>- waste of memory, namespaces aren't cheap (at least use modperl_mgv)
> 
> 
> By default, the <Perl > namespaces aren't saved, only if you explicitely
> specify it with SaveSections
> 
> 
>>- if you add a new <Perl > above the existing ones, this code will break:
>>
>>
>>>-    ok not exists $Apache::ReadConfig::Location{'/perl_sections'};
>>>+    ok not exists $Apache::ReadConfig::1::Location{'/perl_sections'};
>>
> 
> I know that, it's just test code. Nobody should be peeking in
> $Apache::ReadConfig::* by themselves, it's against the API, IMHO
> 
> 
>>why not monitoring what information was seen already using some other method? 
>>e.g. push those chunks into an array and flatten it when you need and you 
>>still have a control on what you have seen and what not? in the worst case you 
>>can store unprocessed flattened sections, then undef everything in 
>>Apache::ReadConfig, and process it again. Will that work?
> 
> 
> That would most likely work, using some soft of mechanism to either flag
> things you've processed or moving things around as you process them.
> 
> What bothers me with this is that <Perl > sections offer the possiblilty
> of writing alternate handlers for it. The only service provided to those
> handlers it evaling the code in the <perl > section in some namespace
> and then invoking the handler, providing it with a pointer to that
> namespace.
> 
> I know there is only one such handler right now, Apache::PerlSection,
> but if there were others at some point, they would have to implement the
> same protective mechanisms in their own code.
> 
> Dunno, but the more I think about it, the more treating <Perl > blocks
> just like ModPerl::Registry treats scripts (to each it's own namespace)
> sounds like a good idea to me. But I could just be lazy or wrong ...

PerlSections or any other handlers still implement their own tracking by 
checking whether gv is defined.

In that case why not use modperl_mgv, which is both: more lightweight and 
hides the implementation from users, who despite the warnings may want to rely 
on it.

I thought that since you use suggested using namespaces 1, 2, 3, 4... and you 
never access those directly other than looping over the numbers an array 
should work just fine.


__________________________________________________________________
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: [MP2 Patch] Bug ? Recursion in parsing of PerlSection

Posted by "Philippe M. Chiasson" <go...@cpan.org>.
On Tue, 2003-05-27 at 13:01, Stas Bekman wrote:
> Philippe M. Chiasson wrote:
> > On Tue, 2003-05-27 at 12:00, Stas Bekman wrote:
> > 
> >>Philippe M. Chiasson wrote:
> >>[...]
> >>
> >>>Is evaluated in Apache::ReadConfig's namespace, even if it does nothing.
> >>>
> >>>Once again Apache::PerlSections is called in with Apache::ReadConfig as
> >>>the namespace
> >>>
> >>>This handler will once again see
> >>>
> >>>@Apache::ReadConfig::include = "foo.conf";
> >>>
> >>>and here starts the recursion.
> >>>
> >>>
> >>>This is a hairy problem indeed. I realized this partially in the past,
> >>>when re-introducing PerlSaveConfig but didn't think of this particular
> >>>recursive possibility.
> >>>
> >>>The way I see it, there is only a few ways to proprely fix this.
> >>>
> >>>1. To use one _extra_ namespace to save the config into, if
> >>>PerlSaveConfig is specified, and delete or move elements from
> >>>Apache::ReadConfig as it is being processed.
> >>>
> >>>2. To use automatically-generated namespaces for each <Perl > sections,
> >>>like ModPerl::Registry, like Apache::ReadConfig::blocknnn or something.
> >>>
> >>>I personally prefer solution #2.
> >>>
> >>>Ideas/suggestions ? If not, I'll probably give a shot at implementing #2
> >>>sometime this week.
> >>
> >>Do you have to process again the include directive? 
> > 
> > 
> > Well, if I PerlSection was smart enough it could treat @Include
> > directives as a special case, but that wouldn't solve the whole problem.
> > 
> > 
> >>If not, why not use the 
> >>same solution as used by require, which stores name=>path of loaded files in 
> >>%INC. I don't know where you currently store the processed result of include, 
> >>but if you lose it, why not store it in %Apache::ReadConfig::INC, with key 
> >>being the included filename and value the result of the include? Though need 
> >>to ensure that we have no duplication when two files with indentical names 
> >>(but located at different paths) are included. Should probably resolve these 
> >>to a full path? or append the parent's path?
> > 
> > 
> > Yes, I thought along those lines, but the simplest problem with this
> > approach is this (unrelated to first problem, it's a second problem)
> > 
> > <Perl >
> > push @Alias = [ '/foo', '/bar' ];
> > </Perl>
> > 
> > And later on
> > 
> > <Perl >
> > 1;
> > </Perl>
> > 
> > Will cause a warning about one Alias shadowing another one.
> > 
> > The core reason why <Perl > blocks are a bit broken right now is
> > because multiple <Perl > sections add to the same namespace (if
> > SaveSections is on, or if @include is used), and when the PerlSection
> > handler kicks in , it has no way to differentiate between the content it
> > has processed before vs. the new stuff.
> > 
> > I thougth about many ways of moving stuff as it's being processed, but
> > it would be quite complex, as it's all recursive and all.
> > 
> > The following patch does work nicely for me though.
> > 
> > It puts each <Perl > block in it's own namespace, and keeps feeding them
> > to the handler as usual. It all works fine. When it's time to have
> > Apache::PerlSections->Dump(), it'll be just a matter of iterating over
> > all those namespaces, striping them out before printing them.
> > 
> > And since Dump() has not been ported yet (mea culpa), I figured this
> > change wouldn't break anything.
> > 
> > Thoughts ?
> 
> I see at least two drawbacks with this approach:
> 
> - waste of memory, namespaces aren't cheap (at least use modperl_mgv)

By default, the <Perl > namespaces aren't saved, only if you explicitely
specify it with SaveSections

> - if you add a new <Perl > above the existing ones, this code will break:
> 
> > -    ok not exists $Apache::ReadConfig::Location{'/perl_sections'};
> > +    ok not exists $Apache::ReadConfig::1::Location{'/perl_sections'};

I know that, it's just test code. Nobody should be peeking in
$Apache::ReadConfig::* by themselves, it's against the API, IMHO

> why not monitoring what information was seen already using some other method? 
> e.g. push those chunks into an array and flatten it when you need and you 
> still have a control on what you have seen and what not? in the worst case you 
> can store unprocessed flattened sections, then undef everything in 
> Apache::ReadConfig, and process it again. Will that work?

That would most likely work, using some soft of mechanism to either flag
things you've processed or moving things around as you process them.

What bothers me with this is that <Perl > sections offer the possiblilty
of writing alternate handlers for it. The only service provided to those
handlers it evaling the code in the <perl > section in some namespace
and then invoking the handler, providing it with a pointer to that
namespace.

I know there is only one such handler right now, Apache::PerlSection,
but if there were others at some point, they would have to implement the
same protective mechanisms in their own code.

Dunno, but the more I think about it, the more treating <Perl > blocks
just like ModPerl::Registry treats scripts (to each it's own namespace)
sounds like a good idea to me. But I could just be lazy or wrong ...

Gozer out.

> 
> __________________________________________________________________
> 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
-- 
-- -----------------------------------------------------------------------------
Philippe M. Chiasson /gozer\@(cpan|ectoplasm)\.org/ 88C3A5A5 (122FF51B/C634E37B)
http://gozer.ectoplasm.org/    F9BF E0C2 480E 7680 1AE5 3631 CB32 A107 88C3 A5A5
Q: It is impossible to make anything foolproof because fools are so ingenious.
perl -e'$$=\${gozer};{$_=unpack(P7,pack(L,$$));/^JAm_pH\n$/&&print||$$++&&redo}'

Re: [MP2 Patch] Bug ? Recursion in parsing of PerlSection

Posted by Stas Bekman <st...@stason.org>.
Philippe M. Chiasson wrote:
> On Tue, 2003-05-27 at 12:00, Stas Bekman wrote:
> 
>>Philippe M. Chiasson wrote:
>>[...]
>>
>>>Is evaluated in Apache::ReadConfig's namespace, even if it does nothing.
>>>
>>>Once again Apache::PerlSections is called in with Apache::ReadConfig as
>>>the namespace
>>>
>>>This handler will once again see
>>>
>>>@Apache::ReadConfig::include = "foo.conf";
>>>
>>>and here starts the recursion.
>>>
>>>
>>>This is a hairy problem indeed. I realized this partially in the past,
>>>when re-introducing PerlSaveConfig but didn't think of this particular
>>>recursive possibility.
>>>
>>>The way I see it, there is only a few ways to proprely fix this.
>>>
>>>1. To use one _extra_ namespace to save the config into, if
>>>PerlSaveConfig is specified, and delete or move elements from
>>>Apache::ReadConfig as it is being processed.
>>>
>>>2. To use automatically-generated namespaces for each <Perl > sections,
>>>like ModPerl::Registry, like Apache::ReadConfig::blocknnn or something.
>>>
>>>I personally prefer solution #2.
>>>
>>>Ideas/suggestions ? If not, I'll probably give a shot at implementing #2
>>>sometime this week.
>>
>>Do you have to process again the include directive? 
> 
> 
> Well, if I PerlSection was smart enough it could treat @Include
> directives as a special case, but that wouldn't solve the whole problem.
> 
> 
>>If not, why not use the 
>>same solution as used by require, which stores name=>path of loaded files in 
>>%INC. I don't know where you currently store the processed result of include, 
>>but if you lose it, why not store it in %Apache::ReadConfig::INC, with key 
>>being the included filename and value the result of the include? Though need 
>>to ensure that we have no duplication when two files with indentical names 
>>(but located at different paths) are included. Should probably resolve these 
>>to a full path? or append the parent's path?
> 
> 
> Yes, I thought along those lines, but the simplest problem with this
> approach is this (unrelated to first problem, it's a second problem)
> 
> <Perl >
> push @Alias = [ '/foo', '/bar' ];
> </Perl>
> 
> And later on
> 
> <Perl >
> 1;
> </Perl>
> 
> Will cause a warning about one Alias shadowing another one.
> 
> The core reason why <Perl > blocks are a bit broken right now is
> because multiple <Perl > sections add to the same namespace (if
> SaveSections is on, or if @include is used), and when the PerlSection
> handler kicks in , it has no way to differentiate between the content it
> has processed before vs. the new stuff.
> 
> I thougth about many ways of moving stuff as it's being processed, but
> it would be quite complex, as it's all recursive and all.
> 
> The following patch does work nicely for me though.
> 
> It puts each <Perl > block in it's own namespace, and keeps feeding them
> to the handler as usual. It all works fine. When it's time to have
> Apache::PerlSections->Dump(), it'll be just a matter of iterating over
> all those namespaces, striping them out before printing them.
> 
> And since Dump() has not been ported yet (mea culpa), I figured this
> change wouldn't break anything.
> 
> Thoughts ?

I see at least two drawbacks with this approach:

- waste of memory, namespaces aren't cheap (at least use modperl_mgv)
- if you add a new <Perl > above the existing ones, this code will break:

> -    ok not exists $Apache::ReadConfig::Location{'/perl_sections'};
> +    ok not exists $Apache::ReadConfig::1::Location{'/perl_sections'};

why not monitoring what information was seen already using some other method? 
e.g. push those chunks into an array and flatten it when you need and you 
still have a control on what you have seen and what not? in the worst case you 
can store unprocessed flattened sections, then undef everything in 
Apache::ReadConfig, and process it again. Will that work?

__________________________________________________________________
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