diff --git a/.github/workflows/plugin-ci-workflow.yml b/.github/workflows/plugin-ci-workflow.yml
index 86237dcf..480010b9 100644
--- a/.github/workflows/plugin-ci-workflow.yml
+++ b/.github/workflows/plugin-ci-workflow.yml
@@ -40,6 +40,7 @@ jobs:
uses: actions/checkout@v4
with:
repository: Cacti/cacti
+ ref: release/1.2.31
path: cacti
- name: Checkout Syslog Plugin
@@ -57,11 +58,18 @@ jobs:
- name: Check PHP Syntax (Lint)
run: |
cd cacti/plugins/syslog
- if find . -name '*.php' -not -path './vendor/*' -exec php -l {} 2>&1 \; | grep -iv 'no syntax errors detected'; then
+ if find . -name '*.php' -not -path './vendor/*' -exec php -l {} \; 2>&1 | grep -iv 'no syntax errors detected'; then
echo "Syntax errors found!"
exit 1
fi
+ - name: Run Regression Tests
+ run: |
+ cd cacti/plugins/syslog
+ for test in tests/regression/*.php; do
+ php "$test"
+ done
+
- name: Run PHP CS Fixer (Dry Run)
run: |
cd cacti/plugins/syslog
@@ -122,6 +130,7 @@ jobs:
uses: actions/checkout@v4
with:
repository: Cacti/cacti
+ ref: release/1.2.31
path: cacti
- name: Checkout Syslog Plugin
@@ -143,7 +152,7 @@ jobs:
run: sudo apt-get update
- name: Install System Dependencies
- run: sudo apt-get install -y apache2 snmp snmpd rrdtool fping libapache2-mod-php${{ matrix.php }}
+ run: sudo apt-get install -y apache2 snmp snmpd rrdtool fping libapache2-mod-php
- name: Start SNMPD Agent and Test
run: |
@@ -163,16 +172,15 @@ jobs:
echo -e "[client]\nuser = root\npassword = cactiroot\nhost = 127.0.0.1\n" > ~/.my.cnf
- name: Initialize Cacti Database
- env:
- MYSQL_AUTH_USR: '--defaults-file=~/.my.cnf'
run: |
- mysql $MYSQL_AUTH_USR -e 'CREATE DATABASE IF NOT EXISTS cacti;'
- mysql $MYSQL_AUTH_USR -e "CREATE USER IF NOT EXISTS 'cactiuser'@'localhost' IDENTIFIED BY 'cactiuser';"
- mysql $MYSQL_AUTH_USR -e "GRANT ALL PRIVILEGES ON cacti.* TO 'cactiuser'@'localhost';"
- mysql $MYSQL_AUTH_USR -e "GRANT SELECT ON mysql.time_zone_name TO 'cactiuser'@'localhost';"
- mysql $MYSQL_AUTH_USR -e "FLUSH PRIVILEGES;"
- mysql $MYSQL_AUTH_USR cacti < ${{ github.workspace }}/cacti/cacti.sql
- mysql $MYSQL_AUTH_USR -e "INSERT INTO settings (name, value) VALUES ('path_php_binary', '/usr/bin/php')" cacti
+ MYSQL_AUTH_USR="--defaults-file=$HOME/.my.cnf"
+ mysql "$MYSQL_AUTH_USR" -e 'CREATE DATABASE IF NOT EXISTS cacti;'
+ mysql "$MYSQL_AUTH_USR" -e "CREATE USER IF NOT EXISTS 'cactiuser'@'localhost' IDENTIFIED BY 'cactiuser';"
+ mysql "$MYSQL_AUTH_USR" -e "GRANT ALL PRIVILEGES ON cacti.* TO 'cactiuser'@'localhost';"
+ mysql "$MYSQL_AUTH_USR" -e "GRANT SELECT ON mysql.time_zone_name TO 'cactiuser'@'localhost';"
+ mysql "$MYSQL_AUTH_USR" -e "FLUSH PRIVILEGES;"
+ mysql "$MYSQL_AUTH_USR" cacti < ${{ github.workspace }}/cacti/cacti.sql
+ mysql "$MYSQL_AUTH_USR" -e "INSERT INTO settings (name, value) VALUES ('path_php_binary', '/usr/bin/php')" cacti
- name: Validate composer files
run: |
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 7a22b427..8b4e5136 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -12,6 +12,7 @@
* issue#278: Extract alert command execution into shared helper in functions.php; command tokenization now uses preg_split (handles tabs and consecutive spaces); /bin/sh fallback for non-executable command templates removed (use absolute paths with execute bit set)
* issue#298: syslog poller: lock timeout, signal handler, and earlier partition rotation
* issue#300: Syslog table drop with no apparent reason
+* issue: Repair plugin integration CI, execute regression tests, and reject empty import files safely
* issue: Making changes to support Cacti 1.3
* issue: Don't use MyISAM for non-analytical tables
* issue: The install advisor for Syslog was broken in current Cacti releases
diff --git a/functions.php b/functions.php
index 1e54c132..a6750e90 100644
--- a/functions.php
+++ b/functions.php
@@ -145,9 +145,11 @@ function syslog_sendemail($to, $from, $subject, $message, $smsmessage = '') {
}
function syslog_get_import_xml_payload($redirect_url) {
- if (trim(get_nfilter_request_var('import_text')) != '') {
+ $import_text = (string) get_nfilter_request_var('import_text');
+
+ if (trim($import_text) !== '') {
// textbox input
- return get_nfilter_request_var('import_text');
+ return $import_text;
}
if (isset($_FILES['import_file']['tmp_name']) &&
@@ -166,19 +168,10 @@ function syslog_get_import_xml_payload($redirect_url) {
exit;
}
- $fp = fopen($tmp_name, 'rb');
-
- if ($fp === false) {
- cacti_log('SYSLOG ERROR: Failed to open uploaded import file', false, 'SYSTEM');
- header('Location: ' . $redirect_url);
- exit;
- }
-
- $xml_data = fread($fp, filesize($tmp_name));
- fclose($fp);
+ $xml_data = syslog_read_import_file($tmp_name);
if ($xml_data === false) {
- cacti_log('SYSLOG ERROR: Failed to read uploaded import file', false, 'SYSTEM');
+ cacti_log('SYSLOG ERROR: Uploaded import file is empty or unreadable', false, 'SYSTEM');
header('Location: ' . $redirect_url);
exit;
}
@@ -190,6 +183,26 @@ function syslog_get_import_xml_payload($redirect_url) {
exit;
}
+function syslog_read_import_file(string $filename): string|false {
+ $size = filesize($filename);
+
+ if ($size === false || $size < 1) {
+ return false;
+ }
+
+ $handle = fopen($filename, 'rb');
+
+ if ($handle === false) {
+ return false;
+ }
+
+ try {
+ return fread($handle, $size);
+ } finally {
+ fclose($handle);
+ }
+}
+
function syslog_is_partitioned() {
global $syslogdb_default;
diff --git a/tests/regression/issue253_alert_sql_placeholder_test.php b/tests/regression/issue253_alert_sql_placeholder_test.php
index ae31c227..0ac4737d 100644
--- a/tests/regression/issue253_alert_sql_placeholder_test.php
+++ b/tests/regression/issue253_alert_sql_placeholder_test.php
@@ -30,14 +30,14 @@ function issue253_assert($condition, $message) {
$hostSql = syslog_get_alert_sql($hostAlert, 55);
$progSql = syslog_get_alert_sql($programAlert, 66);
-issue253_assert(strpos($hostSql['sql'], 'AND `status` = ?') !== false, 'Host alert SQL must keep status as a placeholder.');
-issue253_assert(strpos($hostSql['sql'], '?55') === false, 'Host alert SQL must not concatenate uniqueID into SQL text.');
+issue253_assert(strpos($hostSql['sql'], 'AND `status` = 1') !== false, 'Host alert SQL must select processed incoming rows.');
+issue253_assert(strpos($hostSql['sql'], 'AND `seq` <= ?') !== false, 'Host alert SQL must bound rows by sequence.');
issue253_assert(count($hostSql['params']) === 2, 'Host alert SQL must pass two prepared parameters.');
-issue253_assert($hostSql['params'][1] === 55, 'Host alert status param should be the uniqueID.');
+issue253_assert($hostSql['params'][1] === 55, 'Host alert sequence parameter should be the processing boundary.');
-issue253_assert(strpos($progSql['sql'], 'AND `status` = ?') !== false, 'Program alert SQL must keep status as a placeholder.');
-issue253_assert(strpos($progSql['sql'], '?66') === false, 'Program alert SQL must not concatenate uniqueID into SQL text.');
+issue253_assert(strpos($progSql['sql'], 'AND `status` = 1') !== false, 'Program alert SQL must select processed incoming rows.');
+issue253_assert(strpos($progSql['sql'], 'AND `seq` <= ?') !== false, 'Program alert SQL must bound rows by sequence.');
issue253_assert(count($progSql['params']) === 2, 'Program alert SQL must pass two prepared parameters.');
-issue253_assert($progSql['params'][1] === 66, 'Program alert status param should be the uniqueID.');
+issue253_assert($progSql['params'][1] === 66, 'Program alert sequence parameter should be the processing boundary.');
print "issue253_alert_sql_placeholder_test passed\n";
diff --git a/tests/regression/issue269_import_text_branch_logic_test.php b/tests/regression/issue269_import_text_branch_logic_test.php
index 947bc3c6..3ed9e1db 100644
--- a/tests/regression/issue269_import_text_branch_logic_test.php
+++ b/tests/regression/issue269_import_text_branch_logic_test.php
@@ -3,11 +3,8 @@
/*
* Regression test for issue #269 -- branch-logic invariants.
*
- * These assertions verify the structural properties that make whitespace-only
- * input fall through to the file-upload branch instead of the textbox branch,
- * and that a non-empty payload is assigned to $xml_data without further
- * modification. Pure source inspection: the functions themselves cannot be
- * called in isolation because they depend on the Cacti runtime.
+ * Import parsing now lives in syslog_get_import_xml_payload(). Each route must
+ * delegate to that helper instead of maintaining a divergent text/file branch.
*/
$root = dirname(__DIR__, 2);
@@ -25,42 +22,14 @@
exit(1);
}
- /*
- * 1. The request variable must be captured into a local first.
- * Whitespace-only input falls through only because trim() is applied
- * to the local; if the assignment were missing the condition would
- * be wrong.
- */
- if (!preg_match('/\$import_text\s*=\s*get_nfilter_request_var\s*\(\s*\'import_text\'\s*\)/', $content)) {
- fwrite(STDERR, "$func: \$import_text assignment via get_nfilter_request_var missing in $target\n");
+ if (substr_count($content, 'syslog_get_import_xml_payload(') !== 1) {
+ fwrite(STDERR, "$func: import route must call the shared payload helper exactly once in $target\n");
exit(1);
}
- /*
- * 2. The branch condition must trim the local variable, not the raw
- * request call. This is what makes whitespace-only values fall
- * through to the file-upload branch.
- */
- if (!preg_match('/trim\s*\(\s*\$import_text\s*\)\s*!=\s*\'\'/', $content)) {
- fwrite(STDERR, "$func: trim(\$import_text) != '' condition missing in $target\n");
- exit(1);
- }
-
- /*
- * 3. Inside the textbox branch, $xml_data must be assigned the
- * untrimmed local. A non-empty payload is preserved as-is.
- */
- if (!preg_match('/\$xml_data\s*=\s*\$import_text\s*;/', $content)) {
- fwrite(STDERR, "$func: \$xml_data = \$import_text assignment missing in $target\n");
- exit(1);
- }
-
- /*
- * 4. The file-upload branch must still exist (elseif on $_FILES).
- * Ensures the fallback path was not accidentally removed.
- */
- if (!preg_match('/elseif\s*\(\s*\(\s*\$_FILES\s*\[/', $content)) {
- fwrite(STDERR, "$func: \$_FILES elseif branch missing in $target\n");
+ if (str_contains($content, "get_nfilter_request_var('import_text')") ||
+ str_contains($content, "\$_FILES['import_file']")) {
+ fwrite(STDERR, "$func: route duplicates shared import payload parsing in $target\n");
exit(1);
}
}
diff --git a/tests/regression/issue269_import_text_trim_check_test.php b/tests/regression/issue269_import_text_trim_check_test.php
index 1cd71993..f8e72460 100644
--- a/tests/regression/issue269_import_text_trim_check_test.php
+++ b/tests/regression/issue269_import_text_trim_check_test.php
@@ -1,6 +1,7 @@
$readPosition) {
+ fwrite(STDERR, "Shared import payload loader must validate an upload before opening it.\n");
+ exit(1);
+}
+
+require_once $root . '/functions.php';
+
+$emptyFixture = tempnam(sys_get_temp_dir(), 'syslog-empty-import-');
+$payloadFixture = tempnam(sys_get_temp_dir(), 'syslog-import-');
+
+if ($emptyFixture === false || $payloadFixture === false ||
+ file_put_contents($payloadFixture, 'fixture') === false) {
+ fwrite(STDERR, "Unable to create import payload fixtures.\n");
+ exit(1);
+}
+
+try {
+ if (syslog_read_import_file($emptyFixture) !== false) {
+ fwrite(STDERR, "A zero-byte import must fail without calling fread() with a zero length.\n");
+ exit(1);
+ }
+
+ if (syslog_read_import_file($payloadFixture) !== 'fixture') {
+ fwrite(STDERR, "A non-empty import payload must round trip without data loss.\n");
+ exit(1);
+ }
+} finally {
+ unlink($emptyFixture);
+ unlink($payloadFixture);
+}
+
$targets = [
$root . '/syslog_alerts.php',
$root . '/syslog_reports.php',