fix: PSR, PHPDoc, Prepared Statements, More Timespan by TheWitness · Pull Request #312 · Cacti/plugin_syslog

TheWitness · 2026-04-10T02:57:44Z

PSR including short array syntax
PHPDoc Block Repairs
PHPCSFixit Using Cacti Ruleset
Convert to prepared statements where possible
More Timespan Changes

* PSR including short array syntax * PHPDoc Block Repairs * PHPCSFixit Using Cacti Ruleset * Convert to prepared statements where possible * More Timespan Changes

Copilot

Pull request overview

This PR modernizes parts of the Syslog plugin codebase to better align with Cacti/PHP style expectations and to improve query safety, while updating regression tests to enforce the refactors.

Changes:

PSR/style cleanups (short array syntax, comment style, whitespace, minor refactors).
Expanded prepared-statement usage in several UI and backend code paths.
Regression test updates to reflect new/refactored behaviors and patterns.

Reviewed changes

Copilot reviewed 24 out of 24 changed files in this pull request and generated 11 comments.

Show a summary per file

File	Description
tests/regression/issue278_command_execution_refactor_test.php	Style-only test cleanups (comment style, short arrays).
tests/regression/issue277_import_payload_loader_test.php	Short array syntax + minor formatting.
tests/regression/issue276_bulk_action_dispatch_helper_test.php	Short array syntax + minor formatting.
tests/regression/issue269_import_text_trim_check_test.php	Formatting tweaks + `print` usage and spacing.
tests/regression/issue269_import_text_branch_logic_test.php	Formatting tweaks + `print` usage.
tests/regression/issue258_replication_create_sql_test.php	Short array syntax + formatting consistency in stubs.
tests/regression/issue254_partition_table_locking_test.php	Test formatting; allowlist assertions (needs regex updates for short arrays).
tests/regression/issue253_alert_sql_placeholder_test.php	Short array syntax + string quoting consistency.
template/index.php	Removes trailing whitespace line.
syslog_reports.php	Prepared-statement migration in list/filter paths; parameterized search; export path touched.
syslog_removal.php	Prepared-statement migration in list/filter and newedit record lookup; various formatting updates.
syslog_process.php	Uses `__DIR__`, minor formatting and array destructuring cleanup.
syslog_counter.php	Minor formatting and brace normalization.
syslog_batch_transfer.php	CLI arg parsing cleanup, formatting, and consistent output calls.
syslog_alerts.php	Prepared-statement migration in list/filter and newedit record lookup; formatting updates.
setup.php	`__DIR__` adoption, short arrays, replication helper refactors, installer/uninstaller UI formatting.
locales/po/index.php	Removes trailing whitespace line.
locales/LC_MESSAGES/index.php	Removes trailing whitespace line.
locales/index.php	Removes trailing whitespace line.
index.php	Removes trailing whitespace line.
images/index.php	Removes trailing whitespace line.
database.php	PHPDoc normalization/updates; wrapper defaults normalized.

Copilot · 2026-04-10T03:03:03Z

syslog_reports.php

 	}

-	$reports   = syslog_get_report_records($sql_where, $rows);
+	$reports   = syslog_get_report_records($sql_where, $sql_params $rows);


Syntax error: call to syslog_get_report_records() is missing a comma between $sql_params and $rows, which will break page rendering/execution.

Suggested change

$reports = syslog_get_report_records($sql_where, $sql_params $rows);

$reports = syslog_get_report_records($sql_where, $sql_params, $rows);

Copilot · 2026-04-10T03:03:04Z

syslog_reports.php

+
 			foreach ($selected_items as $id) {
 				if ($id > 0) {
 					$data = db_fetch_row_prepared('SELECT *


report_export() loads report rows using db_fetch_row_prepared() against syslog_reports. If the plugin is configured to use a dedicated syslog database, this will query the wrong connection. Use the syslog_db_* wrapper for syslog tables here to preserve dual-database support.

Suggested change

$data = db_fetch_row_prepared('SELECT *

$data = syslog_db_fetch_row_prepared('SELECT *

Copilot · 2026-04-10T03:03:04Z

syslog_removal.php

+		$sql_where    = 'WHERE seq = ?';
+		$sql_params[] = get_request_var('id');
+
+		if (isset_request_var('date')) {
+			$sql_where   .= ' AND logtime = ?';
+			$sql_params[] = get_request_var('date');
+		}


$sql_params is appended to without being initialized in this branch, which can raise an "Undefined variable $sql_params" warning. Initialize $sql_params = [] before using $sql_params[].

Copilot · 2026-04-10T03:03:04Z

syslog_alerts.php

+		$sql_where    = 'WHERE seq = ?';
+		$sql_params[] = get_request_var('id');
+
+		if (isset_request_var('date')) {
+			$sql_where   .= ' AND logtime = ?';
+			$sql_params[] = get_request_var('date');
+		}


$sql_params is appended to without being initialized in this branch, which can raise an "Undefined variable $sql_params" warning. Initialize $sql_params = [] before using $sql_params[].

Copilot · 2026-04-10T03:03:04Z

syslog_alerts.php


-	/* get the username */
+	// get the username
 	$username = db_fetch_cell('SELECT username FROM user_auth WHERE id=' . $_SESSION['sess_user_id']);


The alert save path builds the user lookup SQL by concatenating $_SESSION['sess_user_id'] into the query string. For consistency with the other updated files and to avoid relying on implicit trust in session values, switch this to a prepared query (db_fetch_cell_prepared) with an id parameter.

Suggested change

$username = db_fetch_cell('SELECT username FROM user_auth WHERE id=' . $_SESSION['sess_user_id']);

$username = db_fetch_cell_prepared('SELECT username FROM user_auth WHERE id = ?', [$_SESSION['sess_user_id']]);

Copilot · 2026-04-10T03:03:05Z

syslog_alerts.php

 function api_syslog_alert_remove($id) {
 	global $syslogdb_default;
-	syslog_db_execute("DELETE FROM `" . $syslogdb_default . "`.`syslog_alert` WHERE id='" . $id . "'");
+	syslog_db_execute('DELETE FROM `' . $syslogdb_default . "`.`syslog_alert` WHERE id='" . $id . "'");
 }

 function api_syslog_alert_disable($id) {
 	global $syslogdb_default;
-	syslog_db_execute("UPDATE `" . $syslogdb_default . "`.`syslog_alert` SET enabled='' WHERE id='" . $id . "'");
+	syslog_db_execute('UPDATE `' . $syslogdb_default . "`.`syslog_alert` SET enabled='' WHERE id='" . $id . "'");
 }


These helper actions build syslog table write queries via string concatenation (e.g., embedding $id directly). To keep the prepared-statement migration complete and avoid injection risks if $id is ever not strictly numeric, use syslog_db_execute_prepared() with an id placeholder here (and similarly in the disable/enable helpers).

Copilot · 2026-04-10T03:03:05Z

setup.php

@@ -978,29 +990,29 @@ function syslog_install_advisor($syslog_exists) {

 	print "<table align='center' width='80%'><tr><td>";
 	html_start_box(__('Syslog %s Advisor', $type, 'syslog') . '<', '100%', '', '3', 'center', '');


html_start_box() title concatenates an extra '<' character onto the translated string, which will render a stray character in the UI. Remove the trailing " . '<'" so the header displays correctly.

Suggested change

html_start_box(__('Syslog %s Advisor', $type, 'syslog') . '<', '100%', '', '3', 'center', '');

html_start_box(__('Syslog %s Advisor', $type, 'syslog'), '100%', '', '3', 'center', '');

Copilot · 2026-04-10T03:03:05Z

database.php

 /**
 * syslog_db_fetch_cell - run a 'select' sql query and return the first column of the
- *   first row found
+ * first row found
 *
- * @param  $sql - the sql query to execute
- * @param  $log - whether to log error messages, defaults to true
- * @param  $col_name - use this column name instead of the first one
+ * @param string $sql      The sql query to execute
+ * @param string $col_name Use this column name instead of the first one
+ * @param bool   $log      Whether to log error messages, defaults to true
 *
- * @return (bool) the output of the sql query as a single variable
+ * @return bool the output of the sql query as a single variable
 */
-function syslog_db_fetch_cell($sql, $col_name = '', $log = TRUE) {
+function syslog_db_fetch_cell($sql, $col_name = '', $log = true) {
 	global $syslog_cnn;
+
 	return db_fetch_cell($sql, $col_name, $log, $syslog_cnn);
 }

 /**
 * syslog_db_fetch_cell_prepared - run a 'select' sql query and return the first column of the
- *   first row found
+ * first row found
 *
- * @param  $sql - the sql query to execute
- * @param  $params - an array of parameters
- * @param  $col_name - use this column name instead of the first one
- * @param  $log - whether to log error messages, defaults to true
+ * @param string $sql      The sql query to execute
+ * @param array  $params   An array of parameters
+ * @param string $col_name Use this column name instead of the first one
+ * @param bool   $log      Whether to log error messages, defaults to true
 *
- * @return (bool) the output of the sql query as a single variable
+ * @return bool The output of the sql query as a single variable
 */
-function syslog_db_fetch_cell_prepared($sql, $params = array(), $col_name = '', $log = TRUE) {
+function syslog_db_fetch_cell_prepared($sql, $params = [], $col_name = '', $log = true) {
 	global $syslog_cnn;
+
 	return db_fetch_cell_prepared($sql, $params, $col_name, $log, $syslog_cnn);
 }


PHPDoc return types for syslog_db_fetch_cell() / syslog_db_fetch_cell_prepared() are documented as bool, but these wrappers return the first-cell value (string/int/etc) or false. Update the @return types to reflect the actual mixed return value to avoid misleading callers and static analysis noise.

Copilot · 2026-04-10T03:03:06Z

tests/regression/issue254_partition_table_locking_test.php


-/* The allowlist must be exactly the two known partition tables, nothing else. */
+// The allowlist must be exactly the two known partition tables, nothing else.
 if (!preg_match("/in_array\(\s*\\\$table\s*,\s*array\s*\(\s*'syslog'\s*,\s*'syslog_removed'\s*\)\s*,\s*true\s*\)/", $functions)) {


This regex hard-codes in_array($table, array(...)), but functions.php now uses short array syntax (in_array($table, ['syslog', 'syslog_removed'], true)). Update the pattern to accept [...] (and optionally still accept array(...)) so the regression test matches the refactored implementation.

Suggested change

if (!preg_match("/in_array$\s*\\\$table\s*,\s*array\s*\(\s*'syslog'\s*,\s*'syslog_removed'\s*$\s*,\s*true\s*\)/", $functions)) {

if (!preg_match("/in_array$\s*\\\$table\s*,\s*(?:array\s*\(\s*'syslog'\s*,\s*'syslog_removed'\s*$|\[\s*'syslog'\s*,\s*'syslog_removed'\s*\])\s*,\s*true\s*\)/", $functions)) {

Copilot · 2026-04-10T03:03:06Z

tests/regression/issue254_partition_table_locking_test.php

-/* syslog_partition_table_allowed must accept exactly these two values. */
-$allowed_tables = array('syslog', 'syslog_removed');
 foreach ($allowed_tables as $t) {
 	if (!preg_match("/in_array\(\s*\\\$table\s*,\s*array\s*\(\s*'syslog'\s*,\s*'syslog_removed'\s*\)/", $functions)) {


This allowlist assertion still matches only the legacy array(...) syntax. Since functions.php uses short arrays now, adjust the regex to match [...] (and optionally array(...)) to prevent false failures.

Suggested change

if (!preg_match("/in_array$\s*\\\$table\s*,\s*array\s*\(\s*'syslog'\s*,\s*'syslog_removed'\s*$/", $functions)) {

if (!preg_match("/in_array$\s*\\\$table\s*,\s*(?:\[\s*'syslog'\s*,\s*'syslog_removed'\s*\]|array\s*\(\s*'syslog'\s*,\s*'syslog_removed'\s*$)/", $functions)) {

fix: PSR, PHPDoc, Prepared Statements, More Timespan

8f03e2b

* PSR including short array syntax * PHPDoc Block Repairs * PHPCSFixit Using Cacti Ruleset * Convert to prepared statements where possible * More Timespan Changes

TheWitness requested review from bmfmancini, browniebraun, Copilot and xmacan April 10, 2026 02:57

Copilot started reviewing on behalf of TheWitness April 10, 2026 02:58 View session

Copilot AI reviewed Apr 10, 2026

View reviewed changes

fix: Syntax error

53b6476

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: PSR, PHPDoc, Prepared Statements, More Timespan#312

fix: PSR, PHPDoc, Prepared Statements, More Timespan#312
TheWitness wants to merge 2 commits intodevelopfrom
psr-docblocks

TheWitness commented Apr 10, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 10, 2026

Uh oh!

Copilot AI Apr 10, 2026

Uh oh!

Copilot AI Apr 10, 2026

Uh oh!

Copilot AI Apr 10, 2026

Uh oh!

Copilot AI Apr 10, 2026

Uh oh!

Copilot AI Apr 10, 2026

Uh oh!

Copilot AI Apr 10, 2026

Uh oh!

Copilot AI Apr 10, 2026

Uh oh!

Copilot AI Apr 10, 2026

Uh oh!

Copilot AI Apr 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	$reports = syslog_get_report_records($sql_where, $sql_params $rows);
	$reports = syslog_get_report_records($sql_where, $sql_params, $rows);

	$data = db_fetch_row_prepared('SELECT *
	$data = syslog_db_fetch_row_prepared('SELECT *

	$username = db_fetch_cell('SELECT username FROM user_auth WHERE id=' . $_SESSION['sess_user_id']);
	$username = db_fetch_cell_prepared('SELECT username FROM user_auth WHERE id = ?', [$_SESSION['sess_user_id']]);

		@@ -978,29 +990,29 @@ function syslog_install_advisor($syslog_exists) {

		print "<table align='center' width='80%'><tr><td>";
		html_start_box(__('Syslog %s Advisor', $type, 'syslog') . '<', '100%', '', '3', 'center', '');

	if (!preg_match("/in_array\(\s\\\$table\s,\sarray\s\(\s'syslog'\s,\s'syslog_removed'\s\)\s,\strue\s*\)/", $functions)) {
	if (!preg_match("/in_array\(\s\\\$table\s,\s(?:array\s\(\s'syslog'\s,\s'syslog_removed'\s\)\|\[\s'syslog'\s,\s'syslog_removed'\s\])\s,\strue\s*\)/", $functions)) {

Conversation

TheWitness commented Apr 10, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants