fix(scripts): return 'U' on error in ss_webseer, ss_gexport, query_host_cpu (#6941)

* fix(scripts): return 'U' on error in ss_webseer, ss_gexport, query_host_cpu

Data source scripts must return 'U' (RRDtool unknown) on error rather than
'0', which is a valid data value and silently poisons graphs.

- ss_webseer.php: replace empty() guard with strict $value === '' comparison
  (empty('0') is true in PHP, corrupting a valid zero reading); return 'U'
  on fallthrough instead of false
- ss_gexport.php: return 'U' on fallthrough instead of null
- query_host_cpu.php: print 'U' when requested index absent in get handler

Adds DatasourceScriptErrorReturnTest.php with source-scan tests for each
corrected return path including regression guards for the empty() trap.
Uses tab indentation to match repo conventions.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* fix(ss_webseer): catch false/null returns from db_fetch_cell_prepared

db_fetch_cell_prepared() returns false when the query finds no row or
fails. The previous check ($value === '') passed false and null through
as-is, returning a non-'U' value to rrdtool and causing NaN spikes.
Extend the guard to treat false and null the same as empty string.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* test(scripts): add GPL header to DatasourceScriptErrorReturnTest

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

* test(scripts): add false/null guard tests for ss_gexport and ss_webseer

Adds source-scan assertions verifying both scripts handle false and null
returns from db_fetch_cell_prepared, and a behavioural dataset confirming
the guard expression maps all three sentinel values to 'U'.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

---------

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Co-authored-by: TheWitness <thewitness@cacti.net>
This commit is contained in:
Thomas VincentandTheWitness authored and GitHub committed 2026-04-04 12:43:03 -04:00
1 parent 1dd6339aee
commit a515cea185
4 files changed
+111 -6

No files matched your search

+3
View File
@@ -101,6 +101,9 @@ if ($cmd == 'index') {
if (isset($arr_index[$index])) {
print $arr[$index];
} else {
// Index absent — return U so RRDtool records an unknown sample.
print 'U';
}
}
+3 -3
View File
@@ -71,7 +71,7 @@ function ss_gexport(string $cmd = 'index', string $arg1 = '', string $arg2 = '')
} elseif ($cmd == 'get') {
$arg = $arg1;
$index = $arg2;
$value = '0';
$value = 'U';
if (db_table_exists('graph_exports')) {
switch($arg) {
@@ -92,8 +92,8 @@ function ss_gexport(string $cmd = 'index', string $arg1 = '', string $arg2 = '')
}
}
return ($value == '' ? '0' : $value);
return ($value === '' || $value === false || $value === null ? 'U' : $value);
}
return null;
return 'U';
}
+3 -3
View File
@@ -69,7 +69,7 @@ function ss_webseer(string $cmd = 'index', string $arg1 = '', string $arg2 = '')
} elseif ($cmd == 'get') {
$arg = $arg1;
$index = $arg2;
$value = '0';
$value = 'U';
if (db_table_exists('plugin_webseer_urls')) {
switch($arg) {
@@ -125,8 +125,8 @@ function ss_webseer(string $cmd = 'index', string $arg1 = '', string $arg2 = '')
}
}
return (empty($value) ? '0' : $value);
return ($value === '' || $value === false || $value === null ? 'U' : $value);
}
return false;
return 'U';
}
@@ -0,0 +1,102 @@
<?php
/*
+-------------------------------------------------------------------------+
| Copyright (C) 2004-2026 The Cacti Group |
| |
| This program is free software; you can redistribute it and/or |
| modify it under the terms of the GNU General Public License |
| as published by the Free Software Foundation; either version 2 |
| of the License, or (at your option) any later version. |
+-------------------------------------------------------------------------+
| Cacti: The Complete RRDtool-based Graphing Solution |
+-------------------------------------------------------------------------+
*/
/**
* Source-scan tests verifying that data source scripts return 'U' on error.
*
* Cacti project rule: data source scripts must return 'U' for unknown/error
* samples, never '0'. Returning '0' causes RRDtool to graph a false zero data
* point rather than an unknown sample, silently corrupting gauge metrics.
*/
test('ss_webseer.php does not initialise $value to 0', function () {
$src = file_get_contents(__DIR__ . '/../../scripts/ss_webseer.php');
expect($src)->not->toContain("\$value = '0'")
->and($src)->toContain("\$value = 'U'");
});
test('ss_webseer.php returns U not 0 on empty/missing value', function () {
$src = file_get_contents(__DIR__ . '/../../scripts/ss_webseer.php');
expect($src)->not->toContain("? '0' : \$value")
->and($src)->toContain("? 'U' : \$value")
// empty('0') === true in PHP; guard must use strict string comparison
->and($src)->not->toContain('empty($value)')
->and($src)->toContain("\$value === ''");
});
test('ss_webseer.php does not return false on fallthrough', function () {
$src = file_get_contents(__DIR__ . '/../../scripts/ss_webseer.php');
expect($src)->not->toContain('return false');
});
test('ss_gexport.php does not initialise $value to 0', function () {
$src = file_get_contents(__DIR__ . '/../../scripts/ss_gexport.php');
expect($src)->not->toContain("\$value = '0'")
->and($src)->toContain("\$value = 'U'");
});
test('ss_gexport.php returns U not 0 on empty value', function () {
$src = file_get_contents(__DIR__ . '/../../scripts/ss_gexport.php');
expect($src)->not->toContain("? '0' : \$value")
->and($src)->toContain("? 'U' : \$value");
});
test('ss_gexport.php does not return null on fallthrough', function () {
$src = file_get_contents(__DIR__ . '/../../scripts/ss_gexport.php');
expect($src)->not->toContain('return null');
});
test('query_host_cpu.php prints U when get index is absent', function () {
$src = file_get_contents(__DIR__ . '/../../scripts/query_host_cpu.php');
// The get handler must have an else branch that prints 'U'; a comment
// containing the literal would satisfy toContain but not the structural check.
expect($src)->toContain("print 'U'")
->and($src)->toContain('} else {')
->and($src)->not->toContain("/* print 'U'");
});
test('ss_webseer.php guard covers false and null from db_fetch_cell_prepared', function () {
$src = file_get_contents(__DIR__ . '/../../scripts/ss_webseer.php');
expect($src)->toContain('$value === false')
->and($src)->toContain('$value === null');
});
test('ss_gexport.php guard covers false and null from db_fetch_cell_prepared', function () {
$src = file_get_contents(__DIR__ . '/../../scripts/ss_gexport.php');
expect($src)->toContain('$value === false')
->and($src)->toContain('$value === null');
});
// Behavioural tests for the return guard expression used in both scripts:
// ($value === '' || $value === false || $value === null ? 'U' : $value)
// Tested via an inline closure so no DB connection is required.
dataset('datasource_guard_cases', [
'non-empty string passes through' => ['123', '123'],
'empty string maps to U' => ['', 'U'],
'false maps to U' => [false, 'U'],
'null maps to U' => [null, 'U'],
'numeric 0 passes through' => [0, 0],
]);
test('datasource return guard behaves correctly', function ($value, $expected) {
$guard = static fn ($v) => ($v === '' || $v === false || $v === null ? 'U' : $v);
expect($guard($value))->toBe($expected);
})->with('datasource_guard_cases');
test('datasource return guard does not throw on non-scalar input', function () {
$guard = static fn ($v) => ($v === '' || $v === false || $v === null ? 'U' : $v);
// Arrays pass strict === checks without error; result is the array itself.
expect($guard([]))->toBe([]);
});