Eval() of code within classes?

Hi Folks,

My apologies for a somewhat long post.

I have a perl class, call it MyAPI.  It exports one function, called
Init(), which simply returns TRUE.  The final MyAPI product will export
200+ functions.

Now, consider this test class:
 ########################################
#
#!/usr/local/bin/perl -w

package APITest;

use strict;
use MyAPI;
use Exporter;

use vars qw( @ISA @EXPORT );
@ISA = qw( Exporter );

@EXPORT = qw
(
DoRequest
) ;

my $api;

sub new
{
my $self = {};
bless $self;

$api = MyAPI->new();
return $self;
}

sub DoRequest
{
my ($self) = @_;
my (
$returnval,
$response
) ;

#    $api;
eval "\$returnval = \$api\-\>Init();";
if ($@)
{
$response = $@;
}
else
{
$response = "Success!\n";
}
return $response;
}
1;
##################################

... and a quick test.pl file to try it out:

##################################
use APITest;

my $xmltest = new APITest;

$response = $xmltest->DoRequest();
print $response;
##################################

OK, so the test class has a global variable, $api, which gets
initialized as a new MyAPI in the constructor.  Then, in DoRequest, we
have this:

eval "\$returnval = \$api\-\>Init();";
# i.e, run the code '$returnval = $api->Init();'

(Yes, I know you're all wondering why all these hoops. Bear with me,
the code I'm presenting here is a small part of a big project.)

The problem is, when you run the test.pl program, you get:

"Can't call method "Init" on an undefined value at (eval 2) line
1."

Apparantly, $api is undefined within that eval() expression.  However,
if you do anything to reference $api directly, before the evel, the
code works and returns "Success!"  Note the commented line above the
eval():

#    $api;

Just uncommenting that line is sufficient to make the code work.

So, my question is, what is going on inside DoRequest()?  Why is $api
apparantly undefined within the eval(), when doing anything to
reference it somehow magically makes it visible?  I'm hoping those with
more experience than me in class/variable/scoping issues will see
something obvious here.  I'm still new at object-oriented programming
in Perl (been doing old-fashioned procedural stuff with it for
years...)

Thanks in advance!
Michael

macapone@yahoo.com wrote:
> Hi Folks,
>
> My apologies for a somewhat long post.
>
> I have a perl class, call it MyAPI.  It exports one function, called
> Init(), which simply returns TRUE.  The final MyAPI product will export
> 200+ functions.
>

<snip code>
>
> OK, so the test class has a global variable, $api, which gets
> initialized as a new MyAPI in the constructor.  Then, in DoRequest, we
> have this:
>
>       eval "\$returnval = \$api\-\>Init();";
>       # i.e, run the code '$returnval = $api->Init();'
>
> (Yes, I know you're all wondering why all these hoops. Bear with me,
> the code I'm presenting here is a small part of a big project.)
>
> The problem is, when you run the test.pl program, you get:
>
>      "Can't call method "Init" on an undefined value at (eval 2) line
> 1."
Try using eval BLOCK instead:

eval {$returnval = $api->Init();};

Is more efficient (read the docs for eval), and still traps errors.

>
> Apparantly, $api is undefined within that eval() expression.  However,
> if you do anything to reference $api directly, before the evel, the
> code works and returns "Success!"  Note the commented line above the
> eval():
>
>       #    $api;

I'm not going to pretend I understand the scoping issues here: I *would*
guess it had something to do with closure, but then I'd expect to get
"variable will not stay shared issues".

I'd argue that $api shouldn't be a global variable anyway, and should
just be a field of APITest, especially since APITest isn't written as a
singleton.

Mark

<macapone@yahoo.com> wrote in comp.lang.perl.misc:
> Hi Folks,
>
> My apologies for a somewhat long post.

Could have been shorter.  See below.

> I have a perl class, call it MyAPI.  It exports one function, called

Classes normally don't export functions.  It's not necessarily wrong,
it can even be useful, but in your case I don't see an advantage.

> Init(), which simply returns TRUE.  The final MyAPI product will export
> 200+ functions.
>
> Now, consider this test class:
>  ########################################
#
>   #!/usr/local/bin/perl -w
>
>   package APITest;
>
>   use strict;
>   use MyAPI;
>   use Exporter;
>
>   use vars qw( @ISA @EXPORT );
>   @ISA = qw( Exporter );
>
>   @EXPORT = qw
>   (
>       DoRequest
>   ) ;

Another unnecessary exportation.

>
>   my $api;
>
>   sub new
>   {
>       my $self = {};
>       bless $self;

That should be "bless $self, shift", otherwise the ->new method can't
be usefully inherited.

>       $api = MyAPI->new();
>       return $self;
>   }
>
>   sub DoRequest
>   {
>       my ($self) = @_;
>       my (
>           $returnval,
>           $response
>          ) ;
>
>   #    $api;
>       eval "\$returnval = \$api\-\>Init();";
>       if ($@)
>       {
>           $response = $@;
>       }
>       else
>       {
>           $response = "Success!\n";
>       }
>       return $response;
>   }
>   1;
> ##################################
>
> ... and a quick test.pl file to try it out:
>
> ##################################
>   use APITest;
>
>   my $xmltest = new APITest;
>
>   $response = $xmltest->DoRequest();
>   print $response;
> ##################################
>
> OK, so the test class has a global variable, $api, which gets
> initialized as a new MyAPI in the constructor.  Then, in DoRequest, we
> have this:
>
>       eval "\$returnval = \$api\-\>Init();";
>       # i.e, run the code '$returnval = $api->Init();'
>
> (Yes, I know you're all wondering why all these hoops. Bear with me,
> the code I'm presenting here is a small part of a big project.)
>
> The problem is, when you run the test.pl program, you get:
>
>      "Can't call method "Init" on an undefined value at (eval 2) line
> 1."
>
> Apparantly, $api is undefined within that eval() expression.  However,
> if you do anything to reference $api directly, before the evel, the
> code works and returns "Success!"  Note the commented line above the
> eval():
>
>       #    $api;
>
> Just uncommenting that line is sufficient to make the code work.

Why, it's a bug.  Just accessing a variable shouldn't make a
difference as to whether is it defined or not.

The bug can be demonstrated more concisely:

File MyLib.pm:

package MyLib;
use strict; use warnings;

my $var;

sub set_it { $var = 123 };
sub see_it {
#   $var = $var; # uncomment this
eval 'print defined $var ? "defined\n" : "not defined\n"';
}

1;

File ttt:

#!/usr/bin/perl
use strict; use warnings; $| = 1; # @^~`

use MyLib;
MyLib::set_it;
MyLib::see_it;

Running ttt with MyLib.pm as given prints "not defined", but when the
line "$var = $var" is uncommented in MyLib.pm it prints "defined".

The bug is not triggered when the code from MyLib.pm is incorporated
into ttt, a separate library is needed.

Another workaround (besides mentioning $var) is to make "$var" ($api
in your code) a package variable.  However, the insecurities in your
OO code make me wonder if you really need string eval to achieve what
you want to do.  There may be other ways, but since you don't explain
why you (think you) need eval it's hard to say.

Anno

<macapone@yahoo.com> wrote in comp.lang.perl.misc:
> Hi Folks,
>
> My apologies for a somewhat long post.

Could have been shorter.  See below.

> I have a perl class, call it MyAPI.  It exports one function, called

Classes normally don't export functions.  It's not necessarily wrong,
it can even be useful, but in your case I don't see an advantage.

> Init(), which simply returns TRUE.  The final MyAPI product will export
> 200+ functions.
>
> Now, consider this test class:
>  ########################################
#
>   #!/usr/local/bin/perl -w
>
>   package APITest;
>
>   use strict;
>   use MyAPI;
>   use Exporter;
>
>   use vars qw( @ISA @EXPORT );
>   @ISA = qw( Exporter );
>
>   @EXPORT = qw
>   (
>       DoRequest
>   ) ;

Another unnecessary exportation.

>
>   my $api;
>
>   sub new
>   {
>       my $self = {};
>       bless $self;

That should be "bless $self, shift", otherwise the ->new method can't
be usefully inherited.

>       $api = MyAPI->new();
>       return $self;
>   }
>
>   sub DoRequest
>   {
>       my ($self) = @_;
>       my (
>           $returnval,
>           $response
>          ) ;
>
>   #    $api;
>       eval "\$returnval = \$api\-\>Init();";
>       if ($@)
>       {
>           $response = $@;
>       }
>       else
>       {
>           $response = "Success!\n";
>       }
>       return $response;
>   }
>   1;
> ##################################
>
> ... and a quick test.pl file to try it out:
>
> ##################################
>   use APITest;
>
>   my $xmltest = new APITest;
>
>   $response = $xmltest->DoRequest();
>   print $response;
> ##################################
>
> OK, so the test class has a global variable, $api, which gets
> initialized as a new MyAPI in the constructor.  Then, in DoRequest, we
> have this:
>
>       eval "\$returnval = \$api\-\>Init();";
>       # i.e, run the code '$returnval = $api->Init();'
>
> (Yes, I know you're all wondering why all these hoops. Bear with me,
> the code I'm presenting here is a small part of a big project.)
>
> The problem is, when you run the test.pl program, you get:
>
>      "Can't call method "Init" on an undefined value at (eval 2) line
> 1."
>
> Apparantly, $api is undefined within that eval() expression.  However,
> if you do anything to reference $api directly, before the evel, the
> code works and returns "Success!"  Note the commented line above the
> eval():
>
>       #    $api;
>
> Just uncommenting that line is sufficient to make the code work.

Why, it's a bug.  Just accessing a variable shouldn't make a
difference as to whether is it defined or not.

The bug can be demonstrated more concisely:

File MyLib.pm:

package MyLib;
use strict; use warnings;

my $var = 123;

sub see_it {
#   $var = $var; # uncomment this
eval 'print defined $var ? "defined\n" : "not defined\n"';
}

1;

File ttt:

#!/usr/bin/perl
use strict; use warnings; $| = 1; # @^~`

use MyLib;
MyLib::see_it;

Running ttt with MyLib.pm as given prints "not defined", but when the
line "$var = $var" is uncommented in MyLib.pm it prints "defined".

The bug is not triggered when the code from MyLib.pm is incorporated
into ttt, a separate library is needed.

Another workaround (besides mentioning $var) is to make "$var" ($api
in your code) a package variable.  However, the insecurities in your
OO code make me wonder if you really need string eval to achieve what
you want to do.  There may be other ways, but since you don't explain
why you (think you) need eval it's hard to say.

Anno

Interesting...  thanks for the simpler code to demonstrate the bug.

In your code, I discovered that changing  "my $var" to "our $var" also
gives it the necessary scope, as well as "use vars qw ($var)", which is
what I'm going to go with to get the job done.

As to why the string eval, the idea was that, with 200+ functions in
the API class, the idea would be to send in some xml like so:
<Request>
<Function>GetUserInfo</Function>
<Param Name='username'>mylogin</Param>
</Request>
... and then parse the xml, building the api call into a string, and
then eval()'ing the string.  Of course, in yesterday's design review,
the obvious security flaw became apparant, so we're re-thinking this
anyway (however, it would be running on a closed network).

<macapone@yahoo.com> wrote in comp.lang.perl.misc:
> Interesting...  thanks for the simpler code to demonstrate the bug.

Please quote some context.  For many readers (yours truly included) only
one posting is visible at a time.  If you don't have the thread by
heart, it's a hassle to find out *what* you find interesting.

> In your code, I discovered that changing  "my $var" to "our $var" also
> gives it the necessary scope, as well as "use vars qw ($var)", which is
> what I'm going to go with to get the job done.

Yes, it's access to the lexical pad that is hard to splice together
with a run-time eval.

> As to why the string eval, the idea was that, with 200+ functions in
> the API class, the idea would be to send in some xml like so:
> <Request>
>    <Function>GetUserInfo</Function>
>    <Param Name='username'>mylogin</Param>
> </Request>
> ... and then parse the xml, building the api call into a string, and
> then eval()'ing the string.  Of course, in yesterday's design review,
> the obvious security flaw became apparant, so we're re-thinking this
> anyway (however, it would be running on a closed network).

That seems to be a case for a dispatch table.  Instead of parsing the
call into a string, parse it into a list: function name followed by
parameters.

Instead of compiling a named subroutine for each function, compile them
anonymously in a hash where the key is the function name.  (That hash
is what's called a dispatch table.)  That would look like this:

my %functab = (
GetUserInfo => sub {
my $name = shift; # user name
..
},

AnotherFunction => sub {
# ...
},

# more functions
);

To call these, do this:

my $result;
my ( $func, @param) = parse_request( ...);
if ( $functab{ $func} ) {
$result = $functab{ $func}->( @param);
} else {
die "Invalid function '$func' requested";
}
# deal with $result

That way all the code that can be called is right in the source -- no
safety implications.

Anno