Files
cacti/tests/Unit/Plugin/PluginMoveorderBugTest.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

196 lines
9.3 KiB
PHP

<?php
/*
+-------------------------------------------------------------------------+
| Copyright (C) 2004-2026 The Cacti Group |
| |
| This program is free software; you can redistribute it and/or |
| modify it under the terms of the GNU General Public License |
| as published by the Free Software Foundation; either version 2 |
| of the License, or (at your option) any later version. |
+-------------------------------------------------------------------------+
| Cacti: The Complete RRDtool-based Graphing Solution |
+-------------------------------------------------------------------------+
*/
/*
* Regression tests for two bugs in lib/plugins.php that corrupt plugin_config:
*
* BUG 1 (moveup/movedown NULL->0 corruption):
* api_plugin_moveup() and api_plugin_movedown() swap plugin order by
* renaming rows via a three-step id rotation (current->temp, prior->current,
* temp->prior). When called at the boundary (first plugin moved up, last
* plugin moved down), the subquery MAX/MIN returns NULL. Cacti strips
* STRICT_TRANS_TABLES from the session SQL mode on every connection; under
* non-strict mode, UPDATE SET id = NULL on a NOT NULL column silently stores
* 0, leaving the plugin with a corrupted primary key.
*
* BUG 2 (plugins_load_temp_table 1062 on id=0 row):
* plugin_config.id is AUTO_INCREMENT. Cacti also strips NO_AUTO_VALUE_ON_ZERO
* on connect. When an id=0 row exists in plugin_config (caused by bug 1 or a
* plugin upgrade script), the bulk INSERT INTO temp SELECT * FROM plugin_config
* reassigns the 0 to the next AUTO_INCREMENT sequence value, colliding with
* whatever row already holds that id and producing ERROR 1062.
*/
$libPluginsPath = __DIR__ . '/../../../lib/plugins.php';
$pluginsPath = __DIR__ . '/../../../plugins.php';
// ---------------------------------------------------------------------------
// api_plugin_moveup
// ---------------------------------------------------------------------------
test('api_plugin_moveup has !empty($prior_id) guard around the three-step swap', function () use ($libPluginsPath) {
$source = file_get_contents($libPluginsPath);
$fn_pos = strpos($source, 'function api_plugin_moveup(');
$guard_pos = strpos($source, 'if (!empty($prior_id))', $fn_pos);
expect($fn_pos)->not->toBeFalse('api_plugin_moveup not found');
expect($guard_pos)->not->toBeFalse('!empty($prior_id) guard missing from api_plugin_moveup');
});
test('api_plugin_moveup swap executes only after the prior_id guard, not before', function () use ($libPluginsPath) {
$source = file_get_contents($libPluginsPath);
$fn_pos = strpos($source, 'function api_plugin_moveup(');
$guard_pos = strpos($source, 'if (!empty($prior_id))', $fn_pos);
$swap_pos = strpos($source, 'UPDATE plugin_config SET id = ? WHERE id = ?', $fn_pos);
expect($guard_pos)->not->toBeFalse();
expect($swap_pos)->not->toBeFalse();
// The first UPDATE must come after the empty() guard.
expect($swap_pos)->toBeGreaterThan($guard_pos);
});
test('api_plugin_moveup temp_id computation is inside the prior_id guard', function () use ($libPluginsPath) {
$source = file_get_contents($libPluginsPath);
$fn_pos = strpos($source, 'function api_plugin_moveup(');
$guard_pos = strpos($source, 'if (!empty($prior_id))', $fn_pos);
$temp_id_pos = strpos($source, '$temp_id = db_fetch_cell(\'SELECT MAX(id) FROM plugin_config\')', $fn_pos);
expect($guard_pos)->not->toBeFalse();
expect($temp_id_pos)->not->toBeFalse('$temp_id not found in moveup');
// temp_id must be computed after the guard, not before the NULL check.
expect($temp_id_pos)->toBeGreaterThan($guard_pos);
});
test('api_plugin_moveup does not assign $prior_id to any id column before the empty guard', function () use ($libPluginsPath) {
$source = file_get_contents($libPluginsPath);
$fn_pos = strpos($source, 'function api_plugin_moveup(');
$guard_pos = strpos($source, 'if (!empty($prior_id))', $fn_pos);
// Extract the slice between function start and the guard; it must contain
// no UPDATE statement, because any UPDATE before the guard would set id
// to a potentially NULL $prior_id value.
$pre_guard = substr($source, $fn_pos, $guard_pos - $fn_pos);
expect($pre_guard)->not->toContain('UPDATE plugin_config SET id');
});
// ---------------------------------------------------------------------------
// api_plugin_movedown
// ---------------------------------------------------------------------------
test('api_plugin_movedown has outer !empty($id) guard', function () use ($libPluginsPath) {
$source = file_get_contents($libPluginsPath);
$fn_pos = strpos($source, 'function api_plugin_movedown(');
$id_guard = strpos($source, 'if (!empty($id))', $fn_pos);
expect($fn_pos)->not->toBeFalse();
expect($id_guard)->not->toBeFalse('!empty($id) guard missing from api_plugin_movedown');
});
test('api_plugin_movedown has !empty($next_id) guard around the three-step swap', function () use ($libPluginsPath) {
$source = file_get_contents($libPluginsPath);
$fn_pos = strpos($source, 'function api_plugin_movedown(');
$guard_pos = strpos($source, 'if (!empty($next_id))', $fn_pos);
expect($fn_pos)->not->toBeFalse();
expect($guard_pos)->not->toBeFalse('!empty($next_id) guard missing from api_plugin_movedown');
});
test('api_plugin_movedown swap executes only after the next_id guard, not before', function () use ($libPluginsPath) {
$source = file_get_contents($libPluginsPath);
$fn_pos = strpos($source, 'function api_plugin_movedown(');
$guard_pos = strpos($source, 'if (!empty($next_id))', $fn_pos);
$swap_pos = strpos($source, 'UPDATE plugin_config SET id = ? WHERE id = ?', $fn_pos);
expect($guard_pos)->not->toBeFalse();
expect($swap_pos)->not->toBeFalse();
expect($swap_pos)->toBeGreaterThan($guard_pos);
});
test('api_plugin_movedown does not assign $next_id to any id column before the empty guard', function () use ($libPluginsPath) {
$source = file_get_contents($libPluginsPath);
$fn_pos = strpos($source, 'function api_plugin_movedown(');
$guard_pos = strpos($source, 'if (!empty($next_id))', $fn_pos);
$pre_guard = substr($source, $fn_pos, $guard_pos - $fn_pos);
expect($pre_guard)->not->toContain('UPDATE plugin_config SET id');
});
// ---------------------------------------------------------------------------
// plugins_load_temp_table sql_mode save/restore
// ---------------------------------------------------------------------------
test('plugins_load_temp_table saves @@SESSION.sql_mode before adding NO_AUTO_VALUE_ON_ZERO', function () use ($pluginsPath) {
$source = file_get_contents($pluginsPath);
$fn_pos = strpos($source, 'function plugins_load_temp_table()');
$save_pos = strpos($source, '$orig_sql_mode = db_fetch_cell(\'SELECT @@SESSION.sql_mode\')', $fn_pos);
// Use the SET SESSION statement (not the comment above it) as the anchor so
// the order check is not confused by the explanatory comment that also mentions
// NO_AUTO_VALUE_ON_ZERO and appears before the save line.
$set_pos = strpos($source, 'db_execute("SET SESSION sql_mode = CONCAT_WS', $fn_pos);
expect($fn_pos)->not->toBeFalse();
expect($save_pos)->not->toBeFalse('$orig_sql_mode save not found in plugins_load_temp_table');
expect($set_pos)->not->toBeFalse('SET SESSION sql_mode = CONCAT_WS not found in plugins_load_temp_table');
// Save must come before the SET that adds NO_AUTO_VALUE_ON_ZERO.
expect($save_pos)->toBeLessThan($set_pos);
});
test('plugins_load_temp_table inserts into temp table while NO_AUTO_VALUE_ON_ZERO is active', function () use ($pluginsPath) {
$source = file_get_contents($pluginsPath);
$fn_pos = strpos($source, 'function plugins_load_temp_table()');
$nav_pos = strpos($source, 'NO_AUTO_VALUE_ON_ZERO', $fn_pos);
$insert_pos = strpos($source, 'INSERT INTO $table SELECT * FROM plugin_config', $fn_pos);
expect($nav_pos)->not->toBeFalse();
expect($insert_pos)->not->toBeFalse('bulk INSERT not found in plugins_load_temp_table');
// INSERT must come after the mode is set.
expect($insert_pos)->toBeGreaterThan($nav_pos);
});
test('plugins_load_temp_table restores original sql_mode after the bulk INSERT', function () use ($pluginsPath) {
$source = file_get_contents($pluginsPath);
$fn_pos = strpos($source, 'function plugins_load_temp_table()');
$insert_pos = strpos($source, 'INSERT INTO $table SELECT * FROM plugin_config', $fn_pos);
$restore_pos = strpos($source, 'SET SESSION sql_mode = ?', $fn_pos);
expect($insert_pos)->not->toBeFalse();
expect($restore_pos)->not->toBeFalse('sql_mode restore not found in plugins_load_temp_table');
// Restore must come after the INSERT.
expect($restore_pos)->toBeGreaterThan($insert_pos);
});
test('plugins_load_temp_table restore uses db_execute_prepared with $orig_sql_mode', function () use ($pluginsPath) {
$source = file_get_contents($pluginsPath);
$fn_pos = strpos($source, 'function plugins_load_temp_table()');
// The restore call must bind $orig_sql_mode, not a hardcoded string, so
// the session mode is returned to exactly what it was before the copy.
$restore_slice_start = strpos($source, 'SET SESSION sql_mode = ?', $fn_pos);
$restore_slice = substr($source, $restore_slice_start, 120);
expect($restore_slice)->toContain('$orig_sql_mode');
});