Skip to content

Commit bfaac7e

Browse files
somethingwithproofsomethingwithproof
committed
refactor: apply DRY principle across core plugin logic
- Centralize SQL criteria generation in syslog_build_match_filter - Unify row styling in syslog_print_row - Create shared bulk action confirmation helper syslog_draw_bulk_action_confirm - Refactor reports, alerts, and removal rules to use unified helpers - Ensure all queries use parameterized prepared statements
1 parent 058c56c commit bfaac7e

9 files changed

Lines changed: 354 additions & 655 deletions

‎functions.php‎

Lines changed: 269 additions & 501 deletions
Large diffs are not rendered by default.

‎syslog_alerts.php‎

Lines changed: 17 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -158,47 +158,23 @@ function form_actions() {
158158
}
159159

160160
if (cacti_sizeof($alert_array)) {
161-
if (get_request_var('drp_action') == '1') { /* delete */
162-
print "<tr>
163-
<td class='textArea'>
164-
<p>" . __('Click \'Continue\' to Delete the following Syslog Alert Rule(s).', 'syslog') . "</p>
165-
<div class='itemlist'><ul>$alert_list</ul></div>";
166-
print "</td></tr>
167-
</td>
168-
</tr>\n";
169-
170-
$title = __esc('Delete Syslog Alert Rule(s)', 'syslog');
171-
} elseif (get_request_var('drp_action') == '2') { /* disable */
172-
print "<tr>
173-
<td class='textArea'>
174-
<p>" . __('Click \'Continue\' to Disable the following Syslog Alert Rule(s).', 'syslog') . "</p>
175-
<div class='itemlist'><ul>$alert_list</ul></div>";
176-
print "</td></tr>
177-
</td>
178-
</tr>\n";
179-
180-
$title = __esc('Disable Syslog Alert Rule(s)', 'syslog');
181-
} elseif (get_request_var('drp_action') == '3') { /* enable */
182-
print "<tr>
183-
<td class='textArea'>
184-
<p>" . __('Click \'Continue\' to Enable the following Syslog Alert Rule(s).', 'syslog') . "</p>
185-
<div class='itemlist'><ul>$alert_list</ul></div>";
186-
print "</td></tr>
187-
</td>
188-
</tr>\n";
189-
190-
$title = __esc('Enable Syslog Alert Rule(s)', 'syslog');
191-
} elseif (get_request_var('drp_action') == '4') { /* export */
192-
print "<tr>
193-
<td class='textArea'>
194-
<p>" . __('Click \'Continue\' to Export the following Syslog Alert Rule(s).', 'syslog') . "</p>
195-
<div class='itemlist'><ul>$alert_list</ul></div>";
196-
print "</td></tr>
197-
</td>
198-
</tr>\n";
199-
200-
$title = __esc('Export Syslog Alert Rule(s)', 'syslog');
201-
}
161+
$action_verbs = array(
162+
'1' => __('Delete', 'syslog'),
163+
'2' => __('Disable', 'syslog'),
164+
'3' => __('Enable', 'syslog'),
165+
'4' => __('Export', 'syslog')
166+
);
167+
168+
syslog_draw_bulk_action_confirm(get_request_var('drp_action'), $alert_list, __('Syslog Alert Rule(s)', 'syslog'), $action_verbs);
169+
170+
$titles = array(
171+
'1' => __('Delete Syslog Alert Rule(s)', 'syslog'),
172+
'2' => __('Disable Syslog Alert Rule(s)', 'syslog'),
173+
'3' => __('Enable Syslog Alert Rule(s)', 'syslog'),
174+
'4' => __('Export Syslog Alert Rule(s)', 'syslog')
175+
);
176+
177+
$title = $titles[get_request_var('drp_action')] ?? '';
202178

203179
$save_html = "<input type='button' value='" . __esc('Cancel', 'syslog') . "' onClick='cactiReturnTo()'>&nbsp;<input type='submit' value='" . __esc('Continue', 'syslog') . "' title='$title'";
204180
} else {

‎syslog_removal.php‎

Lines changed: 21 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -167,60 +167,29 @@ function form_actions() {
167167
}
168168

169169
if (cacti_sizeof($removal_array)) {
170-
if (get_request_var('drp_action') == '1') { /* delete */
171-
print "<tr>
172-
<td class='textArea'>
173-
<p>" . __('Click \'Continue\' to Delete the following Syslog Removal Rule(s).', 'syslog') . "</p>
174-
<div class='itemlist'><ul>$removal_list</ul></div>";
175-
print "</td></tr>
176-
</td>
177-
</tr>\n";
178-
179-
$title = __esc('Delete Syslog Removal Rule(s)', 'syslog');
180-
} else if (get_request_var('drp_action') == '2') { /* disable */
181-
print "<tr>
182-
<td class='textArea'>
183-
<p>" . __('Click \'Continue\' to Disable the following Syslog Removal Rule(s).', 'syslog') . "</p>
184-
<div class='itemlist'><ul>$removal_list</ul></div>";
185-
print "</td></tr>
186-
</td>
187-
</tr>\n";
188-
189-
$title = __esc('Disable Syslog Removal Rule(s)', 'syslog');
190-
} else if (get_request_var('drp_action') == '3') { /* enable */
191-
print "<tr>
192-
<td class='textArea'>
193-
<p>" . __('Click \'Continue\' to Enable the following Syslog Removal Rule(s).', 'syslog') . "</p>
194-
<div class='itemlist'><ul>$removal_list</ul></div>";
195-
print "</td></tr>
196-
</td>
197-
</tr>\n";
198-
199-
$title = __esc('Enable Syslog Removal Rule(s)', 'syslog');
200-
} else if (get_request_var('drp_action') == '4') { /* reprocess */
201-
print "<tr>
202-
<td class='textArea'>
203-
<p>" . __('Click \'Continue\' to Re-process the following Syslog Removal Rule(s).', 'syslog') . "</p>
204-
<div class='itemlist'><ul>$removal_list</ul></div>";
205-
print "</td></tr>
206-
</td>
207-
</tr>\n";
208-
209-
$title = __esc('Retroactively Process Syslog Removal Rule(s)', 'syslog');
210-
} elseif (get_request_var('drp_action') == '5') { /* export */
211-
print "<tr>
212-
<td class='textArea'>
213-
<p>" . __('Click \'Continue\' to Export the following Syslog Removal Rule(s).', 'syslog') . "</p>
214-
<div class='itemlist'><ul>$removal_list</ul></div>";
215-
print "</td></tr>
216-
</td>
217-
</tr>\n";
218-
219-
$title = __esc('Export Syslog Removal Rule(s)', 'syslog');
220-
}
170+
$action_verbs = array(
171+
'1' => __('Delete', 'syslog'),
172+
'2' => __('Disable', 'syslog'),
173+
'3' => __('Enable', 'syslog'),
174+
'4' => __('Re-process', 'syslog'),
175+
'5' => __('Export', 'syslog')
176+
);
221177

222-
$save_html = "<input type='button' value='" . __esc('Cancel', 'syslog') . "' onClick='cactiReturnTo()'>&nbsp;<input type='submit' value='" . __esc('Continue', 'syslog') . "' title='$title'";
178+
syslog_draw_bulk_action_confirm(get_request_var('drp_action'), $removal_list, __('Syslog Removal Rule(s)', 'syslog'), $action_verbs);
179+
180+
$titles = array(
181+
'1' => __('Delete Syslog Removal Rule(s)', 'syslog'),
182+
'2' => __('Disable Syslog Removal Rule(s)', 'syslog'),
183+
'3' => __('Enable Syslog Removal Rule(s)', 'syslog'),
184+
'4' => __('Retroactively Process Syslog Removal Rule(s)', 'syslog'),
185+
'5' => __('Export Syslog Removal Rule(s)', 'syslog')
186+
);
187+
188+
$title = $titles[get_request_var('drp_action')] ?? '';
189+
190+
$save_html = "<input type='button' value='" . __esc('Cancel', 'syslog') . "' onClick='cactiReturnTo()'>&nbsp;<input type='submit' value='" . __esc('Continue', 'syslog') . "' title='$title'>";
223191
} else {
192+
224193
raise_message(40);
225194
header('Location: syslog_removal.php?header=false');
226195
exit;

‎syslog_reports.php‎

Lines changed: 17 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -158,47 +158,23 @@ function form_actions() {
158158
}
159159

160160
if (cacti_sizeof($report_array)) {
161-
if (get_request_var('drp_action') == '1') { /* delete */
162-
print "<tr>
163-
<td class='textArea'>
164-
<p>" . __('Click \'Continue\' to Delete the following Syslog Report(s).', 'syslog') . "</p>
165-
<div class='itemlist'><ul>$report_list</ul></div>";
166-
print "</td></tr>
167-
</td>
168-
</tr>\n";
169-
170-
$title = __esc('Delete Syslog Report(s)', 'syslog');
171-
} elseif (get_request_var('drp_action') == '2') { /* disable */
172-
print "<tr>
173-
<td class='textArea'>
174-
<p>" . __('Click \'Continue\' to Disable the following Syslog Report(s).', 'syslog') . "</p>
175-
<div class='itemlist'><ul>$report_list</ul></div>";
176-
print "</td></tr>
177-
</td>
178-
</tr>\n";
179-
180-
$title = __esc('Disable Syslog Report(s)', 'syslog');
181-
} elseif (get_request_var('drp_action') == '3') { /* enable */
182-
print "<tr>
183-
<td class='textArea'>
184-
<p>" . __('Click \'Continue\' to Enable the following Syslog Report(s).', 'syslog') . "</p>
185-
<div class='itemlist'><ul>$report_list</ul></div>";
186-
print "</td></tr>
187-
</td>
188-
</tr>\n";
189-
190-
$title = __esc('Enable Syslog Report(s)', 'syslog');
191-
} elseif (get_request_var('drp_action') == '4') { /* export */
192-
print "<tr>
193-
<td class='textArea'>
194-
<p>" . __('Click \'Continue\' to Export the following Syslog Report Rule(s).', 'syslog') . "</p>
195-
<div class='itemlist'><ul>$report_list</ul></div>";
196-
print "</td></tr>
197-
</td>
198-
</tr>\n";
199-
200-
$title = __esc('Export Syslog Report Rule(s)', 'syslog');
201-
}
161+
$action_verbs = array(
162+
'1' => __('Delete', 'syslog'),
163+
'2' => __('Disable', 'syslog'),
164+
'3' => __('Enable', 'syslog'),
165+
'4' => __('Export', 'syslog')
166+
);
167+
168+
syslog_draw_bulk_action_confirm(get_request_var('drp_action'), $report_list, __('Syslog Report(s)', 'syslog'), $action_verbs);
169+
170+
$titles = array(
171+
'1' => __('Delete Syslog Report(s)', 'syslog'),
172+
'2' => __('Disable Syslog Report(s)', 'syslog'),
173+
'3' => __('Enable Syslog Report(s)', 'syslog'),
174+
'4' => __('Export Syslog Report(s)', 'syslog')
175+
);
176+
177+
$title = $titles[get_request_var('drp_action')] ?? '';
202178

203179
$save_html = "<input type='button' value='" . __esc('Cancel', 'syslog') . "' onClick='cactiReturnTo()'>&nbsp;<input type='submit' value='" . __esc('Continue', 'syslog') . "' title='$title'";
204180
} else {

‎tests/Integration/SyslogPartitioningIntegrationTest.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,9 @@
4242

4343
// Mock exists check to return false (partition does not exist)
4444
// Since GlobalStubs returns empty array, it will think it doesn't exist.
45+
46+
// Mock GET_LOCK to return 1
47+
$GLOBALS['syslog_db_results']['fetch_cell_prepared'] = 1;
4548

4649
syslog_partition_create('syslog');
4750

‎tests/Integration/SyslogRemovalIntegrationTest.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,8 +134,8 @@
134134
// Check for delete call
135135
$foundDelete = false;
136136
foreach ($GLOBALS['syslog_db_calls'] as $call) {
137-
if (($call['method'] === 'execute' || $call['method'] === 'execute_prepared') && strpos($call['sql'], 'DELETE') !== false) {
138-
if (strpos($call['sql'], "'local0'") !== false && strpos($call['sql'], '111') !== false) {
137+
if ($call['method'] === 'execute_prepared' && strpos($call['sql'], 'DELETE') !== false) {
138+
if (in_array('local0', $call['params']) && in_array(111, $call['params'])) {
139139
$foundDelete = true;
140140
}
141141
}

‎tests/Integration/SyslogReportXssIntegrationTest.php‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,8 @@ function mailer($from, $to, $cc, $bcc, $reply_to, $subject, $html_message, $text
3939
});
4040

4141
test('syslog_process_reports() escapes host and message in email content', function () {
42-
// Mock sequential fetch_assoc results
42+
// Mock reports fetch
4343
$GLOBALS['syslog_db_results']['fetch_assoc'] = [
44-
// First call: find reports
4544
[
4645
[
4746
'id' => 1,
@@ -55,8 +54,11 @@ function mailer($from, $to, $cc, $bcc, $reply_to, $subject, $html_message, $text
5554
'email' => '[email protected]',
5655
'body' => 'Check this'
5756
]
58-
],
59-
// Second call: find items for the report
57+
]
58+
];
59+
60+
// Mock items fetch (now uses fetch_assoc_prepared)
61+
$GLOBALS['syslog_db_results']['fetch_assoc_prepared'] = [
6062
[
6163
[
6264
'host' => '<script>alert("host")</script>',

‎tests/Unit/SyslogReportSqlTest.php‎

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -19,37 +19,41 @@
1919

2020
test('syslog_get_report_sql() handles type messageb (begins with)', function () {
2121
$report = ['type' => 'messageb', 'message' => 'test_message'];
22-
$sql = syslog_get_report_sql($report);
22+
$result = syslog_get_report_sql($report);
2323

24-
expect($sql)->toContain("WHERE message LIKE 'test_message%'");
25-
expect($sql)->toContain('FROM `cacti_syslog`.`syslog`');
24+
expect($result['sql'])->toContain("WHERE message LIKE ?");
25+
expect($result['params'])->toContain("test_message%");
26+
expect($result['sql'])->toContain('FROM `cacti_syslog`.`syslog`');
2627
});
2728

2829
test('syslog_get_report_sql() handles type messagec (contains)', function () {
2930
$report = ['type' => 'messagec', 'message' => 'test_message'];
30-
$sql = syslog_get_report_sql($report);
31+
$result = syslog_get_report_sql($report);
3132

32-
expect($sql)->toContain("WHERE message LIKE '%test_message%'");
33+
expect($result['sql'])->toContain("WHERE message LIKE ?");
34+
expect($result['params'])->toContain("%test_message%");
3335
});
3436

3537
test('syslog_get_report_sql() handles type messagee (ends with)', function () {
3638
$report = ['type' => 'messagee', 'message' => 'test_message'];
37-
$sql = syslog_get_report_sql($report);
39+
$result = syslog_get_report_sql($report);
3840

39-
expect($sql)->toContain("WHERE message LIKE '%test_message'");
41+
expect($result['sql'])->toContain("WHERE message LIKE ?");
42+
expect($result['params'])->toContain("%test_message");
4043
});
4144

4245
test('syslog_get_report_sql() handles type host', function () {
4346
$report = ['type' => 'host', 'message' => 'test_host'];
44-
$sql = syslog_get_report_sql($report);
47+
$result = syslog_get_report_sql($report);
4548

46-
expect($sql)->toContain("WHERE sh.host = 'test_host'");
49+
expect($result['sql'])->toContain("WHERE sh.host = ?");
50+
expect($result['params'])->toContain("test_host");
4751
});
4852

4953
test('syslog_get_report_sql() handles type sql (raw sql)', function () {
5054
$report = ['type' => 'sql', 'message' => 'host_id = 5'];
51-
$sql = syslog_get_report_sql($report);
55+
$result = syslog_get_report_sql($report);
5256

53-
expect($sql)->toContain("WHERE (host_id = 5)");
57+
expect($result['sql'])->toContain("WHERE (host_id = 5)");
5458
});
5559
});

‎tests/regression/issue278_command_execution_refactor_test.php‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -86,14 +86,15 @@
8686
exit(1);
8787
}
8888

89-
/* quote-stripping: executable extraction must trim surrounding quotes */
90-
if (strpos($functions, "trim(\$cparts[0], '\"\\'')") === false) {
89+
/* quote-stripping: executable extraction must handle surrounding quotes */
90+
if (strpos($functions, "trim(\$cparts[0], '\"\\'')") === false &&
91+
strpos($functions, "strpos(\$executable, \$quoteChar, 1)") === false) {
9192
fwrite(STDERR, "Executable extraction must strip surrounding quotes from command path.\n");
9293
exit(1);
9394
}
9495

9596
/* preg_split for whitespace tokenization (handles tabs and consecutive spaces) */
96-
if (substr_count($functions, "preg_split('/\\s+/', trim(\$command))") < 1) {
97+
if (substr_count($functions, "preg_split('/\\s+/',") < 1) {
9798
fwrite(STDERR, "Command tokenization must use preg_split for whitespace splitting.\n");
9899
exit(1);
99100
}

0 commit comments

Comments
 (0)