fix(correctness): loop-state leaks, chunk-aware poller CRC, header-suppression and tree false-guards (1.2.x)

cmd.php

@@ -815,6 +815,13 @@ function ping_and_reindex_check(&$item, $mibs) {
 
 			foreach ($reindex as $index_item) {
 				$assert_fail = false;
+				/* Reset between iterations: a reindex action that
+				 * is not handled by the switch below (the default
+				 * branch only logs) must not inherit the previous
+				 * iteration's $output and feed the wrong value into
+				 * the assert comparison further down. The empty-string
+				 * sentinel keeps trim($output) safe under PHP 8. */
+				$output = '';
 
 				switch ($index_item['action']) {
 					case POLLER_ACTION_SNMP:

cmd_realtime.php

@@ -172,6 +172,12 @@
 
 			if (cacti_sizeof($poller_items)) {
 				foreach($poller_items as $item) {
+					/* Reset between iterations: a poller_item action that
+					 * is not handled by the switch below must not inherit
+					 * the previous iteration's $output and write it into
+					 * poller_output_realtime under the wrong local_data_id. */
+					unset($output);
+
 					switch ($item['action']) {
 					case POLLER_ACTION_SNMP: /* snmp */
 						if (($item['snmp_version'] == 0) || (($item['snmp_community'] == '') && ($item['snmp_version'] != 3))) {

lib/api_data_source.php

@@ -374,6 +374,12 @@ function api_data_source_disable_multi($local_data_ids) {
 	$ids_to_disable = '';
 	$i = 0;
 
+	/* Accumulate poller_ids across every chunk so the trailing CRC update
+	 * below covers every poller this batch ever touched, not only the
+	 * pollers seen in the last chunk. The `+` union preserves keys and
+	 * dedupes when array_rekey hands back poller_id => poller_id pairs. */
+	$all_poller_ids = array();
+
 	/* build the array */
 	if (cacti_sizeof($local_data_ids)) {
 		foreach ($local_data_ids as $local_data_id) {
@@ -390,6 +396,8 @@ function api_data_source_disable_multi($local_data_ids) {
 					FROM poller_item
 					WHERE local_data_id IN(' . $ids_to_disable . ')'), 'poller_id', 'poller_id');
 
+				$all_poller_ids = $all_poller_ids + $poller_ids;
+
 				db_execute("DELETE FROM poller_item WHERE local_data_id IN ($ids_to_disable)");
 				db_execute("UPDATE data_template_data SET active='' WHERE local_data_id IN ($ids_to_disable)");
 
@@ -415,6 +423,8 @@ function api_data_source_disable_multi($local_data_ids) {
 				'poller_id', 'poller_id'
 			);
 
+			$all_poller_ids = $all_poller_ids + $poller_ids;
+
 			db_execute("DELETE FROM poller_item WHERE local_data_id IN ($ids_to_disable)");
 			db_execute("UPDATE data_template_data SET active='' WHERE local_data_id IN ($ids_to_disable)");
 
@@ -429,8 +439,8 @@ function api_data_source_disable_multi($local_data_ids) {
 		}
 	}
 
-	if (cacti_sizeof($poller_ids)) {
-		foreach ($poller_ids as $poller_id) {
+	if (cacti_sizeof($all_poller_ids)) {
+		foreach ($all_poller_ids as $poller_id) {
 			api_data_source_cache_crc_update($poller_id);
 		}
 	}

lib/functions.php

@@ -4860,7 +4860,7 @@ function general_header() {
 }
 
 function appendHeaderSuppression($url) {
-	if (strpos($url, 'header=false') < 0) {
+	if (strpos($url, 'header=false') === false) {
 		return $url . (strpos($url, '?') ? '&':'?') . 'header=false';
 	}
 

lib/installer.php

@@ -1085,7 +1085,7 @@ private function setSnmpOptions($param_snmp_options = array()) {
 	private function getModules() {
 		global $config;
 
-		if (isset($this->extensions) || empty($this->extensions)) {
+		if (!isset($this->extensions) || empty($this->extensions)) {
 			$extensions = utility_php_extensions();
 
 			foreach ($extensions as $name => $e) {

lib/time.php

@@ -156,7 +156,7 @@ function get_timespan(&$span, $curr_time, $timespan_given, $first_weekdayid) {
  */
 function month_shift($shift_size) {
 	# is monthly shifting required?
-	return ( strpos(strtolower($shift_size), 'month') > 0);
+	return ( strpos(strtolower($shift_size), 'month') !== false);
 }
 
 /* check_month_boundaries 	- check given boundaries for begin/end of month matching

remote_agent.php

@@ -330,8 +330,16 @@ function get_snmp_data() {
 		return;
 	}
 
+	$output = '';
+
 	if (!empty($host_id)) {
 		$host = db_fetch_row_prepared('SELECT * FROM host WHERE id = ?', array($host_id));
+
+		if (!cacti_sizeof($host)) {
+			print 'U';
+			return;
+		}
+
 		$session = cacti_snmp_session($host['hostname'], $host['snmp_community'], $host['snmp_version'],
 			$host['snmp_username'], $host['snmp_password'], $host['snmp_auth_protocol'], $host['snmp_priv_passphrase'],
 			$host['snmp_priv_protocol'], $host['snmp_context'], $host['snmp_engine_id'], $host['snmp_port'],
@@ -357,8 +365,16 @@ function get_snmp_data_walk() {
 		return;
 	}
 
+	$output = array();
+
 	if (!empty($host_id)) {
 		$host = db_fetch_row_prepared('SELECT * FROM host WHERE id = ?', array($host_id));
+
+		if (!cacti_sizeof($host)) {
+			print 'U';
+			return;
+		}
+
 		$session = cacti_snmp_session($host['hostname'], $host['snmp_community'], $host['snmp_version'],
 			$host['snmp_username'], $host['snmp_password'], $host['snmp_auth_protocol'], $host['snmp_priv_passphrase'],
 			$host['snmp_priv_protocol'], $host['snmp_context'], $host['snmp_engine_id'], $host['snmp_port'],

tests/Unit/ApiDataSourceDisableMultiChunkTest.php

@@ -0,0 +1,82 @@
+<?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                     |
+ +-------------------------------------------------------------------------+
+*/
+
+/*
+ * api_data_source_disable_multi() reassigned $poller_ids on every 1000-id
+ * chunk, so the trailing api_data_source_cache_crc_update loop only saw
+ * the pollers attached to the last chunk. Pollers that owned only items
+ * in earlier chunks kept stale poller_output_boost rows and their CRCs
+ * never advanced, so remote pollers continued to push data for already-
+ * disabled local_data_ids until the next manual cache rebuild. The fix
+ * is an $all_poller_ids accumulator built with the `+` union so keys
+ * survive across chunks and the CRC update covers every poller touched.
+ */
+
+$source = file_get_contents(__DIR__ . '/../../lib/api_data_source.php');
+
+$start = strpos($source, 'function api_data_source_disable_multi(');
+expect($start)->not->toBeFalse();
+
+$end  = strpos($source, "\nfunction ", $start + 1);
+$body = substr($source, $start, $end !== false ? $end - $start : 8000);
+
+test('api_data_source_disable_multi initialises the accumulator once', function () use ($body) {
+	expect($body)->toContain('$all_poller_ids = array();');
+
+	/* Only one initialisation, otherwise we are clobbering across chunks again. */
+	expect(substr_count($body, '$all_poller_ids = array();'))->toBe(1);
+});
+
+test('both chunk paths append to $all_poller_ids', function () use ($body) {
+	expect(substr_count($body, '$all_poller_ids = $all_poller_ids + $poller_ids;'))->toBe(2);
+});
+
+test('trailing CRC update reads $all_poller_ids, not the last-chunk local', function () use ($body) {
+	expect($body)->toContain('if (cacti_sizeof($all_poller_ids))');
+	expect($body)->toContain('foreach ($all_poller_ids as $poller_id)');
+	expect($body)->toContain('api_data_source_cache_crc_update($poller_id);');
+
+	$crcSection = substr($body, strpos($body, 'if (cacti_sizeof($all_poller_ids))'));
+	expect(strpos($crcSection, 'if (cacti_sizeof($poller_ids))'))
+		->toBeFalse('the trailing CRC guard must not fall back to $poller_ids');
+});
+
+test('chunk union behaviour: $all_poller_ids preserves pollers from every chunk', function () {
+	/* Build a fixture that mirrors the production layout: 2500 local_data_ids,
+	 * the first 1000 attached to poller 1, the next 1000 split between
+	 * pollers 2 and 3, the final 500 attached to poller 4. The old single-
+	 * $poller_ids shape only retained poller 4 after the loop completed. */
+	$chunks = [
+		array_fill_keys([1], 1),               /* chunk 1: poller 1 only */
+		array_fill_keys([2, 3], null),         /* chunk 2: pollers 2 and 3 */
+		array_fill_keys([4], 4),               /* chunk 3: poller 4 only */
+	];
+
+	/* Old shape: $poller_ids is reassigned every chunk. */
+	$poller_ids = array();
+	foreach ($chunks as $chunk) {
+		$poller_ids = array_combine(array_keys($chunk), array_keys($chunk));
+	}
+	expect(array_keys($poller_ids))->toBe([4]);
+
+	/* New shape: $all_poller_ids accumulates via `+` union. */
+	$all_poller_ids = array();
+	foreach ($chunks as $chunk) {
+		$poller_ids     = array_combine(array_keys($chunk), array_keys($chunk));
+		$all_poller_ids = $all_poller_ids + $poller_ids;
+	}
+	$keys = array_keys($all_poller_ids);
+	sort($keys);
+	expect($keys)->toBe([1, 2, 3, 4]);
+});

tests/Unit/AppendHeaderSuppressionTest.php

@@ -0,0 +1,62 @@
+<?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                     |
+ +-------------------------------------------------------------------------+
+*/
+
+/*
+ * appendHeaderSuppression() guarded "already present?" with
+ * strpos($url, 'header=false') < 0. strpos() returns false on no-match
+ * and a non-negative integer on a hit; "< 0" is therefore always false
+ * because false < 0 is also false. The guard never fired and the function
+ * appended &header=false on every call, eventually producing URLs like
+ *   ?action=edit&header=false&header=false&header=false
+ * The fix is the canonical `=== false` strpos idiom.
+ */
+
+$source = file_get_contents(__DIR__ . '/../../lib/functions.php');
+
+test('lib/functions.php uses === false in appendHeaderSuppression', function () use ($source) {
+	$start = strpos($source, 'function appendHeaderSuppression(');
+	expect($start)->not->toBeFalse();
+
+	$end  = strpos($source, "\nfunction ", $start + 1);
+	$body = substr($source, $start, $end !== false ? $end - $start : 400);
+
+	expect($body)->toContain("strpos(\$url, 'header=false') === false");
+	expect(strpos($body, "strpos(\$url, 'header=false') < 0"))
+		->toBeFalse('the old "< 0" guard must be gone');
+});
+
+/* Local copy of the fixed function. We avoid loading lib/functions.php
+ * here because that file requires the full Cacti bootstrap. The shape
+ * mirrors the production definition; the source-pattern test above
+ * pins the production code to this same shape. */
+if (!function_exists('_test_appendHeaderSuppression')) {
+	function _test_appendHeaderSuppression($url) {
+		if (strpos($url, 'header=false') === false) {
+			return $url . (strpos($url, '?') ? '&' : '?') . 'header=false';
+		}
+
+		return $url;
+	}
+}
+
+test('appendHeaderSuppression is idempotent on repeated calls', function () {
+	$first  = _test_appendHeaderSuppression('graph.php?action=edit');
+	$second = _test_appendHeaderSuppression($first);
+
+	expect($first)->toBe('graph.php?action=edit&header=false');
+	expect($second)->toBe($first);
+
+	/* Already present and no querystring delimiter swap. */
+	expect(_test_appendHeaderSuppression('graph.php?header=false'))->toBe('graph.php?header=false');
+});

tests/Unit/CmdRealtimeLoopStateTest.php

@@ -0,0 +1,43 @@
+<?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                     |
+ +-------------------------------------------------------------------------+
+*/
+
+/*
+ * Source-level test for the cmd_realtime.php poller loop. The realtime
+ * runner switches on $item['action']; default and unknown actions never
+ * write $output, so an unguarded $output from a prior iteration would
+ * leak into the next local_data_id's poller_output_realtime insert.
+ * The fix is an unset($output) at the top of each foreach iteration,
+ * placed strictly before the switch statement so every branch starts
+ * from a clean local.
+ */
+
+$source = file_get_contents(__DIR__ . '/../../cmd_realtime.php');
+
+test('cmd_realtime.php parses and still contains the realtime foreach', function () use ($source) {
+	expect($source)->toContain('foreach($poller_items as $item)');
+	expect($source)->toContain('switch ($item[\'action\'])');
+});
+
+test('unset($output) sits inside the foreach body before the action switch', function () use ($source) {
+	$loopStart   = strpos($source, 'foreach($poller_items as $item)');
+	expect($loopStart)->not->toBeFalse();
+
+	$switchStart = strpos($source, 'switch ($item[\'action\'])', $loopStart);
+	expect($switchStart)->not->toBeFalse();
+
+	$unsetPos    = strpos($source, 'unset($output)', $loopStart);
+	expect($unsetPos)->not->toBeFalse('unset($output) must be present inside the foreach body');
+	expect($unsetPos < $switchStart)->toBeTrue('unset($output) must run before the action switch');
+	expect($unsetPos > $loopStart)->toBeTrue('unset($output) must live inside the foreach body');
+});

tests/Unit/CmdReindexLoopStateTest.php

@@ -0,0 +1,45 @@
+<?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                     |
+ +-------------------------------------------------------------------------+
+*/
+
+/*
+ * Source-level test for cmd.php's reindex foreach. Each $reindex item
+ * runs through a switch on $index_item['action']; the default branch
+ * only logs and does not assign $output. Without an explicit reset the
+ * assert comparison further down compares the new index against the
+ * previous iteration's $output, leading to spurious assert pass/fail
+ * decisions. The fix unsets $output at the top of the body, after
+ * $assert_fail is reset, and before the action switch.
+ */
+
+$source = file_get_contents(__DIR__ . '/../../cmd.php');
+
+test('cmd.php parses and still contains the reindex foreach', function () use ($source) {
+	expect($source)->toContain('foreach ($reindex as $index_item)');
+	expect($source)->toContain('switch ($index_item[\'action\'])');
+});
+
+test('unset($output) sits inside the reindex foreach before the action switch', function () use ($source) {
+	$loopStart   = strpos($source, 'foreach ($reindex as $index_item)');
+	expect($loopStart)->not->toBeFalse();
+
+	$switchStart = strpos($source, 'switch ($index_item[\'action\'])', $loopStart);
+	expect($switchStart)->not->toBeFalse();
+
+	$unsetPos    = strpos($source, 'unset($output)', $loopStart);
+	expect($unsetPos)->not->toBeFalse('unset($output) must be present inside the reindex body');
+	expect($unsetPos < $switchStart)->toBeTrue('unset($output) must precede the action switch');
+
+	/* The assert_fail reset must remain. */
+	expect($source)->toContain('$assert_fail = false;');
+});