Re: cacti-0.8.8b: Multiple COMMENT arguments in "rrdtool graph" ??

On 08.08.2013 10:20, Marc-Christian Petersen wrote:
> the problem code is this (diff 0.8.8a -> 0.8.8b):
> 
> lib/rrd.php
> 
> @@ -1344,14 +1344,14 @@ function rrdtool_function_graph($local_g
> 
>                 if ($graph_item_types{$graph_item["graph_type_id"]} == "COMMENT") {
>                         if (read_config_option("rrdtool_version") != "rrd-1.0.x") {
> -                               $comment_string = $graph_item_types{$graph_item["graph_type_id"]} . ":\"" . str_replace(":", "\:", $graph_variables["text_format"][$graph_item_id]) . $hardreturn[$graph_item_id] . "\" ";
> +                               $comment_string = $graph_item_types{$graph_item["graph_type_id"]} . ":" . str_replace(":", "\:", cacti_escapeshellarg($graph_variables["text_format"][$graph_item_id] . $hardreturn[$graph_item_id])) . " ";
>                                 if (trim($comment_string) == 'COMMENT:"\n"') {
>                                         $txt_graph_items .= 'COMMENT:" \n"'; # rrdtool will skip a COMMENT that holds a NL only; so add a blank to make NL work
>                                 } else if (trim($comment_string) != "COMMENT:\"\"") {
>                                         $txt_graph_items .= rrd_substitute_host_query_data($comment_string, $graph, $graph_item);
>                                 }
>                         }else {
> -                               $comment_string = $graph_item_types{$graph_item["graph_type_id"]} . ":\"" . $graph_variables["text_format"][$graph_item_id] . $hardreturn[$graph_item_id] . "\" ";
> +                               $comment_string = $graph_item_types{$graph_item["graph_type_id"]} . ":" . cacti_escapeshellarg($graph_variables["text_format"][$graph_item_id] . $hardreturn[$graph_item_id]) . " ";
>                                 if (trim($comment_string) == 'COMMENT:"\n"') {
>                                         $txt_graph_items .= 'COMMENT:" \n"'; # rrdtool will skip a COMMENT that holds a NL only; so add a blank to make NL work
>                                 } else if (trim($comment_string) != "COMMENT:\"\"") {
> 
> unapplying this fixes the "no empty comments" problem.

Thanks for pointing out the location of the error.
It is not recommended to undo this change, due to security reasons (command line
injection).
I'm working out a different fix, that mostly will keep the old behavior. There's
only one corner case:

Having an empty COMMENT with a HR should result in a "empty line in the legend".
This requires the rrdtool statement to have sth like
"COMMENT: '\n'" (an whitespace has to be added)
But what should an empty COMMENT without a HR yield? Currently, this will go
"COMMENT: ", which includes a whitespace even though not entered by the user.
A different solution would be to skip an empty COMMENT without a HR completely.
I suppose that this is the better solution, even if it does not mimic the old
(wrong?) behavior.

Thoughts?
Reinhard

------------------------------------------------------------------------------
Get 100% visibility into Java/.NET code with AppDynamics Lite!
It's a free troubleshooting tool designed for production.
Get down to code-level detail for bottlenecks, with <2% overhead. 
Download for free and get started troubleshooting in minutes. 
http://pubads.g.doubleclick.net/gampad/clk?id=48897031&iu=/4140/ostg.clktrk