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, Cees Hek <suppressed> wrote:
>  
>
>>Hi Mark,
>>
>>I think the problem is actually in your test code, and not in 
>>CGI::Application.  You are running into a multiple inheritance problem 
>>and your TestImport module is actually using the import function in 
>>CGI::Application instead of the one provided by Exporter.  So the global 
>>never gets exported...
>>
>>Your @ISA looks contains ('CGI::Application', 'Exporter') since you push 
>>Exporter onto the existing @ISA, where Exporter really needs to be first 
>>for it to work correctly...
>>
>>Here are a few ways of solving the problem in your code:
>>
>># unshift to the front of @ISA
>>require Exporter;
>>unshift @ISA, qw(Exporter);
>>
>># bring import into your namespace
>>use Exporter qw(import);
>>
>># copy import into your namespace manually
>>require Exporter;
>>*import = \&Exporter::import;
>>    
>>
>
>Thanks for the help Cees.
>
>I understand what you are saying and agree that it would be best to fix
>this in my code. However, look at like at it like this: I upgraded
>CGI::App and many products quit working, all of which wouldn't be easy
>to track down.  If this happened to me, this will happen to other
>people. 
>
>The solution that makes sense to me is for CGI::App have an import
>function that calls Exporter import to provide a 'standard' import
>function, and then additionally adds the 'cgicarp' code to the mix. 
>Does that seem reasonable or does it just sound like a hack?
>
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 ;)

- Steve


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

--- Application.pm.orig	2004-02-04 16:42:42.000000000 +0000
+++ Application.pm	2004-02-12 09:05:06.496512600 +0000
@@ -8,16 +8,26 @@
 
 
 sub import {
-       my $cgicarp = 1;
-       foreach (@_) { $cgicarp = 0 if /^-nocgicarp$/io }
-       if ($cgicarp) {
-               require CGI::Carp;
-               CGI::Carp->import();
-       }
-       else {
-               require Carp;
-               Carp->import();
-       }
+	my $class = $_[0];
+	my $cgicarp = 1;
+	my $i = 1;
+	while ($i < @_) {
+		if ($_[$i] =~ /^-nocgicarp$/io) {
+			splice @_, $i, 1;
+			$cgicarp = 0;
+			next;
+		}
+		++$i;
+	}
+	if ($cgicarp) {
+		require CGI::Carp;
+		CGI::Carp->import();
+	}
+	else {
+		require Carp;
+		Carp->import();
+	}
+	goto &Exporter::import if $class->isa('Exporter');
 }
 
 

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