Gate pre-1.3 report shims on Cacti version to stop redeclare on 1.3.0 develop

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

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');
 }
 

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 = <<<CHILD
 <?php
 error_reporting(E_ALL);
 ini_set('display_errors', '1');
 
-// Simulate Cacti 1.3, where these are defined natively before the plugin loads.
-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 = []) {}
+$preamble
 
-require $argv[1];
+require \$argv[1];
 
 echo '<<<LOADED_OK>>>';
+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('<<<LOADED_OK>>>');
-		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('<<<LOADED_OK>>>');
+	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('<<<LOADED_OK>>>');
+	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('<<<LOADED_OK>>>');
+	expect($output)->not->toContain('redeclare');
+	expect($output)->toContain('HAS_RLAN=1;');
+	expect($output)->toContain('RLAN_FILE=functions-pre13.php;');
 });