From 843c40e9786cf123ee832f759fd75fbe6c6821a3 Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Sun, 16 Aug 2026 23:37:33 -0700 Subject: [PATCH] fix(thold): make the unit suffix conversions inverses of each other thold_display_to_raw() and thold_raw_to_display() each carried their own suffix table and disagreed. Entering 'p' scaled by 1e-9, while a value of 1e-12 rendered as 'f' and so parsed back as 1e-15: opening a threshold and saving it again divided its bound by a thousand, every time. Nano was missing from both, which is what pushed the rest of the small suffixes one place out of step. Past the largest and smallest entries the render read off the end of its pattern string and dropped the magnitude entirely, so 5e-18 displayed as 5. Both now read one shared table, so they cannot drift apart again, and a value outside the table keeps its scale rather than losing it. Stored bounds are untouched. What changes is the label they display under and the meaning of a newly typed suffix: 'p' is pico, as it always claimed to be. Refs #786 Signed-off-by: Thomas Vincent --- .github/workflows/php-unit-tests.yml | 92 ++++ .gitignore | 7 + composer.json | 49 ++ phpunit.xml | 31 ++ tests/Helpers/CactiStubs.php | 214 +++++++++ tests/TestCase.php | 65 +++ tests/Unit/TholdUnitSuffixTest.php | 173 +++++++ tests/bin/patch-coverage.php | 158 +++++++ tests/bootstrap-unit.php | 521 +++++++++++++++++++++ tests/docker/Dockerfile | 29 ++ tests/docker/docker-compose.yml | 15 + tests/fixtures/cacti-lib/variables.php | 22 + tests/fixtures/optional-core-functions.php | 45 ++ thold_functions.php | 163 ++----- 14 files changed, 1473 insertions(+), 111 deletions(-) create mode 100644 .github/workflows/php-unit-tests.yml create mode 100644 composer.json create mode 100644 phpunit.xml create mode 100644 tests/Helpers/CactiStubs.php create mode 100644 tests/TestCase.php create mode 100644 tests/Unit/TholdUnitSuffixTest.php create mode 100644 tests/bin/patch-coverage.php create mode 100644 tests/bootstrap-unit.php create mode 100644 tests/docker/Dockerfile create mode 100644 tests/docker/docker-compose.yml create mode 100644 tests/fixtures/cacti-lib/variables.php create mode 100644 tests/fixtures/optional-core-functions.php diff --git a/.github/workflows/php-unit-tests.yml b/.github/workflows/php-unit-tests.yml new file mode 100644 index 00000000..9ab561f2 --- /dev/null +++ b/.github/workflows/php-unit-tests.yml @@ -0,0 +1,92 @@ +# +-------------------------------------------------------------------------+ +# | 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. | +# | | +# | This program is distributed in the hope that it will be useful, | +# | but WITHOUT ANY WARRANTY; without even the implied warranty of | +# | MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | +# | GNU General Public License for more details. | +# +-------------------------------------------------------------------------+ +# | Cacti: The Complete RRDtool-based Graphing Solution | +# +-------------------------------------------------------------------------+ +# | This code is designed, written, and maintained by the Cacti Group. See | +# | about.php and/or the AUTHORS file for specific developer information. | +# +-------------------------------------------------------------------------+ +# | http://www.cacti.net/ | +# +-------------------------------------------------------------------------+ + + +name: PHP Unit Tests + +on: + push: + branches: + - main + - develop + pull_request: + branches: + - main + - develop + +permissions: + contents: read + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +jobs: + unit-test: + name: PHPUnit on PHP 8.1 (Docker) + runs-on: ubuntu-latest + + steps: + - name: Checkout Thold Plugin + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + # patch-coverage.php diffs against the base branch. + fetch-depth: 0 + + # The image is the same one developers run locally via + # `composer test:docker`, so a green run here is reproducible off-CI. + - name: Build test image + run: docker build --tag cacti-thold-test:php8.1 --file tests/docker/Dockerfile tests/docker + + - name: Validate composer.json + run: docker run --rm --volume "$PWD":/cacti/plugins/thold cacti-thold-test:php8.1 composer validate --strict --no-check-lock + + - name: Install dependencies + run: docker run --rm --volume "$PWD":/cacti/plugins/thold cacti-thold-test:php8.1 composer install --no-progress --no-ansi + + # Same scripts a developer runs locally, and the same names Cacti core uses. + - name: Lint every PHP source file + run: docker run --rm --volume "$PWD":/cacti/plugins/thold cacti-thold-test:php8.1 composer lint + + - name: Run unit tests with coverage + run: | + docker run --rm --volume "$PWD":/cacti/plugins/thold cacti-thold-test:php8.1 \ + composer test:coverage + + # Whole-file coverage is meaningless here: most of the plugin only runs + # inside a live Cacti. What is enforceable is that a change covers the + # lines it adds. + - name: Enforce coverage of changed lines + if: github.event_name == 'pull_request' + env: + BASE_REF: origin/${{ github.base_ref }} + run: | + docker run --rm --volume "$PWD":/cacti/plugins/thold --env BASE_REF \ + cacti-thold-test:php8.1 \ + sh -c 'git config --global --add safe.directory /cacti/plugins/thold && php tests/bin/patch-coverage.php coverage/clover.xml "$BASE_REF" 100' + + - name: Upload coverage report + if: always() + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 + with: + name: coverage + path: coverage/ + if-no-files-found: warn diff --git a/.gitignore b/.gitignore index eb716067..0806dc45 100644 --- a/.gitignore +++ b/.gitignore @@ -20,3 +20,10 @@ # +-------------------------------------------------------------------------+ locales/po/*.mo + +/vendor/ +/composer.lock +/.phpunit.cache/ +/coverage/ +/coverage/ +/.phpunit.result.cache diff --git a/composer.json b/composer.json new file mode 100644 index 00000000..94aead2f --- /dev/null +++ b/composer.json @@ -0,0 +1,49 @@ +{ + "_comment": [ + "+-------------------------------------------------------------------------+", + "| 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. |", + "| |", + "| This program is distributed in the hope that it will be useful, |", + "| but WITHOUT ANY WARRANTY; without even the implied warranty of |", + "| MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the |", + "| GNU General Public License for more details. |", + "+-------------------------------------------------------------------------+", + "| Cacti: The Complete RRDtool-based Graphing Solution |", + "+-------------------------------------------------------------------------+", + "| http://www.cacti.net/ |", + "+-------------------------------------------------------------------------+" + ], + "name": "cacti/plugin-thold", + "description": "Thold Plugin for Cacti", + "type": "project", + "license": "GPL-2.0-only", + "require-dev": { + "overtrue/phplint": "^9.6", + "phpunit/phpunit": "^10.5.64" + }, + "scripts": { + "lint": "phplint --no-cache --exclude=vendor ", + "test": "phpunit --display-warnings", + "test:coverage": "phpunit --display-warnings --coverage-clover=coverage/clover.xml", + "test:docker": "docker compose -f tests/docker/docker-compose.yml run --rm phpunit" + }, + "config": { + "sort-packages": true, + "vendor-dir": "vendor", + "platform": { + "php": "8.1.0" + }, + "platform-check": true + }, + "autoload-dev": { + "classmap": [ + "tests/Helpers/", + "tests/TestCase.php" + ] + } +} diff --git a/phpunit.xml b/phpunit.xml new file mode 100644 index 00000000..731e8bbc --- /dev/null +++ b/phpunit.xml @@ -0,0 +1,31 @@ + + + + + + + + + ./tests/Unit + + + + + + + thold_functions.php + + + diff --git a/tests/Helpers/CactiStubs.php b/tests/Helpers/CactiStubs.php new file mode 100644 index 00000000..fdb27263 --- /dev/null +++ b/tests/Helpers/CactiStubs.php @@ -0,0 +1,214 @@ +}> + */ + 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 = []; + + /** + * Return values chosen by a fragment of the SQL, keyed by function name. + * Each entry is [fragment, value]. Consulted before $returns. + * + * @var array> + */ + public static $matchedReturns = []; + + /** + * Values handed back on every call, keyed by function name. Consulted last. + * + * @var array + */ + public static $stickyReturns = []; + + /** + * 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 = []; + + /** + * Mail handed to Cacti's mailer(), in order. + * + * @var array + */ + public static $mail = []; + + /** + * Clear all recorded and programmed state. + * + * @return void + */ + public static function reset() { + self::$calls = []; + self::$returns = []; + self::$matchedReturns = []; + self::$stickyReturns = []; + self::$requestVars = []; + self::$configOptions = []; + self::$log = []; + self::$mail = []; + } + + /** + * 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]; + } + + /** + * Hand back $value for every call to $fn. + * + * @param string $fn Cacti function name. + * @param mixed $value Value to hand back. + * + * @return void + */ + public static function willAlwaysReturn($fn, $value) { + self::$stickyReturns[$fn] = $value; + } + + /** + * 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; + } + + /** + * Answer any call to $fn whose SQL contains $fragment with $value. + * + * A function such as db_fetch_cell_prepared is called many times with + * different queries in one run, so a positional queue would break as soon + * as the code under test reordered a lookup. Matching on the query keeps + * the fixture readable and stable. + * + * @param string $fn Cacti function name. + * @param string $fragment Distinctive substring of the SQL. + * @param mixed $value Value to hand back. + * + * @return void + */ + public static function willReturnFor($fn, $fragment, $value) { + self::$matchedReturns[$fn][] = [$fragment, $value]; + } + + /** + * Take the return value for a call: a SQL match first, then the queue, then + * the type default. + * + * @param string $fn Cacti function name. + * @param mixed $default Fallback when nothing matches. + * @param string $sql SQL the caller passed, for matching. + * + * @return mixed + */ + public static function nextReturn($fn, $default, $sql = '') { + if ($sql !== '' && !empty(self::$matchedReturns[$fn])) { + $flat = preg_replace('/\s+/', ' ', $sql); + + foreach (self::$matchedReturns[$fn] as $entry) { + if (strpos($flat, preg_replace('/\s+/', ' ', $entry[0])) !== false) { + return $entry[1]; + } + } + } + + if (!empty(self::$returns[$fn])) { + return array_shift(self::$returns[$fn]); + } + + if (array_key_exists($fn, self::$stickyReturns)) { + return self::$stickyReturns[$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/TestCase.php b/tests/TestCase.php new file mode 100644 index 00000000..67342561 --- /dev/null +++ b/tests/TestCase.php @@ -0,0 +1,65 @@ + + */ + public static function suffixProvider() { + return [ + 'femto' => ['5f', 5.0e-15], + 'pico' => ['5p', 5.0e-12], + 'nano' => ['5n', 5.0e-9], + 'micro' => ['5u', 5.0e-6], + 'milli' => ['5m', 5.0e-3], + 'kilo' => ['5K', 5.0e3], + 'mega' => ['5M', 5.0e6], + 'giga' => ['5G', 5.0e9], + 'tera' => ['5T', 5.0e12], + 'peta' => ['5P', 5.0e15], + 'exa' => ['5E', 5.0e18], + 'zetta' => ['5Z', 5.0e21], + 'yotta' => ['5Y', 5.0e24], + ]; + } + + /** + * @dataProvider suffixProvider + * + * @param string $typed + * @param float $stored + * + * @return void + */ + public function testEachSuffixScalesByItsSiFactor($typed, $stored): void { + $this->assertEqualsWithDelta($stored, thold_display_to_raw($typed, 'thold_hi'), abs($stored) * 1.0e-9); + } + + /** + * @dataProvider suffixProvider + * + * @param string $typed + * @param float $stored + * + * @return void + */ + public function testEachStoredValueRendersWithItsSiSuffix($typed, $stored): void { + $this->assertSame($typed, thold_raw_to_display($stored)); + } + + /** + * Opening a threshold and saving it again must not change it. This is the + * failure that mattered: a value stored at 1e-12 rendered as 5f, which + * parsed back as 1e-15, so every visit to the form divided it by a + * thousand. + * + * @dataProvider suffixProvider + * + * @param string $typed + * @param float $stored + * + * @return void + */ + public function testAValueSurvivesBeingDisplayedAndReEntered($typed, $stored): void { + $round_tripped = thold_display_to_raw(thold_raw_to_display($stored), 'thold_hi'); + + $this->assertEqualsWithDelta($stored, $round_tripped, abs($stored) * 1.0e-9); + } + + /** + * @return void + */ + public function testAPlainNumberIsLeftAlone(): void { + $this->assertSame('42', thold_display_to_raw('42', 'thold_hi')); + $this->assertSame('42', thold_raw_to_display(42)); + } + + /** + * @return void + */ + public function testZeroIsLeftAlone(): void { + $this->assertSame('0', thold_raw_to_display(0)); + } + + /** + * @return void + */ + public function testNegativeValuesKeepTheirSign(): void { + $this->assertSame('-5K', thold_raw_to_display(-5000)); + $this->assertEqualsWithDelta(-5000, thold_display_to_raw('-5K', 'thold_hi'), 1.0e-6); + } + + /** + * @return array + */ + public static function rejectedInputProvider() { + return [ + 'unknown suffix' => ['5x'], + 'letters only' => ['abc'], + 'empty' => [''], + ]; + } + + /** + * @dataProvider rejectedInputProvider + * + * @param string $typed + * + * @return void + */ + public function testUnusableInputIsRejectedAndFlagged($typed): void { + $this->assertFalse(thold_display_to_raw($typed, 'thold_hi')); + $this->assertArrayHasKey('thold_hi', $_SESSION['sess_error_fields']); + } + + /** + * @return void + */ + public function testNonNumericInputHasNoDisplayForm(): void { + $this->assertFalse(thold_raw_to_display('abc')); + } + + /** + * Beyond the largest and smallest suffix there is nothing left to index, + * and the old code read past the end of the pattern and dropped the + * magnitude entirely. + * + * @return void + */ + public function testMagnitudesBeyondTheLargestSuffixKeepTheirScale(): void { + $rendered = thold_raw_to_display(5.0e27); + + $this->assertNotSame('5', $rendered); + $this->assertEqualsWithDelta(5.0e27, (float) thold_display_to_raw($rendered, 'thold_hi'), 5.0e18); + } + + /** + * @return void + */ + public function testMagnitudesBelowTheSmallestSuffixKeepTheirScale(): void { + $rendered = thold_raw_to_display(5.0e-18); + + $this->assertNotSame('5', $rendered); + $this->assertEqualsWithDelta(5.0e-18, (float) thold_display_to_raw($rendered, 'thold_hi'), 5.0e-27); + } +} diff --git a/tests/bin/patch-coverage.php b/tests/bin/patch-coverage.php new file mode 100644 index 00000000..83f9c73f --- /dev/null +++ b/tests/bin/patch-coverage.php @@ -0,0 +1,158 @@ + [min-percent] + * + * Exits 1 if coverage is below the threshold, 2 on bad input. + */ + +if ($argc < 3) { + fwrite(STDERR, "usage: patch-coverage.php [min-percent]\n"); + + exit(2); +} + +$clover_path = $argv[1]; +$base_ref = $argv[2]; +$minimum = isset($argv[3]) ? (float) $argv[3] : 100.0; + +if (!is_readable($clover_path)) { + fwrite(STDERR, "cannot read coverage report: $clover_path\n"); + + exit(2); +} + +/** + * Line numbers each measured file changed, keyed by repository-relative path. + * + * Only added and modified lines count. Deletions have nothing left to cover, + * and context lines were not part of this change. + * + * Paths stay repository-relative so the report can be produced in a container + * and evaluated on the host, where the absolute paths differ. + * + * @param string $base_ref Git ref to diff against. + * + * @return array> + */ +function changed_lines($base_ref) { + $command = 'git diff --no-ext-diff --unified=0 --no-color --diff-filter=AM ' . escapeshellarg($base_ref) . '...HEAD -- "*.php"'; + $diff = shell_exec($command); + + if ($diff === null) { + fwrite(STDERR, "git diff failed\n"); + + exit(2); + } + + $changed = []; + $file = null; + + foreach (explode("\n", $diff) as $line) { + if (strncmp($line, '+++ b/', 6) === 0) { + $file = substr($line, 6); + $changed[$file] = []; + } elseif (strncmp($line, '@@', 2) === 0 && $file !== null) { + if (preg_match('/\+(\d+)(?:,(\d+))?/', $line, $match)) { + $start = (int) $match[1]; + $count = isset($match[2]) ? (int) $match[2] : 1; + + for ($i = 0; $i < $count; $i++) { + $changed[$file][$start + $i] = true; + } + } + } + } + + return $changed; +} + +$changed = changed_lines($base_ref); +$clover = simplexml_load_file($clover_path); + +if ($clover === false) { + fwrite(STDERR, "cannot parse coverage report: $clover_path\n"); + + exit(2); +} + +$covered = 0; +$total = 0; +$missing = []; + +foreach ($clover->xpath('//file') as $file) { + $path = (string) $file['name']; + $relative = null; + + foreach (array_keys($changed) as $candidate) { + if ($path === $candidate || substr($path, -strlen('/' . $candidate)) === '/' . $candidate) { + $relative = $candidate; + + break; + } + } + + if ($relative === null) { + continue; + } + + foreach ($file->line as $line) { + $number = (int) $line['num']; + + // Only statement lines are measurable; method markers double-count. + if ((string) $line['type'] !== 'stmt' || !isset($changed[$relative][$number])) { + continue; + } + + $total++; + + if ((int) $line['count'] > 0) { + $covered++; + } else { + $missing[] = $relative . ':' . $number; + } + } +} + +if ($total === 0) { + print "Patch coverage: no measured lines changed.\n"; + + exit(0); +} + +$percent = ($covered / $total) * 100; + +printf("Patch coverage: %.2f%% (%d/%d lines)\n", $percent, $covered, $total); + +if ($missing !== []) { + print "Uncovered changed lines:\n " . implode("\n ", $missing) . "\n"; +} + +if ($percent + 0.005 < $minimum) { + printf("FAIL: below the %.2f%% minimum.\n", $minimum); + + exit(1); +} + +exit(0); diff --git a/tests/bootstrap-unit.php b/tests/bootstrap-unit.php new file mode 100644 index 00000000..dbb050e7 --- /dev/null +++ b/tests/bootstrap-unit.php @@ -0,0 +1,521 @@ + 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; + +// Cacti's list of enabled plugins; thold_check_threshold() declares it global. +$GLOBALS['plugins'] = []; + +// Cacti's debug flag, also declared global by thold_check_threshold(). +$GLOBALS['debug'] = false; + +if (!function_exists('db_execute')) { + function db_execute($sql, $log = true, $db_conn = false) { + CactiStubs::record('db_execute', $sql); + + return CactiStubs::nextReturn('db_execute', true, $sql); + } +} + +if (!function_exists('db_execute_prepared')) { + function db_execute_prepared($sql, $params = [], $log = true, $db_conn = false) { + CactiStubs::record('db_execute_prepared', $sql, $params); + + return CactiStubs::nextReturn('db_execute_prepared', true, $sql); + } +} + +if (!function_exists('db_fetch_assoc')) { + function db_fetch_assoc($sql, $log = true, $db_conn = false) { + CactiStubs::record('db_fetch_assoc', $sql); + + return CactiStubs::nextReturn('db_fetch_assoc', [], $sql); + } +} + +if (!function_exists('db_fetch_assoc_prepared')) { + function db_fetch_assoc_prepared($sql, $params = [], $log = true, $db_conn = false) { + CactiStubs::record('db_fetch_assoc_prepared', $sql, $params); + + return CactiStubs::nextReturn('db_fetch_assoc_prepared', [], $sql); + } +} + +if (!function_exists('db_fetch_row')) { + function db_fetch_row($sql, $log = true, $db_conn = false) { + CactiStubs::record('db_fetch_row', $sql); + + return CactiStubs::nextReturn('db_fetch_row', [], $sql); + } +} + +if (!function_exists('db_fetch_row_prepared')) { + function db_fetch_row_prepared($sql, $params = [], $log = true, $db_conn = false) { + CactiStubs::record('db_fetch_row_prepared', $sql, $params); + + return CactiStubs::nextReturn('db_fetch_row_prepared', [], $sql); + } +} + +/* + * Cacti's cell fetchers return false, not '', when the query matches no row. + * The difference matters on PHP 8: false coerces to 0 in arithmetic while '' + * raises a TypeError, so a stub returning '' invents failures that production + * does not have. + */ +if (!function_exists('db_fetch_cell')) { + function db_fetch_cell($sql, $col_name = '', $log = true, $db_conn = false) { + CactiStubs::record('db_fetch_cell', $sql); + + return CactiStubs::nextReturn('db_fetch_cell', false, $sql); + } +} + +if (!function_exists('db_fetch_cell_prepared')) { + function db_fetch_cell_prepared($sql, $params = [], $col_name = '', $log = true, $db_conn = false) { + CactiStubs::record('db_fetch_cell_prepared', $sql, $params); + + return CactiStubs::nextReturn('db_fetch_cell_prepared', false, $sql); + } +} + +if (!function_exists('db_qstr')) { + function db_qstr($string) { + return "'" . str_replace("'", "''", (string) $string) . "'"; + } +} + +if (!function_exists('db_begin_transaction')) { + function db_begin_transaction() { + CactiStubs::record('db_begin_transaction'); + + return CactiStubs::nextReturn('db_begin_transaction', true); + } +} + +if (!function_exists('db_commit_transaction')) { + function db_commit_transaction() { + CactiStubs::record('db_commit_transaction'); + + return CactiStubs::nextReturn('db_commit_transaction', true); + } +} + +if (!function_exists('db_rollback_transaction')) { + function db_rollback_transaction() { + CactiStubs::record('db_rollback_transaction'); + + return CactiStubs::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(CactiStubs::$configOptions[$name]) ? CactiStubs::$configOptions[$name] : ''; + } +} + +if (!function_exists('set_config_option')) { + function set_config_option($name, $value) { + CactiStubs::$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) { + CactiStubs::$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(CactiStubs::$requestVars[$name]) ? CactiStubs::$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(CactiStubs::$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 CactiStubs::nextReturn('get_simple_graph_perms', true); + } +} + +if (!function_exists('get_policies')) { + function get_policies($user_id) { + return CactiStubs::nextReturn('get_policies', []); + } +} + +if (!function_exists('get_policy_where')) { + function get_policy_where($graph_auth_method, $policies, $sql_where) { + CactiStubs::record('get_policy_where', $sql_where); + + return CactiStubs::nextReturn('get_policy_where', $sql_where); + } +} + +if (!function_exists('expand_title')) { + function expand_title($host_id, $snmp_query_id, $snmp_index, $title) { + CactiStubs::record('expand_title', $title); + + return CactiStubs::nextReturn('expand_title', $title); + } +} + +if (!function_exists('get_graph_title')) { + function get_graph_title($local_graph_id) { + return CactiStubs::nextReturn('get_graph_title', 'Traffic - eth0'); + } +} + +if (!function_exists('rrdtool_function_fetch')) { + function rrdtool_function_fetch($local_data_id, $start_time, $end_time, $resolution = 0, $show_unknown = false, $rrdtool_file = null) { + CactiStubs::record('rrdtool_function_fetch', (string) $local_data_id); + + return CactiStubs::nextReturn('rrdtool_function_fetch', []); + } +} + +if (!function_exists('get_data_source_path')) { + function get_data_source_path($local_data_id, $expand_paths = true) { + return '/var/lib/cacti/rra/test_' . (int) $local_data_id . '.rrd'; + } +} + +if (!function_exists('sql_save')) { + function sql_save($array_items, $table_name, $key_cols = 'id', $autoinc = true, $db_conn = false) { + CactiStubs::record('sql_save', $table_name, $array_items); + + return CactiStubs::nextReturn('sql_save', 1); + } +} + +if (!function_exists('db_affected_rows')) { + function db_affected_rows($db_conn = false) { + return CactiStubs::nextReturn('db_affected_rows', 1); + } +} + +if (!function_exists('rrdtool_function_graph')) { + function rrdtool_function_graph($local_graph_id, $rra_id, $graph_data_array, $rrdtool_pipe = false, &$xport_meta = [], $user = 0) { + CactiStubs::record('rrdtool_function_graph', (string) $local_graph_id); + + // A one-pixel PNG stands in for the rendered graph. + return base64_decode('iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAAAAAA6fptVAAAACklEQVR4nGMAAQAABQABDQottAAAAABJRU5ErkJggg==', true); + } +} + +if (!function_exists('get_timespan')) { + function get_timespan(&$timespan, $time, $span, $first_weekdayid) { + $timespan['begin_now'] = $time - 86400; + $timespan['end_now'] = $time; + } +} + +if (!function_exists('read_user_setting')) { + function read_user_setting($config_name, $default = false, $force = false, $user = 0) { + return CactiStubs::nextReturn('read_user_setting', $default); + } +} + +if (!function_exists('get_selected_theme')) { + function get_selected_theme() { + return 'modern'; + } +} + +if (!function_exists('mailer')) { + function mailer($from, $to, $cc = '', $bcc = '', $replyto = '', $subject = '', $body = '', $body_text = '', $attachments = null, $headers = [], $html = true) { + CactiStubs::$mail[] = [ + 'to' => is_array($to) ? implode(',', $to) : (string) $to, + 'bcc' => is_array($bcc) ? implode(',', $bcc) : (string) $bcc, + 'subject' => (string) $subject, + ]; + + return CactiStubs::nextReturn('mailer', ''); + } +} + +if (!function_exists('cacti_snmp_send')) { + function cacti_snmp_send($hostname, $version, $community, $oid, $value, $type = 's') { + CactiStubs::record('cacti_snmp_send', (string) $oid); + + return true; + } +} + +if (!function_exists('array_rekey')) { + function array_rekey($array, $key, $key_value) { + $ret_array = []; + + if (is_array($array)) { + foreach ($array as $item) { + $item_key = $item[$key]; + + if (is_array($key_value)) { + foreach ($key_value as $value) { + $ret_array[$item_key][$value] = $item[$value]; + } + } else { + $ret_array[$item_key] = $item[$key_value]; + } + } + } + + return $ret_array; + } +} + +if (!function_exists('clean_up_name')) { + function clean_up_name($string) { + $string = preg_replace('/[\s\.]+/', '_', $string); + $string = preg_replace('/[^a-zA-Z0-9_]+/', '', $string); + + return preg_replace('/_{2,}/', '_', $string); + } +} + +if (!function_exists('plugin_maint_check_cacti_host')) { + function plugin_maint_check_cacti_host($host_id) { + return CactiStubs::nextReturn('plugin_maint_check_cacti_host', false); + } +} + +if (!function_exists('api_plugin_is_enabled')) { + function api_plugin_is_enabled($plugin) { + return CactiStubs::nextReturn('api_plugin_is_enabled', false); + } +} + +if (!function_exists('api_plugin_hook')) { + function api_plugin_hook($name, $data = '') { + CactiStubs::record('api_plugin_hook', $name); + + return $data; + } +} + +if (!function_exists('api_user_realm_auth')) { + function api_user_realm_auth($filename = '') { + return CactiStubs::nextReturn('api_user_realm_auth', true); + } +} + +if (!function_exists('raise_message')) { + function raise_message($message_id, $message = '', $level = 0) { + CactiStubs::record('raise_message', (string) $message_id); + } +} + +if (!function_exists('rrdtool_execute')) { + function rrdtool_execute($command, $log_to_stdout = false, $output_flag = 1, $rrdtool_pipe = false, $logopt = 'WEBLOG') { + CactiStubs::record('rrdtool_execute', $command); + + return CactiStubs::nextReturn('rrdtool_execute', ''); + } +} + +if (!function_exists('rrdtool_function_interface_speed')) { + function rrdtool_function_interface_speed($data_local) { + return CactiStubs::nextReturn('rrdtool_function_interface_speed', 0); + } +} + +if (!function_exists('get_timeinstate')) { + function get_timeinstate($host) { + return CactiStubs::nextReturn('get_timeinstate', '1 day'); + } +} + +if (!function_exists('get_daysfromtime')) { + function get_daysfromtime($timestamp) { + return CactiStubs::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('RRDTOOL_OUTPUT_STDOUT')) { + define('RRDTOOL_OUTPUT_STDOUT', 1); +} + +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..518f7321 --- /dev/null +++ b/tests/docker/Dockerfile @@ -0,0 +1,29 @@ +# 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 gmp-dev \ + && docker-php-ext-install gmp \ + && 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 && composer test"] 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/tests/fixtures/cacti-lib/variables.php b/tests/fixtures/cacti-lib/variables.php new file mode 100644 index 00000000..956faca5 --- /dev/null +++ b/tests/fixtures/cacti-lib/variables.php @@ -0,0 +1,22 @@ + 255) { + $s = substr($s, 0, 255); + } + + $s = str_replace(["\0", '|', '{', '}'], '', $s); + + return 'RLIKE ' . db_qstr($s, $db_conn); + } +} + +if (!function_exists('get_total_row_data')) { + function get_total_row_data($user_id, $sql, $sql_params = [], $class = '', $timeout = 86400) { + CactiStubs::record('get_total_row_data', $sql, $sql_params); + + return CactiStubs::nextReturn('get_total_row_data', 0); + } +} diff --git a/thold_functions.php b/thold_functions.php index 018cf99b..6d25c94c 100644 --- a/thold_functions.php +++ b/thold_functions.php @@ -5279,6 +5279,40 @@ function thold_create_new_graph_from_template() { * @param mixed $number * @param mixed $field_name */ +/** + * SI suffixes thold accepts on a threshold bound, smallest first. + * + * thold_display_to_raw() and thold_raw_to_display() are inverses of each + * other, so they read the same table rather than each carrying their own + * copy. They previously disagreed: 'p' scaled by 1e-9 on the way in while + * 1e-12 rendered as 'f' on the way out, so a bound was divided by a thousand + * every time its form was opened and saved. + * + * @return array Suffix to the factor it multiplies by. + */ +function thold_unit_suffixes() { + static $suffixes = [ + 'y' => 1e-24, + 'z' => 1e-21, + 'a' => 1e-18, + 'f' => 1e-15, + 'p' => 1e-12, + 'n' => 1e-9, + 'u' => 1e-6, + 'm' => 1e-3, + 'K' => 1e3, + 'M' => 1e6, + 'G' => 1e9, + 'T' => 1e12, + 'P' => 1e15, + 'E' => 1e18, + 'Z' => 1e21, + 'Y' => 1e24, + ]; + + return $suffixes; +} + function thold_display_to_raw($number, $field_name) { $number = trim($number); @@ -5291,96 +5325,19 @@ function thold_display_to_raw($number, $field_name) { return $number; } - $number = trim(substr($number, 0, -1)); + $number = trim(substr($number, 0, -1)); + $suffixes = thold_unit_suffixes(); - if (!is_numeric($number)) { + if (!is_numeric($number) || !isset($suffixes[$suffix])) { $_SESSION['sess_error_fields'][$field_name] = $field_name; raise_message(3); return false; } - switch($suffix) { - case 'f': - return $number * 1e-15; - - break; - case 'p': - return $number * 1e-9; - - break; - case 'u': - return $number * 1e-6; - - break; - case 'm': - return $number * 1e-3; - - break; - case 'K': - return $number * 1e3; - - break; - case 'M': - return $number * 1e6; - - break; - case 'G': - return $number * 1e9; - - break; - case 'T': - return $number * 1e12; - - break; - case 'P': - return $number * 1e15; - - break; - case 'E': - return $number * 1e18; - - break; - case 'Z': - return $number * 1e21; - - break; - case 'Y': - return $number * 1e24; - - break; - default: - $_SESSION['sess_error_fields'][$field_name] = $field_name; - raise_message(3); - - return false; - } + return $number * $suffixes[$suffix]; } -/** - * thold_display_to_raw - Converts a displayed number to a raw - * numeric value. This function converts number like '100M' - * to the raw number 100,000,000, etc. - * - * Supported Units - * - * Unit Expression - * ---- ------------------------------------- - * f Fermo (10e-12) - * p Pico (10e-9) - * u Micro (10e-6) - * m Milli (10e-3) - * K Killo (10e3) - * M Mega (10e6) - * G Giga (10e9) - * T Terra (10e12) - * P Peta (10e15) - * E Exa (10e18) - * Z Zeta (10e21) - * Y Yota (10e24) - * - * @param mixed $number - */ function thold_raw_to_display($number) { if ($number != '') { $number = trim($number); @@ -5394,42 +5351,26 @@ function thold_raw_to_display($number) { return trim($number); } - if ($number > 0) { - $multiplier = 1; - } else { - $multiplier = -1; - } - - $number = abs($number); - $suffix = ''; - - if ($number > 1) { - $pattern = 'KMGTPEZY'; - $count = 0; + $multiplier = $number > 0 ? 1 : -1; + $number = abs($number); - while ($number >= 1e3) { - $count++; - $number /= 1e3; - } + // The largest scale that still leaves a value of one or more, where the + // empty suffix stands for a scale of one. Factors ascend, so the ratio + // falls monotonically and the last match is the one wanted. + $scales = thold_unit_suffixes(); + $scales = array_slice($scales, 0, 8, true) + ['' => 1.0] + array_slice($scales, 8, null, true); - if ($count > 0) { - $suffix = $pattern[$count - 1]; - } - } else { - $pattern = 'mupf'; - $count = 0; - - while ($number < 1) { - $count++; - $number *= 1e3; - } + $suffix = ''; + $factor = 1.0; - if ($count > 0) { - $suffix = $pattern[$count - 1]; + foreach ($scales as $candidate => $candidate_factor) { + if ($number / $candidate_factor >= 1) { + $suffix = $candidate; + $factor = $candidate_factor; } } - return trim(($number * $multiplier) . $suffix); + return trim((($number / $factor) * $multiplier) . $suffix); } function save_thold() {