diff --git a/CHANGELOG.md b/CHANGELOG.md index 5ba19b6..499ae45 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ --- develop --- -* bug: Guard the pre-1.3 report compatibility shims (reports_run/reports_queue/reports_log_and_notify) behind a function_exists() check instead of a CACTI_VERSION string comparison, so a develop build that reports a version comparing < 1.3 no longer fatally redeclares those now-native functions +* bug: Load the pre-1.3 report compatibility shims (reports_run/reports_queue/reports_log_and_notify) only when the running Cacti is older than 1.3, decided with cacti_version_compare(CACTI_VERSION, '1.3', ...) rather than a function_exists() probe; because the plugin's functions.php is included during plugin init before core loads lib/reports.php, the function_exists() check ran while the now-native helpers were still absent, declared the shims, and core then fatally redeclared reports_log_and_notify() on 1.3.0 develop builds (falls back to function_exists() only when the Cacti version helpers are unavailable, e.g. the unit tests) * security: Move the plugin's remaining inline event handlers to CSP-safe bindings so the pages no longer trip Cacti's Content-Security-Policy script-src-attr directive: confirmation Cancel buttons use the `cactiReturnTo` class, the database/filter selects are bound via change() in their ready blocks, and the main filter's sort/report selects and time-shift icons are bound in flowview_display_filter's ready block * dev: Replace the translation-template CI check that regenerated locales/po/cacti.pot and compared it with a diff-based gate (tests/bin/check-i18n-pot.php) requiring cacti.pot to be updated only when a pull request adds, removes, or modifies an i18n function call * issue: Prevent flow-capture from fatally erroring (TypeError in spl_object_hash) when flowview_db_check_reconnect() held a null shared-database handle; normalize a non-object connection to false so db_check_reconnect() checks and reconnects the default connection instead diff --git a/includes/functions.php b/includes/functions.php index 55a1850..bbe9f3f 100644 --- a/includes/functions.php +++ b/includes/functions.php @@ -23,11 +23,20 @@ +-------------------------------------------------------------------------+ */ -// reports_run()/reports_queue()/reports_log_and_notify() are native in Cacti -// 1.3+. Load the compat shims only when they are actually missing: a develop -// build can report a version that compares < 1.3 while already defining them -// natively, and unconditionally including the shims would fatally redeclare them. -if (!function_exists('reports_run')) { +// reports_run()/reports_queue()/reports_log_and_notify() became native in +// Cacti 1.3. Whether core will declare them is fixed by the Cacti version at +// bootstrap, so gate the pre-1.3 shims on that rather than on function_exists(): +// this file can be included during plugin init before core loads lib/reports.php, +// so a function_exists() probe misses the now-native functions, declares the +// shims, and core then fatally redeclares them. cacti_version_compare() treats a +// 1.3.0 develop build (CACTI_VERSION '1.3.0') as >= 1.3. Fall back to the +// function_exists() check only when the version helpers are unavailable (e.g. +// including this file outside a full Cacti bootstrap, as the unit tests do). +if ( + (function_exists('cacti_version_compare') && defined('CACTI_VERSION')) + ? cacti_version_compare(CACTI_VERSION, '1.3', '<') + : !function_exists('reports_run') +) { require_once(__DIR__ . '/functions-pre13.php'); } diff --git a/tests/Unit/ReportsShimGuardTest.php b/tests/Unit/ReportsShimGuardTest.php index a24d538..d91f8df 100644 --- a/tests/Unit/ReportsShimGuardTest.php +++ b/tests/Unit/ReportsShimGuardTest.php @@ -6,31 +6,45 @@ */ /* - * Regression guard for includes/functions.php. On Cacti 1.3 the report helpers - * are native, so loading the plugin file must not redeclare them. CI checks out - * Cacti 1.2.x (helpers absent) and the bootstrap has already required - * functions.php, so this exercises the develop-only "natives present" path in a - * clean child process where the three functions are defined up front. + * Regression guard for the pre-1.3 report-shim gate in includes/functions.php. + * + * reports_run()/reports_queue()/reports_log_and_notify() became native in Cacti + * 1.3 (declared unguarded in core lib/reports.php). functions.php is included + * during plugin init BEFORE core loads lib/reports.php, so a function_exists() + * probe would miss the now-native helpers, load the shims, and core would then + * fatally redeclare them. The gate therefore decides from the Cacti version + * (cacti_version_compare(CACTI_VERSION, '1.3', '<')) and only falls back to + * function_exists() when the version helpers are unavailable. + * + * Each case loads the real functions.php in a clean child process with a + * different preamble, then reports whether the shim was loaded (and from where) + * via markers so the three branches of the gate are covered independently. */ -it('loads functions.php without redeclaring the native report helpers', function () { +/** + * Runs includes/functions.php in a child process after $preamble and returns + * its stdout, with markers describing the resulting report-helper state. + */ +function flowview_run_shim_child(string $preamble): string { $functions = dirname(__DIR__, 2) . '/includes/functions.php'; expect(is_readable($functions))->toBeTrue(); - $script = <<<'CHILD' + $script = <<>>'; +echo 'HAS_RLAN=' . (function_exists('reports_log_and_notify') ? '1' : '0') . ';'; +if (function_exists('reports_log_and_notify')) { + \$rf = new ReflectionFunction('reports_log_and_notify'); + echo 'RLAN_FILE=' . basename((string) \$rf->getFileName()) . ';'; +} CHILD; $tmp = tempnam(sys_get_temp_dir(), 'pre13_'); @@ -40,11 +54,61 @@ function reports_log_and_notify($id, $start_time, $report_type, $source, $source file_put_contents($tmp, $script); $command = escapeshellarg(PHP_BINARY) . ' ' . escapeshellarg($tmp) . ' ' . escapeshellarg($functions); - $output = (string) shell_exec($command); - expect($output)->toContain('<<>>'); - expect($output)->not->toContain('redeclare'); + return (string) shell_exec($command); } finally { @unlink($tmp); } +} + +it('does not redeclare natives when the Cacti version helpers are unavailable (fallback path)', function () { + // No CACTI_VERSION / cacti_version_compare(): the gate falls back to + // function_exists(). The helpers are defined up front (as on native Cacti), + // so the shim must not be loaded and nothing may be redeclared. + $preamble = <<<'PRE' +function reports_run($id) { return true; } +function reports_queue($name, $request_type, $source, $source_id, $command, $notification) {} +function reports_log_and_notify($id, $start_time, $report_type, $source, $source_id, $subject, &$raw_data, &$oput_raw, &$oput_html, &$oput_text, $attachments = [], $headers = []) {} +PRE; + + $output = flowview_run_shim_child($preamble); + + expect($output)->toContain('<<>>'); + expect($output)->not->toContain('redeclare'); + // The surviving declaration is the pre-defined native one, not the shim file. + expect($output)->not->toContain('RLAN_FILE=functions-pre13.php'); +}); + +it('keeps the shim unloaded on a 1.3.0 develop build even when the natives are not yet defined', function () { + // The version helper reports 1.3.0 (CACTI_VERSION resolves to '1.3.0' on a + // develop build) and the natives are absent, mirroring plugin init running + // before core loads lib/reports.php. The gate must skip the shim from the + // version alone, so reports_log_and_notify() must NOT exist afterwards. + $preamble = <<<'PRE' +define('CACTI_VERSION', '1.3.0'); +// Mirrors Cacti: a 1.3.0 develop build is treated as >= 1.3. +function cacti_version_compare($v1, $v2, $op = '>') { return version_compare($v1, $v2, $op); } +PRE; + + $output = flowview_run_shim_child($preamble); + + expect($output)->toContain('<<>>'); + expect($output)->not->toContain('redeclare'); + expect($output)->toContain('HAS_RLAN=0;'); +}); + +it('loads the shim on a genuine pre-1.3 release', function () { + // The version helper reports 1.2.29 (the plugin's declared compat floor) and + // the natives are absent, so the shim must load from functions-pre13.php. + $preamble = <<<'PRE' +define('CACTI_VERSION', '1.2.29'); +function cacti_version_compare($v1, $v2, $op = '>') { return version_compare($v1, $v2, $op); } +PRE; + + $output = flowview_run_shim_child($preamble); + + expect($output)->toContain('<<>>'); + expect($output)->not->toContain('redeclare'); + expect($output)->toContain('HAS_RLAN=1;'); + expect($output)->toContain('RLAN_FILE=functions-pre13.php;'); });