[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[cgiapp] Re: critical bug in 'import' of CGI::App 3.21 (test case available)


On 2004-02-12, Steve Hay <suppressed> wrote:
>
> As the person that submitted the import() routine patch in the first 
> place, I feel obliged to help out.
>
> The attached patch against 3.21 will fix your test case, and hopefully 
> most other problems like it.
>
> The solution is as you suggested -- C::A's import() does what it needs 
> to do regarding Carp/CGI::Carp, and then switches to Exporter's import() 
> (via the goto &SUB syntax to preserve the same arguments).  The 
> "-nocgicarp" symbol is removed from @_ first if it was present, and 
> Exporter's import() is only called if the class that C::A's import() was 
> invoked on (i.e. TestImport in your test case) isa() Exporter.
>
> However, this is still not perfect.  For example, suppose you have a 
> C::A subclass that inherits from three parents, e.g.
>
>     package TestImport;
>     our @ISA = qw(CGI::Application Foo Exporter);
>
> You may have written Foo's import() in a similar way to this patch for 
> C::A's import(), so that under C::A 3.1 the above would have found the 
> "first" import() in Foo, which in turn would have called Exporter's 
> import().
>
> Now, of course, the "first" import() will be found in C::A.  With this 
> patch C::A's import() will call Exporter's import(), but Foo's has now 
> been missed :(
>
> I suspect that there is nothing that can be done about this.  Such are 
> the joys of multiple inheritance.  We could have C::A's import() walk up 
> the invocant class' whole inheritance tree calling import() routines in 
> turn wherever they are found, but that is generally not what one expects 
> methods to do (in Perl), and would still have issues itself anyway, 
> because then Exporter's import() would get called twice -- once by Foo's 
> import() and once more by C::A's import() when it crawls up the tree.
>
> This patch at least addresses the common case of wanting to inherit from 
> C::A and Exporter.  If anybody is wanting to inherit from other classes 
> as well, then hopefully they would be familiar with the perils of MI 
> anyway ;)

Thanks for the help on this Steve. Before this gets committed, can I get
a second opinion on it? I'm not a king of multiple-inheritance myself. 
Wouldn't it usually be the case that you only eventually get /one/
import method and not a combination of all of them? That is, this
solution doesn't seem to be creating issues that wouldn't already exist,
correct? (Doh, sorry for the double-negative question).  

If the 'clean' solution is that I should go back and adjust code in
several places, I can do that. I'm just concerned that this will bite
other people, and at the same time the solution that Steve has
implemented seems like a reasonable solution. Would others agree, or am
I deluding myself? 

	Mark

-- 
 . . . . . . . . . . . . . . . . . . . . . . . . . . . 
   Mark Stosberg            Principal Developer  
   suppressed     Summersault, LLC     
   765-939-9301 ext 202     database driven websites
 . . . . . http://www.summersault.com/ . . . . . . . .


---------------------------------------------------------------------
Web Archive:  http://www.mail-archive.com/suppressed/
              http://marc.theaimsgroup.com/?l=cgiapp&r=1&w=2
To unsubscribe, e-mail: suppressed
For additional commands, e-mail: suppressed


Mail converted by mhonarc 2.6.15
This archive provided courtesy of JSW4.NET, Internet Hosting Services for Small Business.