Re: [GT] Input validation in Tools.pm

["Robert A. Schmied" <ras AT acm.org>]

João Costa wrote:
> I was just playing with a new installation of GT. One of the issues I came
> across is the available_timeframes directive supported by genericdbi.pm.  I
> was putting invalid timeframe names in that and was getting some cryptic
> warnings as a result.
>
> The attached patch validates that situation.
>
>
>
> ------------------------------------------------------------------------
>
> --- /tmp/Tools-HEAD.pm	2008-07-01 17:39:28.000000000 +0100
> +++ GT/Tools.pm	2008-07-01 15:24:33.000000000 +0100
> @@ -435,8 +435,13 @@
>  die("Parameter \$db not set in get_timeframe_data") if (!defined($db));
>  
>  if (defined($available_timeframes)) {
> -	foreach (split(',', $available_timeframes)) {
> -		push @tf, GT::DateTime::name_to_timeframe($_);
> +	foreach my $tf_name (split(',', $available_timeframes)) {
> +	        my $tf_code = GT::DateTime::name_to_timeframe($tf_name);
> +		push @tf, $tf_code;
> +		if (!defined($tf_code)) {
> +		    my $tfs = join("\n\t", map(GT::DateTime::name_of_timeframe($_), GT::DateTime::list_of_timeframe));
> +		    die("Invalid timeframe name in available_timeframes configuration item: $tf_name\n\nValid timeframes are: \n\t" . $tfs . "\n\n");
> +		}
>  	}
>  	@tf = sort(@tf);
>  } else {


aloha joao

looks like that code came from a change on Date: 2006-11-20 01:25:48 +0100 (Mon, 20 Nov 2006)
at revision 513. you seem to be in the group that uses genericdbi.pm; have you done enough
testing to be convinced it's a complete fix? my tests are inconclusive, meaning i see no
difference with and without it as i discuss below.

can you share the command line, and other configured data that lead you
to this problem/solution? i'd like to see if it can be duplicated using
beancounter db and bean.pm module as well as a text file db and Text.pm.

without the change with beancounter and bean.pm:
ras [ 3908 ] %    display_indicator.pl -ti 5min  --sta 2008-06-23 I:Prices ENS  
Indicator I:Prices has 1 value ... all values selected
       I:Prices/1  <=> Prices[]
Can't call method "timeframe" on an undefined value at ../GT/Calculator.pm line 70.

interestingly i get 'results' using sample text db and Text.pm
ras [ 3956 ] %    display_indicator.pl -ti 5min  I:Prices 13000
Indicator I:Prices has 1 value ... all values selected
       I:Prices/1  <=> Prices[]

       timeframe 5min, time periods 2224 .. 2424
Calculating indicator ...
display_indicator.pl: interval: 2224 .. 2424
Prices[]            [2001-12-14 00:00:00] = 18.7400
Prices[]            [2001-12-17 00:00:00] = 19.7800
.
.
.

with the change (a bit rearranged) notice the $_ in join should be $tf_name i think
(but my version didn't create a separate variable for tf_name) i got same output.
is there some recent change that uses method get_timeframe_data that i've
missed or is it specific to genericdbi.pm?

do you intend to commit or would you prefer i do it? if the latter,
i would be inclined to re-format the error message code some in an
effort to keep line lengths about 80 chars.


ras

here's the code fragment i used in my test+eval version
   if ( ! defined($available_timeframes) ) {
       @tf = GT::DateTime::list_of_timeframe();
   } else {
       foreach (split(',', $available_timeframes)) {
           my $tf_code = GT::DateTime::name_to_timeframe($_);
           if (!defined($tf_code)) {
               my $tfs = join("\n\t", map(GT::DateTime::name_of_timeframe($_),
                GT::DateTime::list_of_timeframe()));
               my $msg = "Invalid timeframe name in available_timeframes"
                . " configuration item: $_\n\nValid timeframes are: \n\t"
                . "$tfs\n\n";
               die "$msg";
           }
           push @tf, $tf_code;
       }
       @tf = sort(@tf);
   }