Public bug reported:

Summary of changes:

The code to which this comment refers causes numerous PHP warnings to be issued 
each time the script is executed on systems on which PHP open_basedir 
restrictions are in effect. If the location of the rrdtool executable were 
defined as a configuration directive, a) the warnings would disappear and b) 
the application would perform more efficiently (the filesystem is not queried 
for directories that will not exist).
------------------
105c105,109
< 
---
>       
>       # XXX TODO
>       # This path should be defined as a configuration directive, so that
>       # open_basedir warnings are not thrown when the paths don't exist.
>       
------------------

Wrapping this block in conditional logic prevents code from being executed when 
its expected inputs are not set (i.e., when an rrdtool graph is not generated 
for some reason).
------------------
130,143c134,149
< 
<               if (preg_match("/(\d+)x(\d+)/", $output[0], $matches)) {
<                       $retval["xsize"] = $matches[1];
<                       $retval["ysize"] = $matches[2];
<               } else {
<                       $rrd_error_val = $output[0];
<                       return false;
<               }
< 
<               array_shift($output);
< 
<               $retval["calcpr"] = array();
<               foreach ($output as $index => $value) {
<                       $retval["calcpr"][$index] = $value;
---
>               
>               if (is_array($output) && !empty($output)) {
>                       if (preg_match("/(\d+)x(\d+)/", $output[0], $matches)) {
>                               $retval["xsize"] = $matches[1];
>                               $retval["ysize"] = $matches[2];
>                       } else {
>                               $rrd_error_val = $output[0];
>                               return false;
>                       }
>                       
>                       array_shift($output);
>       
>                       $retval["calcpr"] = array();
>                       foreach ($output as $index => $value) {
>                               $retval["calcpr"][$index] = $value;
>                       }
------------------

Suppressing warnings and errors in this way is discouraged, as it makes 
debugging the application extremely difficult. There are other instances of 
this approach elsewhere within the code, but they are less problematic because 
application-specific errors are still triggered.
------------------
292c299
<       $readfile = @file($seenfile);
---
>       $readfile = file($seenfile);
------------------

This change addresses the mysterious output just below the "Generated by
amavis-stats..." line:

amavis-stats::error: rrd_graph():

This output is caused in /usr/share/amavis-
stats/templates/standard/overall_footer.tpl, line 10:

<td align="left">{OUT_MSG}</td>

If we trace this variable back to its origin, we find that it is set in
/usr/share/amavis-stats/includes/page_tail.php, line 35:

'OUT_MSG' => $out_msg ? $out_msg : NULL)

Now we must hunt-down $out_msg, which turns-out to be set in /usr/share
/amavis-stats/index.php, line 250:

function asErr($txt = "") {
        global $as_pkg, $out_msg;
        $out_msg .= "$as_pkg::error: $txt<br>\n";
}

After some addition running-around, we find that the true origin of this
message is in index.php, line 893:

$ret = pre_graph($img, $opts, count($opts));

$infected = 0;

if (!is_array($ret)) {
        $msg = rrd_error();
        asErr("rrd_graph(): $msg");
        return false;
}

We can see that the author's pre_graph() function is returning something
other than an array: boolean false. Reading the relevant comments within
the source code reveals that this message is likely the result of there
being no virus data to graph (which is why the function returns false).
This theory coincides with a message that is output just below the graph
image: NO VIRUS DATA TO GRAPH.

In summary, this is more of a warning than an error. The code should be 
modified such that the warning is printed only when rrdtool actually returns an 
error, in which case calling PHP's rrd_error function yields something other 
than null:
------------------
899c906,908
<               asErr("rrd_graph(): $msg");
---
>               if (!empty($msg)) {
>                       asErr("rrd_graph(): $msg");
>               }
------------------

This change addresses the fact that the original code does not exclude the '..' 
directory operator that PHP's readdir() function returns, which is problematic 
in this context.
------------------
1053c1062
<       if ($file != "." && is_dir("$libdir/$file")) {
---
>       if ($file != "." && $file != '..' && is_dir("$libdir/$file")) {
1069c1078
<       if ($file != "." && is_dir("$libdir/$file")) {
---
>       if ($file != "." && $file != '..' && is_dir("$libdir/$file")) {
------------------

This addresses an unset variable warning.
------------------
1091a1101,1103
> }
> else {
>       $legend_msg = '';
------------------


------------------ SYSTEM INFORMATION ------------------

# lsb_release -rd
Description: Ubuntu 10.04.4 LTS
Release: 10.04

# apt-cache policy amavis-stats
amavis-stats:
  Installed: 0.1.22-1
  Candidate: 0.1.22-1
  Version table:
 *** 0.1.22-1 0
        500 http://us.archive.ubuntu.com/ubuntu/ lucid/universe Packages
        100 /var/lib/dpkg/status

** Affects: amavis-stats (Ubuntu)
     Importance: Undecided
         Status: New

-- 
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
https://bugs.launchpad.net/bugs/963254

Title:
  PHP code throws warnings unnecessarily in several places

To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu/+source/amavis-stats/+bug/963254/+subscriptions

-- 
ubuntu-bugs mailing list
ubuntu-bugs@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs

Reply via email to