-2

In a CGI script that processes arrays of strings the user can supply for each field whether to match a string there or not. If all user-specified criteria are fulfilled, the specific array should be output.

As the test function depends on the user input, I thought I'd avoid checking the user input each time, and build a test function at runtime that only tests for the values the user had specified.

Unfortunately it never worked and in a debug session I found out that the closure returns the expression as string instead of evaluation it.

Example:

  DB<10> x $matcher->(\@fields)
0  '$_[0]->[0] =~ /Zi/ && $_[0]->[1] =~ /A/'
  DB<11> x eval $matcher->(\@fields)
0  ''

The code fragments to construct the closure are:

# collect the specified fields in @matches like this
# (the search_fields strings use the same index as the target):
for (my $i = 0; $i <= $#search_fields; ++$i) {
    push(@matches, "\$_[0]->[$i] =~ /$search_fields[$i]/")
        if (defined $search_fields[$i] && length($search_fields[$i]) > 0);
}
# so the content might be (simple case):
#  DB<13> x @search_fields
#0  'Zi'
#1  'A'
#2  undef
#3  undef
#4  undef
#5  undef
#6  undef
#7  undef
#8  undef
#9  undef
#10  undef

#  DB<7> x  @_matches
#0  '$_[0]->[0] =~ /Zi/'
#1  '$_[0]->[1] =~ /A/')
# (the array given as parameter has the indices to compare pre-computed)

# construction of the closure using eval
eval {
    $matcher = sub ($) { join(' && ', @matches) }
} or $matcher = sub ($) { 0 };

# I also had tried "$matcher = eval { sub ($) { join(' && ', @matches) } }" before
# And for performance reasons I want to avoid using eval in the sub itself

# calling the closure:
if ($select = $matcher->(\@fields)) {
    # record matches criteria
}

As it is now, the closure matches every record. How to do it correctly and efficiently (while still being maintainable (number and order of fields may change))?

2
  • 2
    Your code suffers from a code injection which can be fixed by using push( @matches, qq{do { my $regex = "\Q$search_fields[$i]\E"; "\$_[0]->[$i] =~ /\$regex/ }}). And that's assuming $search_fields[$i] contains regex patterns. If it doesn't, you have a second code injection bug which can be fixed by using push( @matches, qq{do { my $val = "\Q$search_fields[$i]\E"; "\$_[0]->[$i] =~ /\\Q\$val\\E/ }}). Commented Sep 17 at 14:28
  • (There's an extraneous " before \$_[0] in the previous comment) Commented Sep 17 at 16:40

2 Answers 2

2

Build a string that represents the function definition, then eval it.

$matcher = eval 'sub ($) { ' . join(' && ', @matches) . '}';

Note that evaluating user input is dangerous. What happens when the user enters

/;system"rm\x20-rf\x20/";/

?

You can still avoid string eval (and block eval as well):

for (my $i = 0; $i <= $#search_fields; ++$i) {
    my $regex = $search_fields[$i];
    my $j = $i;
    push @matches, sub { $_[0][$j] =~ /$regex/ }
        if defined $search_fields[$i] && length $search_fields[$i];
}

my $matcher = sub { scalar @matches == grep $_->(@_), @matches };
Sign up to request clarification or add additional context in comments.

6 Comments

Originally I wanted to use the brace-variant of eval to protect against stupid syntax errors preventing eval, but it seems it doesn't work that way (I would need an eval within eval (something the shell's $(...) can do)).
Check the update. You don't need any eval.
But the grep variant will not stop on the first mis-match, thus causing poorer performance (the matcher is going to be applied a few thousand times), right?
First benchmark it to see. If it's slow, use all from List::Util or something similar.
Tip: Using the faster and cleaner for my $i ( 0 .. $#search_fields ) would save you from needing $j
See the comment I posted on the question for how to fix the mentioned code injection bug in the eval EXPR version.
-1

It seems the problem was I cannot use the brace-variant of eval: When using the quote-variant and supplying the full closure as a string, it worked. The code used was:

$matcher = eval( 'sub ($) { ' . join(' && ', @matches) . '}' ) || sub ($) { 0 };

1 Comment

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.