bugfix: scope partition maintenance to target table and lock create path by somethingwithproof · Pull Request #271 · Cacti/plugin_syslog

somethingwithproof · 2026-03-07T00:17:06Z

Summary

scope partition create/check/remove queries to the requested table instead of hardcoded syslog
add a table allowlist guard for partition maintenance helpers
serialize partition creation using GET_LOCK/RELEASE_LOCK to avoid concurrent duplicate partition creation
add regression test for table scoping and lock usage
update changelog with issue bug: partition existence check hardcodes table name 'syslog', breaks syslog_removed and allows race condition #254

Fixes #254

Validation

php -l functions.php
php -l tests/regression/issue254_partition_table_locking_test.php
php tests/regression/issue254_partition_table_locking_test.php

Copilot

Pull request overview

Fixes syslog partition maintenance so it correctly targets the requested table (e.g., syslog_removed), adds a guardrail for which tables are eligible for partition operations, and serializes partition creation to avoid concurrent “duplicate partition” errors.

Changes:

Scope information_schema.partitions checks/removals to the $table argument instead of hardcoding syslog.
Add syslog_partition_table_allowed() allowlist and use MySQL GET_LOCK/RELEASE_LOCK to serialize partition creation.
Add a regression test that verifies table scoping, lock usage, and the presence of the allowlist helper; update changelog for #254.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File	Description
functions.php	Scopes partition queries to the target table, adds allowlist guard, and adds locking around partition creation.
tests/regression/issue254_partition_table_locking_test.php	Adds a regression check ensuring table scoping and lock acquisition/release are present.
CHANGELOG.md	Records the fix for issue #254 under the develop section.

Copilot · 2026-03-07T00:20:44Z

functions.php

+		$exists = syslog_db_fetch_row("SELECT *
+			FROM `information_schema`.`partitions`
+			WHERE table_schema='" . $syslogdb_default . "'
+			AND partition_name='" . $cformat . "'
+			AND table_name='" . $table . "'
+			ORDER BY partition_ordinal_position");


The partition existence check query is built via string concatenation for table_schema, partition_name, and table_name. Since database.php provides prepared variants, use syslog_db_fetch_row_prepared() here to avoid quoting edge cases and reduce SQL injection risk (even with the allowlist).

Suggested change

$exists = syslog_db_fetch_row("SELECT *

FROM `information_schema`.`partitions`

WHERE table_schema='" . $syslogdb_default . "'

AND partition_name='" . $cformat . "'

AND table_name='" . $table . "'

ORDER BY partition_ordinal_position");

$exists = syslog_db_fetch_row_prepared(

"SELECT *

FROM `information_schema`.`partitions`

WHERE table_schema = ?

AND partition_name = ?

AND table_name = ?

ORDER BY partition_ordinal_position",

array($syslogdb_default, $cformat, $table)

);

Fixed — using syslog_db_fetch_row_prepared() now.

Copilot · 2026-03-07T00:20:44Z

functions.php

 	$number_of_partitions = syslog_db_fetch_assoc("SELECT *
 		FROM `information_schema`.`partitions`
-		WHERE table_schema='" . $syslogdb_default . "' AND table_name='syslog'
+		WHERE table_schema='" . $syslogdb_default . "' AND table_name='" . $table . "'
 		ORDER BY partition_ordinal_position");


This information_schema.partitions query interpolates $syslogdb_default and $table directly into SQL. Consider switching to syslog_db_fetch_assoc_prepared() with parameters for table_schema / table_name to avoid quoting issues and keep partition-maintenance queries consistently parameterized.

Copilot · 2026-03-07T00:20:44Z

functions.php

 	$last_part = syslog_db_fetch_cell("SELECT PARTITION_NAME
 		FROM `information_schema`.`partitions`
-		WHERE table_schema='" . $syslogdb_default . "' AND table_name='syslog'
+		WHERE table_schema='" . $syslogdb_default . "' AND table_name='" . $table . "'
 		ORDER BY partition_ordinal_position DESC
 		LIMIT 1,1;");


$syslogdb_default and $table are interpolated into this information_schema query. Using syslog_db_fetch_cell_prepared() with placeholders for table_schema / table_name would be safer and more robust if the configured schema name ever contains characters that require escaping.

Copilot · 2026-03-07T00:20:45Z

tests/regression/issue254_partition_table_locking_test.php

+	exit(1);
+}
+
+if (substr_count($functions, "table_name='\" . \$table . \"'") < 3) {


This regression test relies on exact source-code string matching (including specific quoting/whitespace), which is fragile and can fail on harmless refactors/formatting changes. Consider using a more tolerant pattern (e.g., regex allowing whitespace) or validating behavior via function calls against a test DB stub so the test asserts semantics rather than formatting.

Suggested change

if (substr_count($functions, "table_name='\" . \$table . \"'") < 3) {

$scopedTableCount = 0;

foreach (preg_split('/\R/', $functions) as $line) {

if (strpos($line, 'table_name') !== false && strpos($line, '$table') !== false) {

$scopedTableCount++;

}

}

if ($scopedTableCount < 3) {

Fixed — test now uses preg_match_all() patterns.

Fixes Cacti#254 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

Replace three string-interpolated syslog_db_fetch_* calls with their _prepared variants. The original queries concatenated $syslogdb_default, $cformat, and $table directly into SQL strings, which bypasses parameterization on the schema/table name values. Also update the regression test to match the prepared-statement form with preg_match_all patterns rather than the old string-interpolation literal checks. Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

Copilot AI review requested due to automatic review settings March 7, 2026 00:17

Copilot started reviewing on behalf of somethingwithproof March 7, 2026 00:17 View session

Copilot AI reviewed Mar 7, 2026

View reviewed changes

somethingwithproof and others added 2 commits March 7, 2026 05:32

fix: scope partition maintenance by table and lock create

d0a838d

Fixes Cacti#254 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>

somethingwithproof force-pushed the bug/partition-table-lock-fix branch from 5e1ed26 to f3b9613 Compare March 7, 2026 13:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bugfix: scope partition maintenance to target table and lock create path#271

bugfix: scope partition maintenance to target table and lock create path#271
somethingwithproof wants to merge 2 commits intoCacti:developfrom
somethingwithproof:bug/partition-table-lock-fix

somethingwithproof commented Mar 7, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 7, 2026

Uh oh!

somethingwithproof Mar 7, 2026

Uh oh!

Copilot AI Mar 7, 2026

Uh oh!

somethingwithproof Mar 7, 2026

Uh oh!

Copilot AI Mar 7, 2026

Uh oh!

somethingwithproof Mar 7, 2026

Uh oh!

Copilot AI Mar 7, 2026

Uh oh!

somethingwithproof Mar 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-if (substr_count($functions, "table_name='\" . \$table . \"'") < 3) {
+$scopedTableCount = 0;
+foreach (preg_split('/\R/', $functions) as $line) {
+	if (strpos($line, 'table_name') !== false && strpos($line, '$table') !== false) {
+		$scopedTableCount++;
+	}
+}
+if ($scopedTableCount < 3) {

Conversation

somethingwithproof commented Mar 7, 2026

Summary

Validation

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

somethingwithproof Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

somethingwithproof Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

somethingwithproof Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

somethingwithproof Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants