Files
cacti/tests/Unit/DataCollection/Poller/PollerMultiHexOverwriteTest.php
Sean Mancini 96033267cb refactor(tests): reorganize unit tests into domain category folders (#7733)
* refactor(tests): reorganize unit tests into domain category folders

- Consolidate per-advisory GHSA regression tests into Security/ subfolders by vuln class
- Group all 186 test files into domain folders: DataCollection/, Ui/, Core/,
  Database/, Installer/, Scripts/, Security/, Plugin/
- Strip GitHub issue/GHSA numbers from filenames; keep IDs in test descriptions
- Fix all __DIR__ relative paths to match new folder nesting depth
- Update CI workflow path triggers and phpunit XML configs for new file locations
- Update .github/copilot-instructions.md with unit test organization guidelines
- Add tests/vendor/ to .gitignore

* add gaurds

* fix up guard for unit tests

* fix redeclaration of cacti_log in tests

* fix updates to unit tests

* fix test patjhs

* fix: update test __DIR__ paths for reorganized test structure

* fix: conditionally define read_user_setting to prevent redeclaration in tests

* fix: conditionally define functions to prevent redeclaration in tests

- Wrap clean_up_lines, raise_message, cacti_debug_backtrace, and get_debug_prefix
  with function_exists() checks to prevent conflicts when test stubs are loaded
  before lib/functions.php in the same PHP process

* fix: add unit test bootstrap to prevent function redeclaration errors

- Create tests/bootstrap-unit.php to load core libraries before tests
- Ensures lib/functions.php loads before test stubs define stub functions
- Test stubs use function_exists() checks, so they detect pre-loaded functions
- Update phpunit.xml to use bootstrap-unit.php for Unit testsuite
- Fixes 'Cannot redeclare read_user_setting()' errors when tests run in same process

* fix: load test stubs before global.php in unit test bootstrap

- Reorder bootstrap to load CactiStubs.php and UnitStubs.php first
- Ensures stub functions like __() are available when global.php loads
- global.php calls get_cacti_version_text() which needs __() for translations
- Fixes 'Call to undefined function __()' error during bootstrap

* fix: remove function conflicts from test stubs

- Remove stub definitions for functions already in lib/functions.php:
  cacti_sizeof, cacti_log, cacti_strtolower, read_config_option,
  read_user_setting, cacti_count
- Keep translation function stubs (__(), __esc()) needed by global.php
- Update bootstrap to load lib/functions.php after stubs
- This prevents function redeclaration errors while maintaining test isolation

* fix: prevent constant and variable redeclaration in bootstrap

- Remove explicit PHP_TESTING and CACTI_TEST_BOOTSTRAP defines from bootstrap
  (CactiStubs.php now handles this with guards)
- Add guards to CactiStubs.php to prevent redefinition of PHP_TESTING
- Only set CACTI_TEST_BOOTSTRAP env vars if not already set
- Fixes PHP warnings about 'Constant X already defined'

* fix: remove redundant lib/functions.php load from bootstrap

include/global.php already loads lib/functions.php at lines 287-288.
The bootstrap was trying to load it again after global.php, but global.php
needs format_cacti_version() to be available on lines 293-294 to define
CACTI_VERSION and CACTI_VERSION_FULL constants.

Solution: Let global.php handle loading lib/functions.php itself, and don't
load it separately in the bootstrap.

* fix: remove more conflicting function stubs from test helpers

- Remove stubs for functions also defined in lib/functions.php:
  raise_message, clean_up_lines, cacti_debug_backtrace, get_debug_prefix
- Remove POLLER_ID constant stub (defined unconditionally by global.php)
- Keep only translation functions and test-specific helpers
- This prevents function/constant redeclaration errors

* fix: minimize test stubs to only translation functions

Remove ALL helper functions and constants from UnitStubs.php that are:
- Defined in lib/functions.php (raise_message, clean_up_lines, etc.)
- Defined in lib/html_utility.php (srv, set_request_var)
- Defined in global_constants.php (POLLER_VERBOSITY_*, MESSAGE_LEVEL_*)
- Defined in global.php (POLLER_ID)

Keep ONLY translation functions (__(), __esc()) that are:
- Called by global.php during initialization
- Not defined elsewhere
- Required for proper bootstrap sequence

This eliminates all constant and function redeclaration errors.

* fix: remove __esc stub - already defined in global_languages.php

Keep only __() translation function stub, which is needed by global.php
and not defined elsewhere in Cacti core.

* fix: handle no-arg case in __() translation stub

The __() function may be called with just a string and no format arguments.
Previously, vsprintf() would fail with 'arguments array must contain 2 items'.
Now, if no args are provided, return the text as-is.
2026-08-16 19:23:31 -07:00

192 lines
6.3 KiB
PHP

<?php
declare(strict_types=1);
/*
+-------------------------------------------------------------------------+
| Copyright (C) 2004-2026 The Cacti Group |
+-------------------------------------------------------------------------+
| Cacti: The Complete RRDtool-based Graphing Solution |
+-------------------------------------------------------------------------+
*/
/*
* Regression test for issue #6879: multi-DS output hex values
* overwritten by raw string in process_poller_output().
*
* The bug was an unconditional assignment at lib/poller.php:643
* that overwrote the coerced value (hexdec result or 'U') with
* the raw $matches[1] string after the if/elseif/else block.
*
* @group regression
*/
require_once dirname(__DIR__, 3) . '/Helpers/CactiStubs.php';
/**
* Simulate the MULTI-output field value coercion logic from
* process_poller_output() lines 635-641.
*
* This is the FIXED version: no unconditional overwrite after
* the coercion block.
*
* @param string $raw_value The raw value from a key:value pair
*
* @return string|int The coerced value for RRD storage
*/
function coerce_multi_field_value_fixed(string $raw_value): string|int {
if (is_numeric($raw_value) || $raw_value === 'U') {
return $raw_value;
}
if (function_exists('is_hexadecimal') && is_hexadecimal($raw_value)) {
return hexdec($raw_value);
}
return 'U';
}
/**
* Simulate the BUGGY version that had the unconditional overwrite
* at line 643.
*
* @param string $raw_value The raw value from a key:value pair
*
* @return string|int Always returns raw_value (bug)
*/
function coerce_multi_field_value_buggy(string $raw_value): string|int {
if (is_numeric($raw_value) || $raw_value === 'U') {
$result = $raw_value;
} elseif (function_exists('is_hexadecimal') && is_hexadecimal($raw_value)) {
$result = hexdec($raw_value);
} else {
$result = 'U';
}
// BUG: unconditional overwrite (was line 643)
$result = $raw_value;
return $result;
}
if (!function_exists('is_hexadecimal')) {
function is_hexadecimal(string $result): bool {
$hexstr = str_replace([' ', '-'], ':', trim($result));
foreach (explode(':', $hexstr) as $part) {
if (strlen($part) != 2 || !ctype_xdigit($part)) {
return false;
}
}
return true;
}
}
// ===========================================================================
// Regression tests: fixed behavior
// ===========================================================================
describe('Regression #6879: multi-DS hex overwrite fix', function () {
test('numeric value preserved by fixed version', function () {
expect(coerce_multi_field_value_fixed('12345'))->toBe('12345');
});
test('U value preserved by fixed version', function () {
expect(coerce_multi_field_value_fixed('U'))->toBe('U');
});
test('hex value converted to decimal by fixed version', function () {
$r = coerce_multi_field_value_fixed('0A:0B');
expect($r)->toBeInt();
expect($r)->not->toBe('0A:0B');
});
test('non-numeric non-hex becomes U in fixed version', function () {
expect(coerce_multi_field_value_fixed('garbage'))->toBe('U');
});
test('buggy version always returns raw value (demonstrates the bug)', function () {
// The buggy version overwrites everything with raw input
expect(coerce_multi_field_value_buggy('garbage'))->toBe('garbage');
expect(coerce_multi_field_value_buggy('garbage'))->not->toBe('U');
});
test('fixed version returns U for garbage, buggy returns raw', function () {
$fixed = coerce_multi_field_value_fixed('not_a_number');
$buggy = coerce_multi_field_value_buggy('not_a_number');
expect($fixed)->toBe('U');
expect($buggy)->toBe('not_a_number');
expect($fixed)->not->toBe($buggy);
});
});
// ===========================================================================
// Mutation killers for the fix
// ===========================================================================
describe('Mutation killers for #6879 fix', function () {
test('numeric stays numeric, never becomes U', function () {
$r = coerce_multi_field_value_fixed('100');
expect($r)->toBe('100');
expect($r)->not->toBe('U');
});
test('U stays U, never becomes numeric', function () {
$r = coerce_multi_field_value_fixed('U');
expect($r)->toBe('U');
expect($r)->not->toBeInt();
});
test('garbage becomes U, never passes through raw', function () {
$r = coerce_multi_field_value_fixed('random_junk');
expect($r)->toBe('U');
expect($r)->not->toBe('random_junk');
});
test('fixed and buggy produce different results for garbage', function () {
$fixed = coerce_multi_field_value_fixed('xyz');
$buggy = coerce_multi_field_value_buggy('xyz');
expect($fixed)->not->toBe($buggy);
});
test('coercion is deterministic over 100 runs', function () {
$first = coerce_multi_field_value_fixed('test_value');
for ($i = 0; $i < 100; $i++) {
expect(coerce_multi_field_value_fixed('test_value'))->toBe($first);
}
});
test('negative numeric is preserved', function () {
expect(coerce_multi_field_value_fixed('-42.5'))->toBe('-42.5');
});
test('scientific notation is preserved', function () {
expect(coerce_multi_field_value_fixed('1.5e+09'))->toBe('1.5e+09');
});
test('empty string becomes U', function () {
expect(coerce_multi_field_value_fixed(''))->toBe('U');
});
});
// ===========================================================================
// Source verification: the bug is actually fixed
// ===========================================================================
describe('Source verification: overwrite line removed', function () {
test('no unconditional matches[1] overwrite after coercion block', function () {
$c = file_get_contents(__DIR__ . '/../../../../lib/poller.php');
// Find the first multi-DS coercion block (with data template)
$pos = strpos($c, "if (is_numeric(\$matches[1]) || (\$matches[1] == 'U'))");
$segment = substr($c, $pos, 500);
// The segment should contain the coercion if/elseif/else
expect($segment)->toContain('is_numeric($matches[1])');
expect($segment)->toContain('is_hexadecimal');
expect($segment)->toContain("= 'U'");
// But should NOT have an unconditional overwrite right after
// Look for the pattern: closing brace, blank line, then $matches[1] assignment
expect($segment)->not->toMatch('/\}\s*\n\s*\n\s*\$rrd_update_array.*\$matches\[1\]/');
});
});