From 9322e7b34cf3562f8bee1edf4d361a6fa7bc849a Mon Sep 17 00:00:00 2001 From: Zaahir Moolla Date: Thu, 26 Jan 2017 10:45:49 -0500 Subject: [PATCH] Calculator: Improve triggering and execution (#3894) * Update trigger Use Safe for evaluation with the minimum functions Removed unnecessary capturing, redundant function calls * Add some tests * Remove eval since we don't allow die Add comment about debugging failed reval * Add rv2gv from :base_orig to allow compat with Safe 2.35 Remove prtf since that was in :base_orig which was removed * Remove unneeded opsets * Silence all of reval's warnings * Add STRICT --- lib/DDG/Goodie/Calculator.pm | 81 ++++++++++++++++++++++-------------- t/Calculator.t | 4 ++ 2 files changed, 54 insertions(+), 31 deletions(-) diff --git a/lib/DDG/Goodie/Calculator.pm b/lib/DDG/Goodie/Calculator.pm index 29857c2cd1a..b9d1a09122e 100644 --- a/lib/DDG/Goodie/Calculator.pm +++ b/lib/DDG/Goodie/Calculator.pm @@ -5,34 +5,23 @@ use strict; use DDG::Goodie; with 'DDG::GoodieRole::NumberStyler'; -use List::Util qw( max ); +use List::Util 'max'; use Math::Trig; use Math::BigInt; +use Safe; + use utf8; -zci answer_type => "calc"; +zci answer_type => 'calc'; zci is_cached => 1; -triggers query_nowhitespace => qr< - ^ - ( what is | calculate | solve | math )? - - [\( \) x X × ∙ ⋅ * % + ÷ / \^ \$ -]* - - (?: [0-9 \. ,]* ) - (?: gross | dozen | pi | e | c | squared | score |) - [\( \) x X × ∙ ⋅ * % + ÷ / \^ 0-9 \. , _ \$ -]* - - (?(1) (?: -? [0-9 \. , _ ]+ |) |) - (?: [\( \) x X × ∙ ⋅ * % + ÷ / \^ \$ -] | times | divided by | plus | minus | fact | factorial | cos | sin | tan | cotan | log | ln | log[_]?\d{1,3} | exp | tanh | sec | csc | squared | sqrt | pi | e )+ - - (?: [0-9 \. ,]* ) - (?: gross | dozen | pi | e | c | squared | score |) - - [\( \) x X × ∙ ⋅ * % + ÷ / \^ 0-9 \. , _ \$ -]* =? - - $ - >xi; +triggers query_nowhitespace => qr' + (?: [() x X × ∙ ⋅ * % + \- ÷ / \^ \$ 0-9 \. ,] | + times | divided by | plus | minus | fact | factorial | cos | + sin | tan | cotan | log | ln | log_?\d{1,3} | exp | tanh | + sec | csc | squared | sqrt | pi | e | gross | dozen | pi | + | score){2,} +'xi; my $number_re = number_style_regex(); my $funcy = qr/[[a-z]+\(|log[_]?\d{1,3}\(|\^|\*|\/|squared|divided/; # Stuff that looks like functions. @@ -68,6 +57,28 @@ my $ip4_regex = qr/(?:$ip4_octet\.){3}$ip4_octet/; # Th my $up_to_32 = qr/([1-2]?[0-9]{1}|3[1-2])/; # 0-32 my $network = qr#^$ip4_regex\s*/\s*(?:$up_to_32|$ip4_regex)\s*$#; # Looks like network notation, either CIDR or subnet mask +my $safe = new Safe; +$safe->permit_only(qw' + :base_core :base_math + rv2gv require caller padany +'); + +$safe->deny(qw'warn die'); + +$safe->share_from('Math::Trig', [qw'csc sec tanh tan cotan']); +$safe->share_from('main', [qw' + Math::BigInt::modify + Math::BigInt::bzero + Math::BigInt::bfac + Math::BigInt::bstr + Math::BigInt::new + Math::BigInt::round + Math::BigInt::Calc::_new + Math::BigInt::Calc::_str + Math::BigInt::Calc::_zero + Math::BigInt::Calc::_fac +']); + handle query_nowhitespace => sub { my $query = $_; @@ -77,12 +88,11 @@ handle query_nowhitespace => sub { return if ($query =~ /^(?:(?:\+?1\s*(?:[.-]\s*)?)?(?:\(\s*([2-9]1[02-9]|[2-9][02-8]1|[2-9][02-8][02-9])\s*\)|([2-9]1[02-9]|[2-9][02-8]1|[2-9][02-8][02-9]))\s*(?:[.-]\s*)?)([2-9]1[02-9]|[2-9][02-9]1|[2-9][02-9]{2})\s*(?:[.-]\s*)?([0-9]{4})(?:\s*(?:#|x\.?|ext\.?|extension)\s*(\d+))?$/); # Probably are searching for a phone number, not making a calculation $query =~ s/^(?:whatis|calculate|solve|math)//; - $query =~ s/(factorial)/fact/; #replace factorial with fact + $query =~ s/factorial/fact/; #replace factorial with fact # Grab expression. my $tmp_expr = spacing($query, 1); - return if $tmp_expr eq $query; # If it didn't get spaced out, there are no operations to be done. # First replace named operations with their computable equivalents. @@ -108,10 +118,18 @@ handle query_nowhitespace => sub { return unless $style; $tmp_expr = $style->for_computation($tmp_expr); - $tmp_expr =~ s/(Math::BigInt->new\((.*)\))/(Math::BigInt->new\($2\))->bfac()/g; #correct expression for fact + $tmp_expr =~ s/Math::BigInt->new\(([^)]+)\)/Math::BigInt->new\($1\)->bfac->bstr/g; #correct expression for fact # Using functions makes us want answers with more precision than our inputs indicate. my $precision = ($query =~ $funcy) ? undef : ($query =~ /^\$/) ? 2 : max(map { $style->precision_of($_) } @numbers); - my $tmp_result = eval($tmp_expr); + my $tmp_result; + # e.g. sin(100000)/100000 completely makes this go haywire. + { + # we don't care about reval's warnings + local $SIG{__WARN__} = sub {}; + $tmp_result = $safe->reval($tmp_expr, 'STRICT'); + } + # if you want to see why $tmp_expr wasn't evaluated, uncomment the following + # warn "reval failed: $@"; # Guard against non-result results return unless (defined $tmp_result && $tmp_result ne 'inf' && $tmp_result ne ''); @@ -140,17 +158,18 @@ sub prepare_for_display { $query =~ s/(\d)[ _](\d)/$1$2/g; # Squeeze out spaces and underscores. # Show them how 'E' was interpreted. This should use the number styler, too. $query =~ s/((?:\d+?|\s))E(-?\d+)/\($1 * 10^$2\)/i; - $query =~ s/\s*\*{2}\s*/^/g; # Use prettier exponentiation. - $query =~ s/(Math::BigInt->new\((.*)\))/fact\($2\)/g; #replace Math::BigInt->new( with fact( + $query =~ s/\s*\*\*\s*/^/g; # Use prettier exponentiation. + $query =~ s/Math::BigInt->new\(([^)]+)\)/fact\($1\)/g; #replace Math::BigInt->new( with fact( $result = $style->for_display($result); foreach my $name (keys %named_constants) { $query =~ s#\($name\)#$name#xig; } + my $spaced_query = spacing($query); return +{ - text => spacing($query) . ' = ' . $result, + text => $spaced_query . ' = ' . $result, structured => { - input => [spacing($query)], + input => [$spaced_query], operation => 'Calculate', result => "" . $style->with_html($result) . "" }, @@ -166,7 +185,7 @@ sub spacing { $text =~ s/(\s*(? undef, '01780-111-111x400' => undef, '(01780) 111 111' => undef, + 'warn "hi"; 1 + 1' => undef, + 'die "killed"; 1 + 3' => undef, + '1 + 1; die' => undef, + '`ls -al /`; 3 * 4' => undef ); done_testing;