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

[Steve Hay <suppressed>]

Hi Mark,

Mark Stosberg wrote:

>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? 
>
Yes, absolutely -- you get the first /one/ that is found.  That was 
exactly your problem -- you were finding C::A's import() before (i.e. 
instead of) Exporter's import().

Now you'll get Exporter's import() too, which is not "normal", but is 
generally what people would want.

Of course, if people inherit from Exporter before C::A then they'll only 
get Exporter's import(), which doesn't kindly call C::A's one as well.  
But that really is a problem that they would need to fix in their code.

>That is, this
>solution doesn't seem to be creating issues that wouldn't already exist,
>correct? (Doh, sorry for the double-negative question).  
>
The only issue being created that doesn't already exist which I can 
think of is the one that I alluded to with my patch, namely where people 
inherit from three or more classes.  They could code a Foo class such 
that @ISA = qw(CGI::Application Foo Exporter) will work under C::A 3.1 
(i.e. Foo's import() calls Exporter's), but now, because of C::A 
introducing an import() routine, that would break.

Perhaps an "Incompatible Changes" section should be introduced into the 
README or POD documentation describing this possible scenario.

>
>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, 
>
I agree that it could well affect others, hence my attempt to find a 
solution.

Cheers,
- Steve

>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
>
>  
>




------------------------------------------------
Radan Computational Ltd.

The information contained in this message and any files transmitted with it are confidential and intended for the addressee(s) only.  If you have received this message in error or there are any problems, please notify the sender immediately.  The unauthorized use, disclosure, copying or alteration of this message is strictly forbidden.  Note that any views or opinions presented in this email are solely those of the author and do not necessarily represent those of Radan Computational Ltd.  The recipient(s) of this message should check it and any attached files for viruses: Radan Computational will accept no liability for any damage caused by any virus transmitted by this email.


---------------------------------------------------------------------
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