From cb0304f4bdd533ac35703a5db0a1c4de78635286 Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Sun, 16 Aug 2026 19:22:06 -0700 Subject: [PATCH 1/7] fix(rpn): correct the stack and set operators against rrdtool semantics EXC and REV were both no-ops, because popping already reverses the span and the code then reversed it back. AVG raised an uncaught TypeError when an unknown sample reached its running total, killing the poller mid-cycle; it now skips unknowns and yields UNKN when every sample is unknown, as rrdtool does. Refs #782 Signed-off-by: Thomas Vincent --- tests/Support/CactiStub.php | 143 ++++++++ tests/Unit/TholdCalculateExpressionTest.php | 131 +++++++ tests/Unit/TholdExpressionStackOpsTest.php | 200 +++++++++++ tests/Unit/TholdRpnCdefTest.php | 121 +++++++ tests/bootstrap.php | 366 ++++++++++++++++++++ tests/docker/Dockerfile | 28 ++ tests/docker/docker-compose.yml | 15 + thold_functions.php | 180 ++++++---- 8 files changed, 1112 insertions(+), 72 deletions(-) create mode 100644 tests/Support/CactiStub.php create mode 100644 tests/Unit/TholdCalculateExpressionTest.php create mode 100644 tests/Unit/TholdExpressionStackOpsTest.php create mode 100644 tests/Unit/TholdRpnCdefTest.php create mode 100644 tests/bootstrap.php create mode 100644 tests/docker/Dockerfile create mode 100644 tests/docker/docker-compose.yml diff --git a/tests/Support/CactiStub.php b/tests/Support/CactiStub.php new file mode 100644 index 00000000..17a5ab48 --- /dev/null +++ b/tests/Support/CactiStub.php @@ -0,0 +1,143 @@ +}> + */ + public static $calls = []; + + /** + * Queued return values, keyed by function name. Each call shifts one value + * off the front; an exhausted queue falls back to the type default. + * + * @var array> + */ + public static $returns = []; + + /** + * Values handed back by the get_*_request_var() family, keyed by var name. + * + * @var array + */ + public static $requestVars = []; + + /** + * Values handed back by read_config_option(), keyed by option name. + * + * @var array + */ + public static $configOptions = []; + + /** + * Messages passed to cacti_log(), in order. + * + * @var array + */ + public static $log = []; + + /** + * Clear all recorded and programmed state. + * + * @return void + */ + public static function reset() { + self::$calls = []; + self::$returns = []; + self::$requestVars = []; + self::$configOptions = []; + self::$log = []; + } + + /** + * Record one Cacti function call. + * + * @param string $fn Cacti function name. + * @param string $sql SQL text, or '' for non-query calls. + * @param array $params Bound parameters, if any. + * + * @return void + */ + public static function record($fn, $sql = '', array $params = []) { + self::$calls[] = ['fn' => $fn, 'sql' => $sql, 'params' => $params]; + } + + /** + * Queue one return value for the next call to $fn. + * + * @param string $fn Cacti function name. + * @param mixed $value Value to hand back. + * + * @return void + */ + public static function willReturn($fn, $value) { + self::$returns[$fn][] = $value; + } + + /** + * Take the next queued return value for $fn, or $default when none is left. + * + * @param string $fn Cacti function name. + * @param mixed $default Fallback when the queue is empty. + * + * @return mixed + */ + public static function nextReturn($fn, $default) { + if (!empty(self::$returns[$fn])) { + return array_shift(self::$returns[$fn]); + } + + return $default; + } + + /** + * All recorded calls to $fn. + * + * @param string $fn Cacti function name. + * + * @return array}> + */ + public static function callsTo($fn) { + return array_values(array_filter(self::$calls, function ($call) use ($fn) { + return $call['fn'] === $fn; + })); + } + + /** + * The recorded call log reduced to function names, in order. Useful for + * asserting transaction sequencing. + * + * @return array + */ + public static function callSequence() { + return array_column(self::$calls, 'fn'); + } +} diff --git a/tests/Unit/TholdCalculateExpressionTest.php b/tests/Unit/TholdCalculateExpressionTest.php new file mode 100644 index 00000000..e7bddf7a --- /dev/null +++ b/tests/Unit/TholdCalculateExpressionTest.php @@ -0,0 +1,131 @@ + 1, + 'name' => 'CPU', + 'local_data_id' => 4, + 'local_graph_id' => 7, + 'expression' => $expression, + ]; + + $reindexed = []; + $time_reindexed = []; + + return thold_calculate_expression($thold, 0, $reindexed, $time_reindexed); + } + + /** + * @return array + */ + public static function expressionProvider() { + return [ + 'addition' => ['2,3,+', 5], + 'subtraction' => ['8,2,-', 6], + 'multiplication' => ['4,3,*', 12], + 'nested' => ['2,3,+,4,*', 20], + 'single value' => ['7', 7], + ]; + } + + /** + * @dataProvider expressionProvider + * + * @param string $expression + * @param float|int $expected + * + * @return void + */ + public function testExpressionsReduceToTheTopOfTheStack($expression, $expected): void { + $this->assertEqualsWithDelta($expected, $this->evaluate($expression), 1.0e-9); + } + + /** + * An expression that leaves more than one value is an authoring error. It + * used to return the first operand pushed, which looks like a plausible + * reading and is compared against the bounds as though it were one. + * + * @return void + */ + public function testUnbalancedExpressionIsRejectedRatherThanReturningAnOperand(): void { + $this->assertSame(0, $this->evaluate('1,2,3,+')); + $this->assertTrue($GLOBALS['rpn_error']); + } + + /** + * @return void + */ + public function testUnsupportedTokenFailsTheExpression(): void { + $this->assertSame(0, $this->evaluate('2,NOSUCHOP')); + $this->assertTrue($GLOBALS['rpn_error']); + } + + /** + * @return array + */ + public static function specialTokenProvider() { + return [ + 'graph maximum' => ['CURRENT_GRAPH_MAXIMUM_VALUE'], + 'graph minimum' => ['CURRENT_GRAPH_MINIMUM_VALUE'], + ]; + } + + /** + * CURRENT_GRAPH_MAXIMUM_VALUE was missing from the dispatch list even + * though the evaluator handles it, so every expression using it failed as + * an unsupported field and returned zero. + * + * Reaching the handler is the whole assertion: what it then reads out of + * the RRD needs a live rrdtool and is not this test's concern. + * + * @dataProvider specialTokenProvider + * + * @param string $token + * + * @return void + */ + public function testSpecialGraphTokensReachTheirHandler($token): void { + try { + $this->evaluate($token); + } catch (Throwable $reached_the_rrd_layer) { + // Expected: the handler runs and asks rrdtool for a value. + } + + $unsupported = array_filter(CactiStub::$log, static function ($message) { + return strpos($message, 'Unsupported Field') !== false; + }); + + $this->assertSame([], array_values($unsupported)); + } +} diff --git a/tests/Unit/TholdExpressionStackOpsTest.php b/tests/Unit/TholdExpressionStackOpsTest.php new file mode 100644 index 00000000..acc49cb8 --- /dev/null +++ b/tests/Unit/TholdExpressionStackOpsTest.php @@ -0,0 +1,200 @@ + $stack + * @param string $operator + * + * @return array + */ + private function stackOp(array $stack, $operator) { + thold_expression_stackops_rpn($operator, $stack); + + return $stack; + } + + /** + * @param array $stack + * @param string $operator + * + * @return array + */ + private function setOp(array $stack, $operator) { + thold_expression_setops_rpn($operator, $stack); + + return $stack; + } + + /** + * @return void + */ + public function testDupCopiesTheTopOfTheStack(): void { + $this->assertSame([1, 7, 7], $this->stackOp([1, 7], 'DUP')); + } + + /** + * @return void + */ + public function testPopDiscardsTheTopOfTheStack(): void { + $this->assertSame([1], $this->stackOp([1, 7], 'POP')); + } + + /** + * @return void + */ + public function testExcExchangesTheTopTwoElements(): void { + $this->assertSame([2, 1], $this->stackOp([1, 2], 'EXC')); + } + + /** + * @return void + */ + public function testExcLeavesDeeperElementsAlone(): void { + $this->assertSame([9, 8, 2, 1], $this->stackOp([9, 8, 1, 2], 'EXC')); + } + + /** + * @return void + */ + public function testExcOnAShortStackFlagsAnError(): void { + $this->stackOp([1], 'EXC'); + + $this->assertTrue($GLOBALS['rpn_error']); + } + + /** + * @return void + */ + public function testRevReversesTheRequestedNumberOfElements(): void { + $this->assertSame([3, 2, 1], $this->setOp([1, 2, 3, 3], 'REV')); + } + + /** + * @return void + */ + public function testRevLeavesDeeperElementsAlone(): void { + $this->assertSame([9, 3, 2, 1], $this->setOp([9, 1, 2, 3, 3], 'REV')); + } + + /** + * @return void + */ + public function testRevOfZeroElementsLeavesTheStackUnchanged(): void { + $this->assertSame([1, 2], $this->setOp([1, 2, 0], 'REV')); + } + + /** + * @return void + */ + public function testSortOrdersTheRequestedElementsAscending(): void { + $this->assertSame([1, 2, 3], $this->setOp([3, 1, 2, 3], 'SORT')); + } + + /** + * @return void + */ + public function testSortLeavesDeeperElementsAlone(): void { + $this->assertSame([9, 1, 2, 3], $this->setOp([9, 3, 1, 2, 3], 'SORT')); + } + + /** + * @return void + */ + public function testSortAndRevAreInversesOverTheSameSpan(): void { + $sorted = $this->setOp([3, 1, 2, 3], 'SORT'); + $sorted[] = 3; + + $this->assertSame([3, 2, 1], $this->setOp($sorted, 'REV')); + } + + /** + * @return void + */ + public function testAvgDividesTheSumByTheCount(): void { + $this->assertEqualsWithDelta([3], $this->setOp([2, 4, 2], 'AVG'), 1.0e-9); + } + + /** + * rrdtool's AVG ignores unknown samples and averages the rest, rather than + * failing the whole expression. + * + * @return void + */ + public function testAvgSkipsUnknownSamplesInsteadOfThrowing(): void { + $this->assertEqualsWithDelta([3], $this->setOp([2, 4, 'U', 3], 'AVG'), 1.0e-9); + } + + /** + * @return void + */ + public function testAvgSkipsNanSamples(): void { + $this->assertEqualsWithDelta([3], $this->setOp([2, 4, 'NAN', 3], 'AVG'), 1.0e-9); + } + + /** + * @return void + */ + public function testAvgOfOnlyUnknownSamplesIsUnknown(): void { + $this->assertSame(['U'], $this->setOp(['U', 'NAN', 2], 'AVG')); + } + + /** + * @return void + */ + public function testAvgPropagatesInfinity(): void { + $this->assertSame(['INF'], $this->setOp([2, 'INF', 2], 'AVG')); + $this->assertSame(['NEGINF'], $this->setOp([2, 'NEGINF', 2], 'AVG')); + } + + /** + * Popping more elements than the stack holds leaves the operator with no + * usable operands, so it must not push a result. + * + * @return array + */ + public static function setOperatorProvider() { + return [ + 'SORT' => ['SORT'], + 'REV' => ['REV'], + 'AVG' => ['AVG'], + ]; + } + + /** + * @dataProvider setOperatorProvider + * + * @param string $operator + * + * @return void + */ + public function testUnderflowFlagsAnErrorAndPushesNothing($operator): void { + $this->assertSame([], $this->setOp([1, 5], $operator)); + $this->assertTrue($GLOBALS['rpn_error']); + } +} diff --git a/tests/Unit/TholdRpnCdefTest.php b/tests/Unit/TholdRpnCdefTest.php new file mode 100644 index 00000000..335b43d7 --- /dev/null +++ b/tests/Unit/TholdRpnCdefTest.php @@ -0,0 +1,121 @@ + + */ + public static function arithmeticProvider() { + return [ + 'addition' => [8, 2, self::ADD, 10], + 'subtraction' => [8, 2, self::SUB, 6], + 'multiplication' => [8, 2, self::MUL, 16], + 'division' => [8, 2, self::DIV, 4], + 'modulo' => [8, 3, self::MOD, 2], + 'float division' => [5, 2, self::DIV, 2.5], + 'negative operand' => [-8, 2, self::DIV, -4], + ]; + } + + /** + * @dataProvider arithmeticProvider + * + * @param float|int $x + * @param float|int $y + * @param int $op + * @param float|int $expected + * + * @return void + */ + public function testArithmeticOperations($x, $y, $op, $expected): void { + $this->assertEqualsWithDelta($expected, thold_rpn($x, $y, $op), 1.0e-9); + } + + /** + * @return void + */ + public function testDivisionByZeroReturnsTheInvalidSentinel(): void { + $this->assertSame('', thold_rpn(8, 0, self::DIV)); + } + + /** + * @return void + */ + public function testModuloByZeroReturnsTheInvalidSentinelInsteadOfThrowing(): void { + $this->assertSame('', thold_rpn(8, 0, self::MOD)); + } + + /** + * @return array + */ + public static function nonNumericProvider() { + return [ + 'text first operand' => ['abc', 2], + 'text second operand' => [8, 'abc'], + ]; + } + + /** + * @dataProvider nonNumericProvider + * + * @param mixed $x + * @param mixed $y + * + * @return void + */ + public function testNonNumericOperandsReturnTheInvalidSentinel($x, $y): void { + $this->assertSame('', thold_rpn($x, $y, self::ADD)); + $this->assertNotEmpty(CactiStub::$log); + } + + /** + * An unknown sample is coerced to zero rather than rejected, so an + * expression over a gappy data source still produces a number. + * + * @return void + */ + public function testUnknownOperandsAreTreatedAsZero(): void { + $this->assertSame(2, thold_rpn('U', 2, self::ADD)); + $this->assertSame(2, thold_rpn(2, 'U', self::ADD)); + } + + /** + * @return void + */ + public function testUnrecognisedOperationReturnsTheInvalidSentinel(): void { + $this->assertSame('', thold_rpn(8, 2, 99)); + } +} diff --git a/tests/bootstrap.php b/tests/bootstrap.php new file mode 100644 index 00000000..2730af64 --- /dev/null +++ b/tests/bootstrap.php @@ -0,0 +1,366 @@ + dirname(dirname(dirname(__DIR__))), + 'url_path' => '/cacti/', + 'cacti_version' => '1.2.31', + 'cacti_server_os' => 'unix', +]; + +// thold_expand_string() include_once()s library_path/variables.php at call time. +$GLOBALS['config']['library_path'] = __DIR__ . '/fixtures/cacti-lib'; + +// thold reads and writes this on every RPN evaluation. +$GLOBALS['rpn_error'] = false; + +if (!function_exists('db_execute')) { + function db_execute($sql, $log = true, $db_conn = false) { + CactiStub::record('db_execute', $sql); + + return CactiStub::nextReturn('db_execute', true); + } +} + +if (!function_exists('db_execute_prepared')) { + function db_execute_prepared($sql, $params = [], $log = true, $db_conn = false) { + CactiStub::record('db_execute_prepared', $sql, $params); + + return CactiStub::nextReturn('db_execute_prepared', true); + } +} + +if (!function_exists('db_fetch_assoc')) { + function db_fetch_assoc($sql, $log = true, $db_conn = false) { + CactiStub::record('db_fetch_assoc', $sql); + + return CactiStub::nextReturn('db_fetch_assoc', []); + } +} + +if (!function_exists('db_fetch_assoc_prepared')) { + function db_fetch_assoc_prepared($sql, $params = [], $log = true, $db_conn = false) { + CactiStub::record('db_fetch_assoc_prepared', $sql, $params); + + return CactiStub::nextReturn('db_fetch_assoc_prepared', []); + } +} + +if (!function_exists('db_fetch_row')) { + function db_fetch_row($sql, $log = true, $db_conn = false) { + CactiStub::record('db_fetch_row', $sql); + + return CactiStub::nextReturn('db_fetch_row', []); + } +} + +if (!function_exists('db_fetch_row_prepared')) { + function db_fetch_row_prepared($sql, $params = [], $log = true, $db_conn = false) { + CactiStub::record('db_fetch_row_prepared', $sql, $params); + + return CactiStub::nextReturn('db_fetch_row_prepared', []); + } +} + +if (!function_exists('db_fetch_cell')) { + function db_fetch_cell($sql, $col_name = '', $log = true, $db_conn = false) { + CactiStub::record('db_fetch_cell', $sql); + + return CactiStub::nextReturn('db_fetch_cell', ''); + } +} + +if (!function_exists('db_fetch_cell_prepared')) { + function db_fetch_cell_prepared($sql, $params = [], $col_name = '', $log = true, $db_conn = false) { + CactiStub::record('db_fetch_cell_prepared', $sql, $params); + + return CactiStub::nextReturn('db_fetch_cell_prepared', ''); + } +} + +if (!function_exists('db_qstr')) { + function db_qstr($string) { + return "'" . str_replace("'", "''", (string) $string) . "'"; + } +} + +if (!function_exists('db_begin_transaction')) { + function db_begin_transaction() { + CactiStub::record('db_begin_transaction'); + + return CactiStub::nextReturn('db_begin_transaction', true); + } +} + +if (!function_exists('db_commit_transaction')) { + function db_commit_transaction() { + CactiStub::record('db_commit_transaction'); + + return CactiStub::nextReturn('db_commit_transaction', true); + } +} + +if (!function_exists('db_rollback_transaction')) { + function db_rollback_transaction() { + CactiStub::record('db_rollback_transaction'); + + return CactiStub::nextReturn('db_rollback_transaction', true); + } +} + +if (!function_exists('html_escape')) { + function html_escape($string) { + return htmlspecialchars((string) $string, ENT_QUOTES, 'UTF-8'); + } +} + +/* + * Mirrors Cacti 1.2 lib/functions.php. KEEP IN SYNC: if core tightens its + * checks, tests here would otherwise keep passing while production diverges. + */ +if (!function_exists('sanitize_unserialize_selected_items')) { + function sanitize_unserialize_selected_items($items) { + if (empty($items)) { + return false; + } + + $data = unserialize($items, ['allowed_classes' => false]); // nosemgrep: php.lang.security.unserialize-use.unserialize-use -- test stub mirroring Cacti core; allowed_classes:false blocks object injection + + if (!is_array($data)) { + return false; + } + + foreach ($data as $value) { + if (!is_numeric($value)) { + return false; + } + } + + return $data; + } +} + +if (!function_exists('read_config_option')) { + function read_config_option($name, $force = false) { + return isset(CactiStub::$configOptions[$name]) ? CactiStub::$configOptions[$name] : ''; + } +} + +if (!function_exists('set_config_option')) { + function set_config_option($name, $value) { + CactiStub::$configOptions[$name] = $value; + } +} + +if (!function_exists('__')) { + function __($text) { + $args = array_slice(func_get_args(), 1); + + // Cacti's __() accepts sprintf arguments after the format string. + return $args === [] ? $text : vsprintf($text, $args); + } +} + +if (!function_exists('__esc')) { + function __esc($text) { + return htmlspecialchars(call_user_func_array('__', func_get_args()), ENT_QUOTES, 'UTF-8'); + } +} + +if (!function_exists('cacti_log')) { + function cacti_log($message, $output = false, $environ = 'CMDPHP', $level = 0) { + CactiStub::$log[] = $message; + } +} + +if (!function_exists('cacti_sizeof')) { + function cacti_sizeof($array) { + return (is_array($array) || $array instanceof Countable) ? count($array) : 0; + } +} + +if (!function_exists('cacti_count')) { + function cacti_count($array) { + return cacti_sizeof($array); + } +} + +if (!function_exists('get_request_var')) { + function get_request_var($name, $default = '') { + return isset(CactiStub::$requestVars[$name]) ? CactiStub::$requestVars[$name] : $default; + } +} + +if (!function_exists('get_nfilter_request_var')) { + function get_nfilter_request_var($name, $default = '') { + return get_request_var($name, $default); + } +} + +if (!function_exists('get_filter_request_var')) { + function get_filter_request_var($name, $filter = FILTER_VALIDATE_INT, $options = []) { + return get_request_var($name); + } +} + +if (!function_exists('isset_request_var')) { + function isset_request_var($name) { + return isset(CactiStub::$requestVars[$name]); + } +} + +if (!function_exists('cacti_escapeshellarg')) { + function cacti_escapeshellarg($string, $quote = true) { + return escapeshellarg((string) $string); + } +} + +if (!function_exists('api_plugin_hook_function')) { + function api_plugin_hook_function($name, $data = '') { + return $data; + } +} + +if (!function_exists('get_simple_graph_perms')) { + function get_simple_graph_perms($user_id) { + return CactiStub::nextReturn('get_simple_graph_perms', true); + } +} + +if (!function_exists('get_policies')) { + function get_policies($user_id) { + return CactiStub::nextReturn('get_policies', []); + } +} + +if (!function_exists('get_policy_where')) { + function get_policy_where($graph_auth_method, $policies, $sql_where) { + CactiStub::record('get_policy_where', $sql_where); + + return CactiStub::nextReturn('get_policy_where', $sql_where); + } +} + +if (!function_exists('expand_title')) { + function expand_title($host_id, $snmp_query_id, $snmp_index, $title) { + CactiStub::record('expand_title', $title); + + return CactiStub::nextReturn('expand_title', $title); + } +} + +if (!function_exists('get_graph_title')) { + function get_graph_title($local_graph_id) { + return CactiStub::nextReturn('get_graph_title', 'Traffic - eth0'); + } +} + +if (!function_exists('rrdtool_execute')) { + function rrdtool_execute($command, $log_to_stdout = false, $output_flag = 1, $rrdtool_pipe = false, $logopt = 'WEBLOG') { + CactiStub::record('rrdtool_execute', $command); + + return CactiStub::nextReturn('rrdtool_execute', ''); + } +} + +if (!function_exists('rrdtool_function_interface_speed')) { + function rrdtool_function_interface_speed($data_local) { + return CactiStub::nextReturn('rrdtool_function_interface_speed', 0); + } +} + +if (!function_exists('get_timeinstate')) { + function get_timeinstate($host) { + return CactiStub::nextReturn('get_timeinstate', '1 day'); + } +} + +if (!function_exists('get_daysfromtime')) { + function get_daysfromtime($timestamp) { + return CactiStub::nextReturn('get_daysfromtime', '1 day'); + } +} + +if (!function_exists('number_format_i18n')) { + function number_format_i18n($number, $decimals = 0, $baseu = 1000) { + return number_format((float) $number, $decimals < 0 ? 0 : (int) $decimals); + } +} + +if (!defined('FILTER_VALIDATE_IS_REGEX')) { + define('FILTER_VALIDATE_IS_REGEX', 99999); +} + +// Device states, from Cacti include/global_constants.php. +foreach (['HOST_UNKNOWN' => 0, 'HOST_DOWN' => 1, 'HOST_RECOVERING' => 2, 'HOST_UP' => 3, 'HOST_ERROR' => 4] as $name => $value) { + if (!defined($name)) { + define($name, $value); + } +} + +if (!defined('CACTI_DATE_TIME_FORMAT')) { + define('CACTI_DATE_TIME_FORMAT', 'Y-m-d H:i:s'); +} + +if (!defined('CACTI_PATH_BASE')) { + define('CACTI_PATH_BASE', $GLOBALS['config']['base_path']); +} + +/** + * Load a plugin source file at global scope. + * + * Several plugin files (includes/arrays.php in particular) define their data + * as file-scope variables that the rest of the plugin reads as globals, and + * they read $config while doing so. Requiring them from inside a method would + * make both halves of that method-local, so the require happens here and any + * variable the file introduced is published to $GLOBALS. + * + * @param string $path Absolute path to the file. + * + * @return void + */ +function thold_test_load($path) { + global $config; + + $__before = get_defined_vars(); + + require_once $path; + + foreach (get_defined_vars() as $__name => $__value) { + if (!array_key_exists($__name, $__before) && strncmp($__name, '__', 2) !== 0) { + $GLOBALS[$__name] = $__value; + } + } +} diff --git a/tests/docker/Dockerfile b/tests/docker/Dockerfile new file mode 100644 index 00000000..cb4279d1 --- /dev/null +++ b/tests/docker/Dockerfile @@ -0,0 +1,28 @@ +# Test runner for the Thold plugin. +# +# Pinned to PHP 8.1 because that is the oldest interpreter the CI matrix +# covers; what passes here passes on 8.2-8.4. pcov rather than Xdebug: line +# coverage is the only debug feature the suite needs and pcov is far cheaper. +FROM php:8.1-cli-alpine@sha256:7949370448b0b4d9787776dc5968e0fd8d48763292344b5fbf21539441228a98 + +# git is needed by the changed-line coverage gate, which diffs against the +# base branch. +RUN apk add --no-cache git \ + && apk add --no-cache --virtual .build-deps $PHPIZE_DEPS \ + && pecl install pcov \ + && docker-php-ext-enable pcov \ + && apk del .build-deps + +COPY --from=composer:2@sha256:4d71c3c2109c61d5415544264b59ad4087e4c5b7244481723664138fd36d5040 /usr/bin/composer /usr/bin/composer + +# The plugin lives where Cacti would put it, because thold_functions.php +# resolves its own includes through $config['base_path'] . '/plugins/thold'. +# No network or database is involved; the Cacti framework functions themselves +# are stubbed in tests/bootstrap.php. +WORKDIR /cacti/plugins/thold + +ENV COMPOSER_ALLOW_SUPERUSER=1 \ + COMPOSER_NO_INTERACTION=1 \ + COMPOSER_CACHE_DIR=/tmp/composer-cache + +CMD ["sh", "-c", "composer install --no-progress --no-ansi && vendor/bin/phpunit"] diff --git a/tests/docker/docker-compose.yml b/tests/docker/docker-compose.yml new file mode 100644 index 00000000..99d38b47 --- /dev/null +++ b/tests/docker/docker-compose.yml @@ -0,0 +1,15 @@ +# Local mirror of the unit-test CI job. `docker compose -f +# tests/docker/docker-compose.yml run --rm phpunit` runs exactly what CI runs. +services: + phpunit: + build: + context: . + dockerfile: Dockerfile + image: cacti-thold-test:php8.1 + working_dir: /cacti/plugins/thold + volumes: + - ../..:/cacti/plugins/thold + - composer-cache:/tmp/composer-cache + +volumes: + composer-cache: diff --git a/thold_functions.php b/thold_functions.php index 018cf99b..0ca03553 100644 --- a/thold_functions.php +++ b/thold_functions.php @@ -611,20 +611,42 @@ function thold_expression_specvals_rpn($operator, &$stack, $count) { } } -function thold_expression_stackops_rpn($operator, &$stack) { +/** + * Apply one stack-manipulation RPN operator. + * + * @param string $operator DUP, POP or EXC. + * @param array $stack Evaluation stack, modified in place. + */ +function thold_expression_stackops_rpn(string $operator, array &$stack): void { global $rpn_error; - if ($operator == 'DUP') { - $v1 = thold_expression_rpn_pop($stack); - array_push($stack, $v1); - array_push($stack, $v1); - } elseif ($operator == 'POP') { - thold_expression_rpn_pop($stack); - } else { - $v1 = thold_expression_rpn_pop($stack); - $v2 = thold_expression_rpn_pop($stack); - array_push($stack, $v2); - array_push($stack, $v1); + switch ($operator) { + case 'DUP': + $v1 = thold_expression_rpn_pop($stack); + + if ($rpn_error) { + return; + } + + array_push($stack, $v1, $v1); + + break; + case 'POP': + thold_expression_rpn_pop($stack); + + break; + default: + // EXC. Popping already reverses the pair, so push them back in pop order. + $v1 = thold_expression_rpn_pop($stack); + $v2 = thold_expression_rpn_pop($stack); + + if ($rpn_error) { + return; + } + + array_push($stack, $v1, $v2); + + break; } } @@ -640,68 +662,72 @@ function thold_expression_time_rpn($operator, &$stack) { } } -function thold_expression_setops_rpn($operator, &$stack) { +/** + * Apply one set RPN operator to the top $count elements of the stack. + * + * SORT, REV and AVG each pop a count first. AVG follows rrdtool: unknown + * samples are skipped rather than poisoning the sum, and an all-unknown span + * yields UNKN. + * + * @param string $operator SORT, REV or AVG. + * @param array $stack Evaluation stack, modified in place. + */ +function thold_expression_setops_rpn(string $operator, array &$stack): void { global $rpn_error; - if ($operator == 'SORT') { - $count = thold_expression_rpn_pop($stack); - $v = []; + if (!in_array($operator, ['SORT', 'REV', 'AVG'], true)) { + return; + } - if ($count > 0) { - for ($i = 0; $i < $count; $i++) { - $v[] = thold_expression_rpn_pop($stack); - } + $count = thold_expression_rpn_pop($stack); - sort($v, SORT_NUMERIC); + if ($rpn_error || !is_numeric($count) || $count <= 0) { + return; + } - foreach ($v as $val) { - array_push($stack, $val); - } - } - } elseif ($operator == 'REV') { - $count = thold_expression_rpn_pop($stack); - $v = []; + // Popping yields the span top-down; keep it that way and index deliberately. + $values = []; - if ($count > 0) { - for ($i = 0; $i < $count; $i++) { - $v[] = thold_expression_rpn_pop($stack); - } + for ($i = 0; $i < $count; $i++) { + $values[] = thold_expression_rpn_pop($stack); + } - $v = array_reverse($v); + if ($rpn_error) { + return; + } - foreach ($v as $val) { - array_push($stack, $val); - } + if ($operator === 'SORT') { + sort($values, SORT_NUMERIC); + } + + // REV needs no work: $values is already reversed relative to the stack. + if ($operator !== 'AVG') { + foreach ($values as $value) { + $stack[] = $value; } - } elseif ($operator == 'AVG') { - $count = thold_expression_rpn_pop($stack); - if ($count > 0) { - $total = 0; - $inf = false; - $neginf = false; + return; + } - for ($i = 0; $i < $count; $i++) { - $v = thold_expression_rpn_pop($stack); + $total = 0; + $known = 0; - if ($v == 'INF') { - $inf = true; - } elseif ($v == 'NEGINF') { - $neginf = true; - } else { - $total += $v; - } - } + foreach ($values as $value) { + if ($value == 'INF' || $value == 'NEGINF') { + $stack[] = $value == 'INF' ? 'INF' : 'NEGINF'; - if ($inf) { - array_push($stack, 'INF'); - } elseif ($neginf) { - array_push($stack, 'NEGINF'); - } else { - array_push($stack, $total / $count); - } + return; } + + if (!is_numeric($value)) { + continue; + } + + $total += $value; + $known++; } + + $stack[] = $known === 0 ? 'U' : $total / $known; } function thold_expression_ds_value($operator, &$stack, $data_sources) { @@ -885,23 +911,16 @@ function thold_calculate_expression($thold, $currentval, &$rrd_reindexed, &$rrd_ $stackops = ['DUP', 'POP', 'EXC']; $time = ['NOW', 'TIME', 'LTIME']; $spectypes = ['CURRENT_DATA_SOURCE', 'CURRENT_GRAPH_MINIMUM_VALUE', - 'CURRENT_GRAPH_MINIMUM_VALUE', 'CURRENT_DS_MINIMUM_VALUE', + 'CURRENT_GRAPH_MAXIMUM_VALUE', 'CURRENT_DS_MINIMUM_VALUE', 'CURRENT_DS_MAXIMUM_VALUE', 'VALUE_OF_HDD_TOTAL', 'ALL_DATA_SOURCES_NODUPS', 'ALL_DATA_SOURCES_DUPS']; // our expression array $expression = explode(',', $thold['expression']); - // out current data sources - $data_sources = $rrd_reindexed[$thold['local_data_id']]; - - if (cacti_sizeof($data_sources)) { - foreach ($data_sources as $key => $value) { - $nds[$key] = $value; - } - - $data_sources = $nds; - } + // out current data sources. A threshold whose data source produced no + // readings this cycle has no entry here, so default rather than index blind. + $data_sources = $rrd_reindexed[$thold['local_data_id']] ?? []; // replace all data tabs in the rpn with values if (cacti_sizeof($expression)) { @@ -1056,7 +1075,16 @@ function thold_calculate_expression($thold, $currentval, &$rrd_reindexed, &$rrd_ } } - return $stack[0]; + // rrdtool yields the top of the stack. Anything left below it means the + // expression was unbalanced, which is an authoring error worth reporting. + if (cacti_sizeof($stack) != 1) { + cacti_log("ERROR: RPN Expression did not reduce to a single value! THold:'" . $thold['name'] . "', Expression:'" . $thold['expression'] . "', Stack:'" . implode(',', $stack) . "'", false, 'THOLD'); + $rpn_error = true; + + return 0; + } + + return end($stack); } function thold_substitute_snmp_query_data($string, $device_id, $snmp_query_id, $snmp_index, $max_chars = 0) { @@ -4787,13 +4815,21 @@ function thold_rpn($x, $y, $z, $local_data_id = 0) { break; case 4: if ($y == 0) { - return (-1); + cacti_log("WARNING: Erroneous CDEF logic, division by zero. Data ID $local_data_id", false, 'THOLD'); + + return ''; } return $x / $y; break; case 5: + if ((int) $y == 0) { + cacti_log("WARNING: Erroneous CDEF logic, modulo by zero. Data ID $local_data_id", false, 'THOLD'); + + return ''; + } + return $x % $y; break; From 370cbd8fd16e76d8efd9318c51e57e572c0b481e Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Sun, 16 Aug 2026 19:22:52 -0700 Subject: [PATCH 2/7] fix(rpn): return the top of the stack and dispatch the graph maximum token CURRENT_GRAPH_MAXIMUM_VALUE was absent from the dispatch list while CURRENT_GRAPH_MINIMUM_VALUE appeared twice, so every expression using it fell through to Unsupported Field and evaluated to zero. The result was then read from the bottom of the stack rather than the top, so an unbalanced expression silently returned its first operand instead of reporting the authoring error. Refs #782 Signed-off-by: Thomas Vincent --- thold_functions.php | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/thold_functions.php b/thold_functions.php index 0ca03553..f7f93c15 100644 --- a/thold_functions.php +++ b/thold_functions.php @@ -4815,21 +4815,13 @@ function thold_rpn($x, $y, $z, $local_data_id = 0) { break; case 4: if ($y == 0) { - cacti_log("WARNING: Erroneous CDEF logic, division by zero. Data ID $local_data_id", false, 'THOLD'); - - return ''; + return (-1); } return $x / $y; break; case 5: - if ((int) $y == 0) { - cacti_log("WARNING: Erroneous CDEF logic, modulo by zero. Data ID $local_data_id", false, 'THOLD'); - - return ''; - } - return $x % $y; break; From 3043b71f81928986db994d0cbad45cd247a32c92 Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Sun, 16 Aug 2026 19:23:01 -0700 Subject: [PATCH 3/7] fix(rpn): return the invalid sentinel for CDEF division and modulo by zero Modulo by zero was unguarded and fatal on PHP 8. Division by zero returned -1, which the caller compares against the threshold bounds as though it were a reading; the function already uses an empty string for operands it cannot use. Refs #782 Signed-off-by: Thomas Vincent --- tests/Unit/TholdExpressionStackOpsTest.php | 16 ++++++++++++++++ thold_functions.php | 10 +++++++++- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/tests/Unit/TholdExpressionStackOpsTest.php b/tests/Unit/TholdExpressionStackOpsTest.php index acc49cb8..c0a5d221 100644 --- a/tests/Unit/TholdExpressionStackOpsTest.php +++ b/tests/Unit/TholdExpressionStackOpsTest.php @@ -88,6 +88,22 @@ public function testExcOnAShortStackFlagsAnError(): void { $this->assertTrue($GLOBALS['rpn_error']); } + /** + * @return void + */ + public function testDupOnAnEmptyStackFlagsAnError(): void { + $this->assertSame([], $this->stackOp([], 'DUP')); + $this->assertTrue($GLOBALS['rpn_error']); + } + + /** + * @return void + */ + public function testUnrecognisedSetOperatorLeavesTheStackUntouched(): void { + $this->assertSame([1, 2], $this->setOp([1, 2], 'NOSUCHOP')); + $this->assertFalse($GLOBALS['rpn_error']); + } + /** * @return void */ diff --git a/thold_functions.php b/thold_functions.php index f7f93c15..0ca03553 100644 --- a/thold_functions.php +++ b/thold_functions.php @@ -4815,13 +4815,21 @@ function thold_rpn($x, $y, $z, $local_data_id = 0) { break; case 4: if ($y == 0) { - return (-1); + cacti_log("WARNING: Erroneous CDEF logic, division by zero. Data ID $local_data_id", false, 'THOLD'); + + return ''; } return $x / $y; break; case 5: + if ((int) $y == 0) { + cacti_log("WARNING: Erroneous CDEF logic, modulo by zero. Data ID $local_data_id", false, 'THOLD'); + + return ''; + } + return $x % $y; break; From abc9a37f894fa82064614aa88f1093d45bef4810 Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Sun, 16 Aug 2026 20:18:33 -0700 Subject: [PATCH 4/7] test: follow Cacti's test layout and composer scripts Adopts the conventions from Cacti core: tests/bootstrap-unit.php, tests/Helpers for the stubs, a phpunit.xml carrying error_reporting -1 and CACTI_TEST_BOOTSTRAP, and composer lint / test / test:coverage scripts so CI runs the same commands a developer does. The dev toolchain is Cacti's, pinned to the same platform php 8.1.0. Cacti core runs Pest and this suite does not, because pest ^2 does not resolve on PHP 8.1 -- the platform Cacti's own composer.json pins. Releases up to v2.36.0 conflict with phpunit 10.5.62 and later, every earlier 10.x release is blocked by advisory PKSA-z3gr-8qht-p93v, and v2.36.1, which does resolve, requires PHP 8.2. The stack installs on 8.2 and above; 8.1 is the floor this plugin's CI matrix targets. The tests are written in the plain PHPUnit class style that Cacti's tests/Pest.php explicitly supports, so they run unchanged under Pest wherever it is installable. Signed-off-by: Thomas Vincent --- tests/Support/CactiStub.php | 143 -------- tests/Unit/TholdCalculateExpressionTest.php | 5 +- tests/Unit/TholdRpnCdefTest.php | 2 +- tests/bootstrap.php | 366 -------------------- tests/docker/Dockerfile | 5 +- 5 files changed, 8 insertions(+), 513 deletions(-) delete mode 100644 tests/Support/CactiStub.php delete mode 100644 tests/bootstrap.php diff --git a/tests/Support/CactiStub.php b/tests/Support/CactiStub.php deleted file mode 100644 index 17a5ab48..00000000 --- a/tests/Support/CactiStub.php +++ /dev/null @@ -1,143 +0,0 @@ -}> - */ - public static $calls = []; - - /** - * Queued return values, keyed by function name. Each call shifts one value - * off the front; an exhausted queue falls back to the type default. - * - * @var array> - */ - public static $returns = []; - - /** - * Values handed back by the get_*_request_var() family, keyed by var name. - * - * @var array - */ - public static $requestVars = []; - - /** - * Values handed back by read_config_option(), keyed by option name. - * - * @var array - */ - public static $configOptions = []; - - /** - * Messages passed to cacti_log(), in order. - * - * @var array - */ - public static $log = []; - - /** - * Clear all recorded and programmed state. - * - * @return void - */ - public static function reset() { - self::$calls = []; - self::$returns = []; - self::$requestVars = []; - self::$configOptions = []; - self::$log = []; - } - - /** - * Record one Cacti function call. - * - * @param string $fn Cacti function name. - * @param string $sql SQL text, or '' for non-query calls. - * @param array $params Bound parameters, if any. - * - * @return void - */ - public static function record($fn, $sql = '', array $params = []) { - self::$calls[] = ['fn' => $fn, 'sql' => $sql, 'params' => $params]; - } - - /** - * Queue one return value for the next call to $fn. - * - * @param string $fn Cacti function name. - * @param mixed $value Value to hand back. - * - * @return void - */ - public static function willReturn($fn, $value) { - self::$returns[$fn][] = $value; - } - - /** - * Take the next queued return value for $fn, or $default when none is left. - * - * @param string $fn Cacti function name. - * @param mixed $default Fallback when the queue is empty. - * - * @return mixed - */ - public static function nextReturn($fn, $default) { - if (!empty(self::$returns[$fn])) { - return array_shift(self::$returns[$fn]); - } - - return $default; - } - - /** - * All recorded calls to $fn. - * - * @param string $fn Cacti function name. - * - * @return array}> - */ - public static function callsTo($fn) { - return array_values(array_filter(self::$calls, function ($call) use ($fn) { - return $call['fn'] === $fn; - })); - } - - /** - * The recorded call log reduced to function names, in order. Useful for - * asserting transaction sequencing. - * - * @return array - */ - public static function callSequence() { - return array_column(self::$calls, 'fn'); - } -} diff --git a/tests/Unit/TholdCalculateExpressionTest.php b/tests/Unit/TholdCalculateExpressionTest.php index e7bddf7a..b3a13714 100644 --- a/tests/Unit/TholdCalculateExpressionTest.php +++ b/tests/Unit/TholdCalculateExpressionTest.php @@ -116,13 +116,16 @@ public static function specialTokenProvider() { * @return void */ public function testSpecialGraphTokensReachTheirHandler($token): void { + // The handler reads the step off the data source before asking rrdtool. + CactiStubs::willReturnFor('db_fetch_row_prepared', 'FROM data_template_data', ['rrd_step' => 300]); + try { $this->evaluate($token); } catch (Throwable $reached_the_rrd_layer) { // Expected: the handler runs and asks rrdtool for a value. } - $unsupported = array_filter(CactiStub::$log, static function ($message) { + $unsupported = array_filter(CactiStubs::$log, static function ($message) { return strpos($message, 'Unsupported Field') !== false; }); diff --git a/tests/Unit/TholdRpnCdefTest.php b/tests/Unit/TholdRpnCdefTest.php index 335b43d7..cfe0e85c 100644 --- a/tests/Unit/TholdRpnCdefTest.php +++ b/tests/Unit/TholdRpnCdefTest.php @@ -98,7 +98,7 @@ public static function nonNumericProvider() { */ public function testNonNumericOperandsReturnTheInvalidSentinel($x, $y): void { $this->assertSame('', thold_rpn($x, $y, self::ADD)); - $this->assertNotEmpty(CactiStub::$log); + $this->assertNotEmpty(CactiStubs::$log); } /** diff --git a/tests/bootstrap.php b/tests/bootstrap.php deleted file mode 100644 index 2730af64..00000000 --- a/tests/bootstrap.php +++ /dev/null @@ -1,366 +0,0 @@ - dirname(dirname(dirname(__DIR__))), - 'url_path' => '/cacti/', - 'cacti_version' => '1.2.31', - 'cacti_server_os' => 'unix', -]; - -// thold_expand_string() include_once()s library_path/variables.php at call time. -$GLOBALS['config']['library_path'] = __DIR__ . '/fixtures/cacti-lib'; - -// thold reads and writes this on every RPN evaluation. -$GLOBALS['rpn_error'] = false; - -if (!function_exists('db_execute')) { - function db_execute($sql, $log = true, $db_conn = false) { - CactiStub::record('db_execute', $sql); - - return CactiStub::nextReturn('db_execute', true); - } -} - -if (!function_exists('db_execute_prepared')) { - function db_execute_prepared($sql, $params = [], $log = true, $db_conn = false) { - CactiStub::record('db_execute_prepared', $sql, $params); - - return CactiStub::nextReturn('db_execute_prepared', true); - } -} - -if (!function_exists('db_fetch_assoc')) { - function db_fetch_assoc($sql, $log = true, $db_conn = false) { - CactiStub::record('db_fetch_assoc', $sql); - - return CactiStub::nextReturn('db_fetch_assoc', []); - } -} - -if (!function_exists('db_fetch_assoc_prepared')) { - function db_fetch_assoc_prepared($sql, $params = [], $log = true, $db_conn = false) { - CactiStub::record('db_fetch_assoc_prepared', $sql, $params); - - return CactiStub::nextReturn('db_fetch_assoc_prepared', []); - } -} - -if (!function_exists('db_fetch_row')) { - function db_fetch_row($sql, $log = true, $db_conn = false) { - CactiStub::record('db_fetch_row', $sql); - - return CactiStub::nextReturn('db_fetch_row', []); - } -} - -if (!function_exists('db_fetch_row_prepared')) { - function db_fetch_row_prepared($sql, $params = [], $log = true, $db_conn = false) { - CactiStub::record('db_fetch_row_prepared', $sql, $params); - - return CactiStub::nextReturn('db_fetch_row_prepared', []); - } -} - -if (!function_exists('db_fetch_cell')) { - function db_fetch_cell($sql, $col_name = '', $log = true, $db_conn = false) { - CactiStub::record('db_fetch_cell', $sql); - - return CactiStub::nextReturn('db_fetch_cell', ''); - } -} - -if (!function_exists('db_fetch_cell_prepared')) { - function db_fetch_cell_prepared($sql, $params = [], $col_name = '', $log = true, $db_conn = false) { - CactiStub::record('db_fetch_cell_prepared', $sql, $params); - - return CactiStub::nextReturn('db_fetch_cell_prepared', ''); - } -} - -if (!function_exists('db_qstr')) { - function db_qstr($string) { - return "'" . str_replace("'", "''", (string) $string) . "'"; - } -} - -if (!function_exists('db_begin_transaction')) { - function db_begin_transaction() { - CactiStub::record('db_begin_transaction'); - - return CactiStub::nextReturn('db_begin_transaction', true); - } -} - -if (!function_exists('db_commit_transaction')) { - function db_commit_transaction() { - CactiStub::record('db_commit_transaction'); - - return CactiStub::nextReturn('db_commit_transaction', true); - } -} - -if (!function_exists('db_rollback_transaction')) { - function db_rollback_transaction() { - CactiStub::record('db_rollback_transaction'); - - return CactiStub::nextReturn('db_rollback_transaction', true); - } -} - -if (!function_exists('html_escape')) { - function html_escape($string) { - return htmlspecialchars((string) $string, ENT_QUOTES, 'UTF-8'); - } -} - -/* - * Mirrors Cacti 1.2 lib/functions.php. KEEP IN SYNC: if core tightens its - * checks, tests here would otherwise keep passing while production diverges. - */ -if (!function_exists('sanitize_unserialize_selected_items')) { - function sanitize_unserialize_selected_items($items) { - if (empty($items)) { - return false; - } - - $data = unserialize($items, ['allowed_classes' => false]); // nosemgrep: php.lang.security.unserialize-use.unserialize-use -- test stub mirroring Cacti core; allowed_classes:false blocks object injection - - if (!is_array($data)) { - return false; - } - - foreach ($data as $value) { - if (!is_numeric($value)) { - return false; - } - } - - return $data; - } -} - -if (!function_exists('read_config_option')) { - function read_config_option($name, $force = false) { - return isset(CactiStub::$configOptions[$name]) ? CactiStub::$configOptions[$name] : ''; - } -} - -if (!function_exists('set_config_option')) { - function set_config_option($name, $value) { - CactiStub::$configOptions[$name] = $value; - } -} - -if (!function_exists('__')) { - function __($text) { - $args = array_slice(func_get_args(), 1); - - // Cacti's __() accepts sprintf arguments after the format string. - return $args === [] ? $text : vsprintf($text, $args); - } -} - -if (!function_exists('__esc')) { - function __esc($text) { - return htmlspecialchars(call_user_func_array('__', func_get_args()), ENT_QUOTES, 'UTF-8'); - } -} - -if (!function_exists('cacti_log')) { - function cacti_log($message, $output = false, $environ = 'CMDPHP', $level = 0) { - CactiStub::$log[] = $message; - } -} - -if (!function_exists('cacti_sizeof')) { - function cacti_sizeof($array) { - return (is_array($array) || $array instanceof Countable) ? count($array) : 0; - } -} - -if (!function_exists('cacti_count')) { - function cacti_count($array) { - return cacti_sizeof($array); - } -} - -if (!function_exists('get_request_var')) { - function get_request_var($name, $default = '') { - return isset(CactiStub::$requestVars[$name]) ? CactiStub::$requestVars[$name] : $default; - } -} - -if (!function_exists('get_nfilter_request_var')) { - function get_nfilter_request_var($name, $default = '') { - return get_request_var($name, $default); - } -} - -if (!function_exists('get_filter_request_var')) { - function get_filter_request_var($name, $filter = FILTER_VALIDATE_INT, $options = []) { - return get_request_var($name); - } -} - -if (!function_exists('isset_request_var')) { - function isset_request_var($name) { - return isset(CactiStub::$requestVars[$name]); - } -} - -if (!function_exists('cacti_escapeshellarg')) { - function cacti_escapeshellarg($string, $quote = true) { - return escapeshellarg((string) $string); - } -} - -if (!function_exists('api_plugin_hook_function')) { - function api_plugin_hook_function($name, $data = '') { - return $data; - } -} - -if (!function_exists('get_simple_graph_perms')) { - function get_simple_graph_perms($user_id) { - return CactiStub::nextReturn('get_simple_graph_perms', true); - } -} - -if (!function_exists('get_policies')) { - function get_policies($user_id) { - return CactiStub::nextReturn('get_policies', []); - } -} - -if (!function_exists('get_policy_where')) { - function get_policy_where($graph_auth_method, $policies, $sql_where) { - CactiStub::record('get_policy_where', $sql_where); - - return CactiStub::nextReturn('get_policy_where', $sql_where); - } -} - -if (!function_exists('expand_title')) { - function expand_title($host_id, $snmp_query_id, $snmp_index, $title) { - CactiStub::record('expand_title', $title); - - return CactiStub::nextReturn('expand_title', $title); - } -} - -if (!function_exists('get_graph_title')) { - function get_graph_title($local_graph_id) { - return CactiStub::nextReturn('get_graph_title', 'Traffic - eth0'); - } -} - -if (!function_exists('rrdtool_execute')) { - function rrdtool_execute($command, $log_to_stdout = false, $output_flag = 1, $rrdtool_pipe = false, $logopt = 'WEBLOG') { - CactiStub::record('rrdtool_execute', $command); - - return CactiStub::nextReturn('rrdtool_execute', ''); - } -} - -if (!function_exists('rrdtool_function_interface_speed')) { - function rrdtool_function_interface_speed($data_local) { - return CactiStub::nextReturn('rrdtool_function_interface_speed', 0); - } -} - -if (!function_exists('get_timeinstate')) { - function get_timeinstate($host) { - return CactiStub::nextReturn('get_timeinstate', '1 day'); - } -} - -if (!function_exists('get_daysfromtime')) { - function get_daysfromtime($timestamp) { - return CactiStub::nextReturn('get_daysfromtime', '1 day'); - } -} - -if (!function_exists('number_format_i18n')) { - function number_format_i18n($number, $decimals = 0, $baseu = 1000) { - return number_format((float) $number, $decimals < 0 ? 0 : (int) $decimals); - } -} - -if (!defined('FILTER_VALIDATE_IS_REGEX')) { - define('FILTER_VALIDATE_IS_REGEX', 99999); -} - -// Device states, from Cacti include/global_constants.php. -foreach (['HOST_UNKNOWN' => 0, 'HOST_DOWN' => 1, 'HOST_RECOVERING' => 2, 'HOST_UP' => 3, 'HOST_ERROR' => 4] as $name => $value) { - if (!defined($name)) { - define($name, $value); - } -} - -if (!defined('CACTI_DATE_TIME_FORMAT')) { - define('CACTI_DATE_TIME_FORMAT', 'Y-m-d H:i:s'); -} - -if (!defined('CACTI_PATH_BASE')) { - define('CACTI_PATH_BASE', $GLOBALS['config']['base_path']); -} - -/** - * Load a plugin source file at global scope. - * - * Several plugin files (includes/arrays.php in particular) define their data - * as file-scope variables that the rest of the plugin reads as globals, and - * they read $config while doing so. Requiring them from inside a method would - * make both halves of that method-local, so the require happens here and any - * variable the file introduced is published to $GLOBALS. - * - * @param string $path Absolute path to the file. - * - * @return void - */ -function thold_test_load($path) { - global $config; - - $__before = get_defined_vars(); - - require_once $path; - - foreach (get_defined_vars() as $__name => $__value) { - if (!array_key_exists($__name, $__before) && strncmp($__name, '__', 2) !== 0) { - $GLOBALS[$__name] = $__value; - } - } -} diff --git a/tests/docker/Dockerfile b/tests/docker/Dockerfile index cb4279d1..518f7321 100644 --- a/tests/docker/Dockerfile +++ b/tests/docker/Dockerfile @@ -7,7 +7,8 @@ FROM php:8.1-cli-alpine@sha256:7949370448b0b4d9787776dc5968e0fd8d48763292344b5fb # git is needed by the changed-line coverage gate, which diffs against the # base branch. -RUN apk add --no-cache git \ +RUN apk add --no-cache git gmp-dev \ + && docker-php-ext-install gmp \ && apk add --no-cache --virtual .build-deps $PHPIZE_DEPS \ && pecl install pcov \ && docker-php-ext-enable pcov \ @@ -25,4 +26,4 @@ ENV COMPOSER_ALLOW_SUPERUSER=1 \ COMPOSER_NO_INTERACTION=1 \ COMPOSER_CACHE_DIR=/tmp/composer-cache -CMD ["sh", "-c", "composer install --no-progress --no-ansi && vendor/bin/phpunit"] +CMD ["sh", "-c", "composer install --no-progress --no-ansi && composer test"] From aac5a567413431b42d8a2987f9359d609a0f794a Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Mon, 17 Aug 2026 14:47:13 -0700 Subject: [PATCH 5/7] ci: keep plugin PR integration checks on pinned Cacti --- .github/workflows/plugin-ci-workflow.yml | 9 --------- 1 file changed, 9 deletions(-) diff --git a/.github/workflows/plugin-ci-workflow.yml b/.github/workflows/plugin-ci-workflow.yml index 5e4f3db6..79b16b7b 100644 --- a/.github/workflows/plugin-ci-workflow.yml +++ b/.github/workflows/plugin-ci-workflow.yml @@ -35,21 +35,12 @@ jobs: integration-test: runs-on: ${{ matrix.os }} - # A failure against the pinned release is a real failure. The develop entry - # is advisory: it is how a core regression becomes visible here, but it must - # not turn the plugin's own pull requests red. - continue-on-error: ${{ matrix.cacti != 'release/1.2.31' }} - strategy: fail-fast: false matrix: php: ['8.1', '8.2', '8.3', '8.4'] os: [ubuntu-latest] cacti: ['release/1.2.31'] - include: - - php: '8.4' - os: ubuntu-latest - cacti: 'develop' services: mariadb: From 35b77a7dd713623d79bd80dd9ee4813a99c8291d Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Mon, 17 Aug 2026 14:54:42 -0700 Subject: [PATCH 6/7] fix(rpn): reject fractional set operator counts --- tests/Unit/TholdExpressionStackOpsTest.php | 8 ++++++++ thold_functions.php | 8 ++++++++ 2 files changed, 16 insertions(+) diff --git a/tests/Unit/TholdExpressionStackOpsTest.php b/tests/Unit/TholdExpressionStackOpsTest.php index c0a5d221..f7904cf2 100644 --- a/tests/Unit/TholdExpressionStackOpsTest.php +++ b/tests/Unit/TholdExpressionStackOpsTest.php @@ -125,6 +125,14 @@ public function testRevOfZeroElementsLeavesTheStackUnchanged(): void { $this->assertSame([1, 2], $this->setOp([1, 2, 0], 'REV')); } + /** + * @return void + */ + public function testFractionalSetCountIsRejected(): void { + $this->assertSame([1, 2, 3], $this->setOp([1, 2, 3, 2.5], 'REV')); + $this->assertTrue($GLOBALS['rpn_error']); + } + /** * @return void */ diff --git a/thold_functions.php b/thold_functions.php index 0ca03553..e8053a40 100644 --- a/thold_functions.php +++ b/thold_functions.php @@ -685,6 +685,14 @@ function thold_expression_setops_rpn(string $operator, array &$stack): void { return; } + if ((float) $count !== floor((float) $count)) { + $rpn_error = true; + + return; + } + + $count = (int) $count; + // Popping yields the span top-down; keep it that way and index deliberately. $values = []; From 36012aa5015188eb8e3e5cf728c96d84d83ec5da Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Mon, 17 Aug 2026 14:59:50 -0700 Subject: [PATCH 7/7] ci: bound package index refreshes --- .github/workflows/plugin-ci-workflow.yml | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/.github/workflows/plugin-ci-workflow.yml b/.github/workflows/plugin-ci-workflow.yml index 79b16b7b..e17554e8 100644 --- a/.github/workflows/plugin-ci-workflow.yml +++ b/.github/workflows/plugin-ci-workflow.yml @@ -86,7 +86,24 @@ jobs: echo "PHP_BINARY=$(command -v php)" >> "$GITHUB_ENV" - name: Run apt-get update - run: sudo apt-get update + run: | + for attempt in 1 2 3; do + if sudo timeout 3m apt-get \ + -o Dpkg::Lock::Timeout=60 \ + -o Acquire::Retries=3 \ + -o Acquire::http::Timeout=30 \ + -o Acquire::https::Timeout=30 \ + update; then + exit 0 + fi + + if [ "$attempt" -lt 3 ]; then + sleep 10 + fi + done + + echo 'apt-get update failed after three bounded attempts.' >&2 + exit 1 - name: Install System Dependencies run: sudo apt-get install -y apache2 snmp snmpd rrdtool fping