From 076b18755e66412f170b5e087e45dd70b121ba3f Mon Sep 17 00:00:00 2001 From: TheWitness Date: Fri, 25 Sep 2026 02:51:28 -0400 Subject: [PATCH 1/2] chore: PHPStan level 8 typing pass + bugfixes Adds native param/return types where safe, PHPDoc-only types at Cacti hook boundaries, and fixes several genuine bugs surfaced during the typing pass: - setup.php: quicktree_show_tab() built its tab link URL with a leftover literal "tab" concatenated directly before quicktree_page_location()'s result, producing an invalid ?location=tabtab / ?location=tabconsole query string. Fixed to concatenate only the function's return value. - quicktree.php: the "no graph specified" branch of the add action passed an undefined $title into __() where the format string has no %s placeholder - clearly meant to be the 'quicktree' text-domain arg used everywhere else in the file; fixed. - quicktree.php: removed 3 unreachable break; statements after exit;. - quicktree.php: simplified an always-true empty($save['id']) check (the key is unconditionally set to '' immediately before) - PHPStan correctly flagged this as dead logic; behavior unchanged. - parse_ini_file() result narrowed before array access (can return false). - html_start_box() $div/$cell_padding args fixed to native bool/int in 3 call sites across setup.php/ui_helpers.php/quicktree.php. Result: PHPStan level 8 clean (18 -> 0 errors), full Pest suite (58/58) passing against the 1.2.x baseline. Two behaviorally-neutral exceptions made to keep pre-existing tests green (see PR body for details): reverted one api_plugin_register_hook() call's 5th arg from true back to the literal 1 a test asserts on (loosened only the scratch PHPStan stub's param type instead), and reverted one db_execute_prepared() call's params back to array() syntax since php-cs-fixer's []-shorthand conversion broke a test's own array()-based SQL-interpolation-detection regex. --- locales/po/cacti.pot | 100 ++++++++++---------- quicktree.php | 122 ++++++++++++------------ quicktree_security.php | 2 +- setup.php | 207 +++++++++++++++++++++-------------------- ui_helpers.php | 6 +- 5 files changed, 217 insertions(+), 220 deletions(-) diff --git a/locales/po/cacti.pot b/locales/po/cacti.pot index 2ed5b81..86f7959 100644 --- a/locales/po/cacti.pot +++ b/locales/po/cacti.pot @@ -8,7 +8,7 @@ msgid "" msgstr "" "Project-Id-Version: Cacti \n" "Report-Msgid-Bugs-To: developers@cacti.net\n" -"POT-Creation-Date: 2026-09-24 13:43-0400\n" +"POT-Creation-Date: 2026-09-25 02:50-0400\n" "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n" "Last-Translator: Howard Jones >\n" "Language-Team: Howard Jones \n" @@ -17,159 +17,159 @@ msgstr "" "Content-Type: text/plain; charset=UTF-8\n" "Content-Transfer-Encoding: 8bit\n" -#: quicktree.php:36 quicktree.php:357 +#: quicktree.php:37 quicktree.php:349 msgid "Save To New Tree" msgstr "" -#: quicktree.php:37 quicktree.php:358 +#: quicktree.php:38 quicktree.php:350 msgid "Save To Branch" msgstr "" -#: quicktree.php:38 +#: quicktree.php:39 msgid "Clear All Graphs" msgstr "" -#: quicktree.php:85 +#: quicktree.php:86 msgid "Graph Added to QuickTree" msgstr "" -#: quicktree.php:86 +#: quicktree.php:87 #, php-format msgid "Graph %s added to QuickTree. Goto the QuickTree page to move all Graphs to a new or existing Tree." msgstr "" -#: quicktree.php:89 +#: quicktree.php:90 msgid "Graph Not Added to QuickTree" msgstr "" -#: quicktree.php:90 +#: quicktree.php:91 #, php-format msgid "Graph %s was NOT added to QuickTree as it's already there waiting to be placed on a Tree. Goto the QuickTree page to move all Graphs to a new or existing Tree." msgstr "" -#: quicktree.php:94 +#: quicktree.php:95 msgid "Graph Not Specified. Not added to QuickTree" msgstr "" -#: quicktree.php:95 +#: quicktree.php:96 msgid "No Graph was added to QuickTree as no Graph was specified." msgstr "" -#: quicktree.php:107 +#: quicktree.php:108 msgid "Click 'Continue' to Add the following Tree." msgstr "" -#: quicktree.php:113 quicktree.php:157 +#: quicktree.php:114 quicktree.php:158 msgid "Tree Name" msgstr "" -#: quicktree.php:116 +#: quicktree.php:117 msgid "New Tree" msgstr "" -#: quicktree.php:122 quicktree.php:174 +#: quicktree.php:123 quicktree.php:175 msgid "Branch Name" msgstr "" -#: quicktree.php:125 quicktree.php:177 quicktree.php:343 setup.php:266 -#: setup.php:273 setup.php:280 setup.php:287 setup.php:294 setup.php:301 +#: quicktree.php:126 quicktree.php:178 quicktree.php:335 setup.php:269 +#: setup.php:276 setup.php:283 setup.php:290 setup.php:297 setup.php:304 msgid "QuickTree" msgstr "" -#: quicktree.php:133 quicktree.php:185 +#: quicktree.php:134 quicktree.php:186 msgid "Add To Branch" msgstr "" -#: quicktree.php:133 quicktree.php:185 +#: quicktree.php:134 quicktree.php:186 msgid "Cancel" msgstr "" -#: quicktree.php:133 quicktree.php:185 +#: quicktree.php:134 quicktree.php:186 msgid "Continue" msgstr "" -#: quicktree.php:149 +#: quicktree.php:150 msgid "Click 'Continue' to Add the following Tree Branch." msgstr "" -#: quicktree.php:151 +#: quicktree.php:152 msgid "Add to which Graph Tree?" msgstr "" -#: quicktree.php:292 +#: quicktree.php:288 msgid "QuickTree has Created Tree and/or Branch and added Graphs" msgstr "" -#: quicktree.php:306 +#: quicktree.php:302 msgid "QuickTree has no Graphs Queued" msgstr "" -#: quicktree.php:346 +#: quicktree.php:338 msgid "Information/Directions" msgstr "" -#: quicktree.php:356 +#: quicktree.php:348 msgid "The Graphs below are Queue to be added to a Cacti Tree. You may keep them here for as long as you like, or you can perform one of the following actions" msgstr "" -#: quicktree.php:357 +#: quicktree.php:349 msgid "Save your selection to a new Graph Tree so you can keep them for later and work on something new." msgstr "" -#: quicktree.php:358 +#: quicktree.php:350 msgid "Save your selection as a branch to an existing tree so that they appear in a specific section of an existing tree." msgstr "" -#: quicktree.php:359 +#: quicktree.php:351 msgid "Clear all graphs" msgstr "" -#: quicktree.php:359 +#: quicktree.php:351 msgid "Clear the Graphs on this page from the Graphs Queue so that you have a blank QuickTree ready for new selections" msgstr "" -#: quicktree.php:360 +#: quicktree.php:352 msgid "You can manage the individual graphs that appear here by clicking:" msgstr "" -#: quicktree.php:361 +#: quicktree.php:353 msgid "Add" msgstr "" -#: quicktree.php:361 +#: quicktree.php:353 msgid "Graph View Page" msgstr "" -#: quicktree.php:361 +#: quicktree.php:353 #, php-format msgid "This icon is next to a Graph on the %s tab." msgstr "" -#: quicktree.php:362 +#: quicktree.php:354 msgid "Delete" msgstr "" -#: quicktree.php:362 +#: quicktree.php:354 msgid "This icon next to the Graphs below to remove them from the QuickTree Queue." msgstr "" -#: quicktree.php:363 +#: quicktree.php:355 msgid "Note:" msgstr "" -#: quicktree.php:364 +#: quicktree.php:356 msgid "Adding, removing or clearing on this page does not affect any other parts of Cacti (only Creating/Saving does)" msgstr "" -#: quicktree.php:381 +#: quicktree.php:373 msgid "Actions" msgstr "" -#: quicktree.php:405 +#: quicktree.php:397 msgid "Remove This Graph From QuickTree" msgstr "" -#: quicktree.php:416 +#: quicktree.php:408 msgid "No Graphs Added Yet" msgstr "" @@ -177,42 +177,42 @@ msgstr "" msgid "QuickTree Management" msgstr "" -#: setup.php:88 +#: setup.php:90 msgid "Misc" msgstr "" -#: setup.php:92 setup.php:172 +#: setup.php:94 setup.php:175 msgid "Quicktree" msgstr "" -#: setup.php:96 +#: setup.php:98 msgid "Page Style" msgstr "" -#: setup.php:97 +#: setup.php:99 msgid "Where to display the QuickTree page" msgstr "" -#: setup.php:100 +#: setup.php:102 msgid "Tab" msgstr "" -#: setup.php:101 +#: setup.php:103 msgid "Console Menu" msgstr "" -#: setup.php:102 +#: setup.php:104 msgid "Both Tab and Console Menu" msgstr "" -#: setup.php:200 +#: setup.php:203 msgid "Add this graph to QuickTree" msgstr "" -#: setup.php:223 +#: setup.php:226 msgid "Management" msgstr "" -#: setup.php:223 +#: setup.php:226 msgid "QuickTree Trees" msgstr "" diff --git a/quicktree.php b/quicktree.php index 57e696f..d8e318e 100644 --- a/quicktree.php +++ b/quicktree.php @@ -30,29 +30,30 @@ include_once('plugins/quicktree/ui_helpers.php'); include_once('plugins/quicktree/quicktree_security.php'); +/** @var array $config */ define('QUICKTREE_BASE_URI', $config['url_path'] . 'plugins/quicktree/'); -$form_actions = array( +$form_actions = [ 1 => __('Save To New Tree', 'quicktree'), 2 => __('Save To Branch', 'quicktree'), 3 => __('Clear All Graphs', 'quicktree') -); +]; -$code_actions = array( +$code_actions = [ 1 => 'add_tree', 2 => 'add_branch', 3 => 'clear' -); +]; set_default_action(); -$action = get_request_var('action'); +$action = get_request_var('action'); $location = quicktree_normalize_location(get_nfilter_request_var('location')); -$user = $_SESSION['sess_user_id']; +$user = $_SESSION['sess_user_id']; -/* ================= input validation ================= */ -$drp_action = get_filter_request_var('drp_action', FILTER_VALIDATE_REGEXP, array('options' => array('regexp' => '/^([a-zA-Z0-9_]+)$/'))); -/* ==================================================== */ +// ================= input validation ================= +$drp_action = get_filter_request_var('drp_action', FILTER_VALIDATE_REGEXP, ['options' => ['regexp' => '/^([a-zA-Z0-9_]+)$/']]); +// ==================================================== if ($drp_action != null) { $action = $code_actions[$drp_action]; @@ -69,18 +70,18 @@ $title = db_fetch_cell_prepared('SELECT title_cache FROM graph_templates_graph WHERE local_graph_id = ?', - array($graph)); + [$graph]); $exists = db_fetch_cell_prepared('SELECT COUNT(*) FROM quicktree_graphs WHERE local_graph_id = ?', - array($graph)); + [$graph]); if (!$exists) { db_execute_prepared('INSERT INTO quicktree_graphs (userid, local_graph_id, rra_id, title) VALUES (?, ?, ?, ?)', - array($user, $graph, $rra, $title)); + [$user, $graph, $rra, $title]); $message['title'] = __('Graph Added to QuickTree', 'quicktree'); $message['message'] = __('Graph %s added to QuickTree. Goto the QuickTree page to move all Graphs to a new or existing Tree.', $title, 'quicktree'); @@ -92,7 +93,7 @@ } } else { $message['title'] = __('Graph Not Specified. Not added to QuickTree', 'quicktree'); - $message['message'] = __('No Graph was added to QuickTree as no Graph was specified.', $title); + $message['message'] = __('No Graph was added to QuickTree as no Graph was specified.', 'quicktree'); $message['level'] = MESSAGE_LEVEL_ERROR; } @@ -119,7 +120,7 @@ print ' -

+

@@ -142,7 +143,7 @@ $queryrows = db_fetch_assoc_prepared('SELECT g.id, g.name FROM graph_tree AS g ORDER BY g.name', - array()); + []); print ''; @@ -159,19 +160,19 @@ + print ' '; print ' -

+

@@ -193,8 +194,6 @@ header('Location: quicktree.php?header=false&drp_action=&action=&location=' . $location); exit; - - break; case 'save': $new_tree_id = -1; $parent_id = 0; @@ -202,7 +201,7 @@ $username = db_fetch_cell_prepared('SELECT username FROM user_auth WHERE id = ?', - array($user)); + [$user]); if (isset_request_var('tree_id')) { $new_tree_id = get_filter_request_var('tree_id'); @@ -210,12 +209,12 @@ $tree_name = db_fetch_cell_prepared('SELECT name FROM graph_tree WHERE id = ?', - array($new_tree_id)); + [$new_tree_id]); } elseif (isset_request_var('tree')) { $new_tree_id = db_fetch_cell_prepared('SELECT id FROM graph_tree WHERE name = ?', - array(get_nfilter_request_var('tree'))); + [get_nfilter_request_var('tree')]); $tree_name = get_nfilter_request_var('tree'); } @@ -233,7 +232,7 @@ $graphs = db_fetch_assoc_prepared('SELECT * FROM quicktree_graphs WHERE userid = ?', - array($user)); + [$user]); if (cacti_sizeof($graphs)) { include_once($config['base_path'] . '/lib/api_tree.php'); @@ -241,23 +240,20 @@ if (empty($new_tree_id)) { $seq = db_fetch_cell_prepared('SELECT MAX(sequence) FROM graph_tree', - array()); + []); - if ($seq == NULL || $seq < 0) { + if ($seq == null || $seq < 0) { $seq = 1; } - $save = array(); + $save = []; $save['id'] = ''; $save['name'] = $tree_name; $save['sort_type'] = TREE_ORDERING_ALPHABETIC; $save['sequence'] = $seq; $save['last_modified'] = date('Y-m-d H:i:s', time()); $save['modified_by'] = $_SESSION['sess_user_id']; - - if (empty($save['id'])) { - $save['user_id'] = $_SESSION['sess_user_id']; - } + $save['user_id'] = $_SESSION['sess_user_id']; $new_tree_id = sql_save($save, 'graph_tree'); @@ -276,7 +272,7 @@ AND local_graph_id = 0 ORDER BY id LIMIT 1', - array($new_tree_id, $branch)); + [$new_tree_id, $branch]); if (empty($parent_id)) { $parent_id = api_tree_item_save(0, $new_tree_id, TREE_ITEM_TYPE_HEADER, @@ -298,7 +294,7 @@ } ?> - +
" . __('Information/Directions', 'quicktree') . " @@ -352,26 +344,26 @@ print ""; - $form_items = array( - array(__('The Graphs below are Queue to be added to a Cacti Tree. You may keep them here for as long as you like, or you can perform one of the following actions', 'quicktree')), - array('' . __('Save To New Tree', 'quicktree') . '', __('Save your selection to a new Graph Tree so you can keep them for later and work on something new.', 'quicktree')), - array('' . __('Save To Branch', 'quicktree') . '', __('Save your selection as a branch to an existing tree so that they appear in a specific section of an existing tree.', 'quicktree')), - array('' . __('Clear all graphs', 'quicktree') . '', __('Clear the Graphs on this page from the Graphs Queue so that you have a blank QuickTree ready for new selections', 'quicktree')), - array('
' . __('You can manage the individual graphs that appear here by clicking:', 'quicktree')), - array('' . __('Add', 'quicktree'), __('This icon is next to a Graph on the %s tab.', '' . __('Graph View Page', 'quicktree') . '', 'quicktree')), - array('' . __('Delete', 'quicktree'), __('This icon next to the Graphs below to remove them from the QuickTree Queue.', 'qucktree')), - array('
' . __('Note:', 'quicktree') . '
'), - array(__('Adding, removing or clearing on this page does not affect any other parts of Cacti (only Creating/Saving does)', 'quicktree')) - ); + $form_items = [ + [__('The Graphs below are Queue to be added to a Cacti Tree. You may keep them here for as long as you like, or you can perform one of the following actions', 'quicktree')], + ['' . __('Save To New Tree', 'quicktree') . '', __('Save your selection to a new Graph Tree so you can keep them for later and work on something new.', 'quicktree')], + ['' . __('Save To Branch', 'quicktree') . '', __('Save your selection as a branch to an existing tree so that they appear in a specific section of an existing tree.', 'quicktree')], + ['' . __('Clear all graphs', 'quicktree') . '', __('Clear the Graphs on this page from the Graphs Queue so that you have a blank QuickTree ready for new selections', 'quicktree')], + ['
' . __('You can manage the individual graphs that appear here by clicking:', 'quicktree')], + ['' . __('Add', 'quicktree'), __('This icon is next to a Graph on the %s tab.', '' . __('Graph View Page', 'quicktree') . '', 'quicktree')], + ['' . __('Delete', 'quicktree'), __('This icon next to the Graphs below to remove them from the QuickTree Queue.', 'qucktree')], + ['
' . __('Note:', 'quicktree') . '
'], + [__('Adding, removing or clearing on this page does not affect any other parts of Cacti (only Creating/Saving does)', 'quicktree')] + ]; foreach ($form_items as $details) { form_alternate_row(); - if (cacti_sizeof($details) == 1) { - print ''; + if (isset($details[1])) { + print ''; + print ''; } else { - print ''; - print ''; + print ''; } form_end_row(); @@ -393,9 +385,9 @@ INNER JOIN graph_templates_graph AS gtg ON qt.local_graph_id = gtg.local_graph_id WHERE userid = ?', - array($user)); + [$user]); - if (cacti_sizeof($queryrows)) { + if (cacti_sizeof($queryrows)) { foreach ($queryrows as $gr) { $graph_title = html_escape($gr['title_cache']); @@ -409,12 +401,12 @@ print '' . $graph_title . ''; print '
'.$details[0].'' . $details[0] . '' . $details[1] . ''.$details[0].''.$details[1].'' . $details[0] . '
'; - } + } - print '
'; - } else { - print '

' . __('No Graphs Added Yet', 'quicktree') . '

'; - } + print '
'; + } else { + print '

' . __('No Graphs Added Yet', 'quicktree') . '

'; + } bottom_footer(); diff --git a/quicktree_security.php b/quicktree_security.php index f1f35d0..90e60e7 100644 --- a/quicktree_security.php +++ b/quicktree_security.php @@ -33,7 +33,7 @@ * * @return string Either 'console' or 'tab'. */ -function quicktree_normalize_location($value) { +function quicktree_normalize_location(string $value): string { if ($value === 'console') { return 'console'; } diff --git a/setup.php b/setup.php index 07840ae..42efdc6 100644 --- a/setup.php +++ b/setup.php @@ -33,7 +33,7 @@ * * @return bool Always returns true. */ -function plugin_quicktree_install() { +function plugin_quicktree_install(): bool { api_plugin_register_hook('quicktree', 'top_header_tabs', 'quicktree_show_tab', 'setup.php'); api_plugin_register_hook('quicktree', 'top_graph_header_tabs', 'quicktree_show_tab', 'setup.php'); api_plugin_register_hook('quicktree', 'config_arrays', 'quicktree_config_arrays', 'setup.php'); @@ -62,10 +62,12 @@ function plugin_quicktree_install() { * @global array $config Cacti global configuration array; used to locate * the plugin's base path. */ -function plugin_quicktree_version() { +function plugin_quicktree_version(): array { global $config; $info = parse_ini_file($config['base_path'] . '/plugins/quicktree/INFO', true); + $info = is_array($info) ? $info : []; + return $info['info']; } @@ -82,27 +84,27 @@ function plugin_quicktree_version() { * @global array $settings Cacti's registered Settings page fields, extended * here under the 'misc' key. */ -function quicktree_config_settings() { +function quicktree_config_settings(): void { global $tabs, $settings; $tabs['misc'] = __('Misc'); - $temp = array( - 'quicktree_header' => array( + $temp = [ + 'quicktree_header' => [ 'friendly_name' => __('Quicktree', 'quicktree'), - 'method' => 'spacer', - ), - 'quicktree_pagestyle' => array( + 'method' => 'spacer', + ], + 'quicktree_pagestyle' => [ 'friendly_name' => __('Page Style', 'quicktree'), - 'description' => __('Where to display the QuickTree page', 'quicktree'), - 'method' => 'drop_array', - 'array' => array( + 'description' => __('Where to display the QuickTree page', 'quicktree'), + 'method' => 'drop_array', + 'array' => [ 0 => __('Tab', 'quicktree'), 1 => __('Console Menu', 'quicktree'), 2 => __('Both Tab and Console Menu', 'quicktree') - ) - ) - ); + ] + ] + ]; if (isset($settings['misc'])) { $settings['misc'] = array_merge($settings['misc'], $temp); @@ -118,13 +120,13 @@ function quicktree_config_settings() { * quicktree_show_tab() when building the tab/console menu link. * * @param string $preferred The caller's preferred location, 'tab' or - * 'console'; defaults to 'tab'. + * 'console'; defaults to 'tab'. * * @return string The location to actually use: $preferred when allowed by * the current page-style setting, otherwise the other * location. */ -function quicktree_page_location($preferred = 'tab') { +function quicktree_page_location(string $preferred = 'tab'): string { $locsetting = read_config_option('quicktree_pagestyle'); if ($locsetting == 2) { @@ -159,17 +161,18 @@ function quicktree_page_location($preferred = 'tab') { * @global array $config Cacti global configuration array; used to build * the tab's image/link URLs. */ -function quicktree_show_tab() { +function quicktree_show_tab(): void { global $config; if (api_user_realm_auth('quicktree.php')) { $cp = false; + if (basename($_SERVER['PHP_SELF']) == 'quicktree.php') { $cp = true; } if (read_config_option('quicktree_pagestyle') != 1) { - print '' . __esc('Quicktree', 'quicktree') . ''; + print '' . __esc('Quicktree', 'quicktree') . ''; } } } @@ -182,7 +185,7 @@ function quicktree_show_tab() { * rendering a graph's action buttons. * * @param array $data Hook payload; $data[1] contains the current graph's - * 'local_graph_id' and 'rra' (RRA id). + * 'local_graph_id' and 'rra' (RRA id). * * @return void Outputs HTML directly. * @@ -190,7 +193,7 @@ function quicktree_show_tab() { * here; declared for parity with other graph_buttons * hook implementations). */ -function quicktree_graph_buttons($data) { +function quicktree_graph_buttons($data): void { global $config; if (api_user_realm_auth('quicktree.php')) { @@ -214,7 +217,7 @@ function quicktree_graph_buttons($data) { * @global array $menu Cacti's main navigation menu array, extended here * with this plugin's entry when applicable. */ -function quicktree_config_arrays() { +function quicktree_config_arrays(): void { global $menu; quicktree_check_upgrade(); @@ -235,14 +238,14 @@ function quicktree_config_arrays() { * @global array $config Cacti global configuration array; used to build * the JS/CSS asset URLs. */ -function quicktree_page_head() { +function quicktree_page_head(): void { global $config; $page = get_current_page(); print ''; - if (strstr($page, 'quicktree.php') !== false) { + if (strstr($page, 'quicktree.php') !== false) { print ''; } } @@ -256,53 +259,53 @@ function quicktree_page_head() { * breadcrumb trail. * * @param array $nav The existing breadcrumb map contributed by Cacti core - * and other plugins. + * and other plugins. * * @return array The $nav array with this plugin's breadcrumb entries * added. */ -function quicktree_draw_navigation_text($nav) { - $nav['quicktree.php:'] = array ( - 'title' => __('QuickTree', 'quicktree'), - 'mapping' => (get_nfilter_request_var('location') == 'console' ? 'index.php:':''), - 'url' => 'quicktree.php?location=' . get_nfilter_request_var('location'), - 'level' => '1' - ); - - $nav['quicktree.php:add_ajax'] = array ( - 'title' => __('QuickTree', 'quicktree'), - 'mapping' => (get_nfilter_request_var('location') == 'console' ? 'index.php:':''), - 'url' => 'quicktree.php?location=console', - 'level' => '1' - ); - - $nav['quicktree.php:add'] = array ( - 'title' => __('QuickTree', 'quicktree'), - 'mapping' => (get_nfilter_request_var('location') == 'console' ? 'index.php:':''), - 'url' => 'quicktree.php?location=console', - 'level' => '1' - ); - - $nav['quicktree.php:remove'] = array ( - 'title' => __('QuickTree', 'quicktree'), - 'mapping' => (get_nfilter_request_var('location') == 'console' ? 'index.php:':''), - 'url' => 'quicktree.php?location=console', - 'level' => '1' - ); - - $nav['quicktree.php:save'] = array ( - 'title' => __('QuickTree', 'quicktree'), - 'mapping' => (get_nfilter_request_var('location') == 'console' ? 'index.php:':''), - 'url' => 'quicktree.php?location=console', - 'level' => '1' - ); - - $nav['quicktree.php:clear'] = array ( - 'title' => __('QuickTree', 'quicktree'), - 'mapping' => (get_nfilter_request_var('location') == 'console' ? 'index.php:':''), - 'url' => 'quicktree.php?location=console', - 'level' => '1' - ); +function quicktree_draw_navigation_text($nav): array { + $nav['quicktree.php:'] = [ + 'title' => __('QuickTree', 'quicktree'), + 'mapping' => (get_nfilter_request_var('location') == 'console' ? 'index.php:' : ''), + 'url' => 'quicktree.php?location=' . get_nfilter_request_var('location'), + 'level' => '1' + ]; + + $nav['quicktree.php:add_ajax'] = [ + 'title' => __('QuickTree', 'quicktree'), + 'mapping' => (get_nfilter_request_var('location') == 'console' ? 'index.php:' : ''), + 'url' => 'quicktree.php?location=console', + 'level' => '1' + ]; + + $nav['quicktree.php:add'] = [ + 'title' => __('QuickTree', 'quicktree'), + 'mapping' => (get_nfilter_request_var('location') == 'console' ? 'index.php:' : ''), + 'url' => 'quicktree.php?location=console', + 'level' => '1' + ]; + + $nav['quicktree.php:remove'] = [ + 'title' => __('QuickTree', 'quicktree'), + 'mapping' => (get_nfilter_request_var('location') == 'console' ? 'index.php:' : ''), + 'url' => 'quicktree.php?location=console', + 'level' => '1' + ]; + + $nav['quicktree.php:save'] = [ + 'title' => __('QuickTree', 'quicktree'), + 'mapping' => (get_nfilter_request_var('location') == 'console' ? 'index.php:' : ''), + 'url' => 'quicktree.php?location=console', + 'level' => '1' + ]; + + $nav['quicktree.php:clear'] = [ + 'title' => __('QuickTree', 'quicktree'), + 'mapping' => (get_nfilter_request_var('location') == 'console' ? 'index.php:' : ''), + 'url' => 'quicktree.php?location=console', + 'level' => '1' + ]; return $nav; } @@ -315,22 +318,22 @@ function quicktree_draw_navigation_text($nav) { * * @return void */ -function quicktree_setup_table() { - $data = array(); - - $data['columns'][] = array('name' => 'id', 'type' => 'int(11)', 'NULL' => false, 'auto_increment' => true); - $data['columns'][] = array('name' => 'userid', 'type' => 'int(11)', 'NULL' => false); - $data['columns'][] = array('name' => 'local_graph_id', 'type' => 'int(11)', 'NULL' => false, 'default' => '0'); - $data['columns'][] = array('name' => 'rra_id', 'type' => 'int(11)', 'NULL' => false, 'default' => '0'); - $data['columns'][] = array('name' => 'title', 'type' => 'varchar(191)', 'NULL' => false, 'default' => ''); - $data['primary'] = 'id'; - $data['type'] = 'InnoDB'; - $data['comment'] = 'Quicktree data'; +function quicktree_setup_table(): void { + $data = []; + + $data['columns'][] = ['name' => 'id', 'type' => 'int(11)', 'NULL' => false, 'auto_increment' => true]; + $data['columns'][] = ['name' => 'userid', 'type' => 'int(11)', 'NULL' => false]; + $data['columns'][] = ['name' => 'local_graph_id', 'type' => 'int(11)', 'NULL' => false, 'default' => '0']; + $data['columns'][] = ['name' => 'rra_id', 'type' => 'int(11)', 'NULL' => false, 'default' => '0']; + $data['columns'][] = ['name' => 'title', 'type' => 'varchar(191)', 'NULL' => false, 'default' => '']; + $data['primary'] = 'id'; + $data['type'] = 'InnoDB'; + $data['comment'] = 'Quicktree data'; api_plugin_db_table_create('quicktree', 'quicktree_graphs', $data); $pagestyle = read_config_option('quicktree_pagestyle'); - if ($pagestyle == '' or $pagestyle < 0 or $pagestyle > 2) { + if ($pagestyle == '' || $pagestyle < 0 || $pagestyle > 2) { set_config_option('quicktree_pagestyle', '0'); } } @@ -342,10 +345,11 @@ function quicktree_setup_table() { * * @return bool Always returns false (no separate upgrade routine to run). */ -function plugin_quicktree_upgrade() { - /* Here we will upgrade to the newest version */ +function plugin_quicktree_upgrade(): bool { + // Here we will upgrade to the newest version quicktree_check_upgrade(); - return FALSE; + + return false; } /** @@ -356,8 +360,8 @@ function plugin_quicktree_upgrade() { * * @return void */ -function plugin_quicktree_uninstall() { - /* Do any extra Uninstall stuff here */ +function plugin_quicktree_uninstall(): void { + // Do any extra Uninstall stuff here } /** @@ -367,9 +371,9 @@ function plugin_quicktree_uninstall() { * * @return bool Always returns true. */ -function plugin_quicktree_check_config() { - /* Here we will check to ensure everything is configured */ - return TRUE; +function plugin_quicktree_check_config(): bool { + // Here we will check to ensure everything is configured + return true; } /** @@ -384,39 +388,40 @@ function plugin_quicktree_check_config() { * * @return void */ -function quicktree_check_upgrade() { - $files = array('plugins.php', 'quicktree.php', 'index.php', 'graph_view.php'); - if (isset($_SERVER['PHP_SELF']) && !in_array(basename($_SERVER['PHP_SELF']), $files)) { - return; - } +function quicktree_check_upgrade(): void { + $files = ['plugins.php', 'quicktree.php', 'index.php', 'graph_view.php']; - $info = plugin_quicktree_version(); - $current = $info['version']; + if (isset($_SERVER['PHP_SELF']) && !in_array(basename($_SERVER['PHP_SELF']), $files, true)) { + return; + } + + $info = plugin_quicktree_version(); + $current = $info['version']; $old = db_fetch_cell_prepared('SELECT version FROM plugin_config WHERE directory = ?', - array('quicktree')); + ['quicktree']); - if ($current != $old) { - api_plugin_register_hook('quicktree', 'page_head', 'quicktree_page_head', 'setup.php', 1); + if ($current != $old) { + api_plugin_register_hook('quicktree', 'page_head', 'quicktree_page_head', 'setup.php', 1); - quicktree_setup_table(); + quicktree_setup_table(); if (function_exists('api_plugin_upgrade_register')) { api_plugin_upgrade_register('quicktree'); } else { - db_execute_prepared('UPDATE plugin_config SET + db_execute_prepared('UPDATE plugin_config SET version = ?, name = ?, author = ?, webpage = ? WHERE directory = ?', - array( + [ $info['version'], $info['longname'], $info['author'], $info['homepage'], $info['name'] - ) + ] ); } - } + } } diff --git a/ui_helpers.php b/ui_helpers.php index a7bb0df..0bd6406 100644 --- a/ui_helpers.php +++ b/ui_helpers.php @@ -22,13 +22,13 @@ * * @return void Outputs HTML directly. */ - function quicktree_action_form_begin($title) { + function quicktree_action_form_begin(string $title): void { if (get_nfilter_request_var('header') == null) { top_header(); } form_start('quicktree.php', 'quicktree_form'); - html_start_box($title, '60%', '', '3', 'center', ''); + html_start_box($title, '60%', false, 3, 'center', ''); } } @@ -40,7 +40,7 @@ function quicktree_action_form_begin($title) { * * @return void Outputs HTML directly. */ - function quicktree_action_form_end() { + function quicktree_action_form_end(): void { html_end_box(); form_end(); } From 20b8fc2a036eb0aff55169580f0f4c678012038e Mon Sep 17 00:00:00 2001 From: TheWitness Date: Fri, 25 Sep 2026 03:00:10 -0400 Subject: [PATCH 2/2] fix: address Copilot review findings - quicktree.php: fixed a misspelled 'qucktree' text domain in a translated string (was silently falling back to untranslated text). - quicktree_security.php: removed the native `string` param type from quicktree_normalize_location() - its only caller passes get_nfilter_request_var('location'), which returns `mixed` and can be a non-string (e.g. an array via `location[]=console`), which would raise a TypeError instead of falling back to 'tab'. Return type stays `string` since the body always returns one of two string literals. - CHANGELOG.md: added a develop entry documenting this pass's bugfixes. - tests/Unit/QuicktreeShowTabTest.php: new regression test suite for quicktree_show_tab(), asserting its rendered link is exactly '?location=tab' (never '?location=tabtab' or '?location=tabconsole'), covering the Tab-only, Tab+Console, Console-only, and unauthorized cases. Added a minimal api_user_realm_auth() stub to tests/bootstrap-unit.php to support it. Re-verified: PHPStan level 8 clean, php -l clean, full Pest suite (62/62, 4 new) passing against the 1.2.x baseline. --- CHANGELOG.md | 1 + locales/po/cacti.pot | 2 +- quicktree.php | 2 +- quicktree_security.php | 2 +- tests/Unit/QuicktreeShowTabTest.php | 69 +++++++++++++++++++++++++++++ tests/bootstrap-unit.php | 9 ++-- 6 files changed, 79 insertions(+), 6 deletions(-) create mode 100644 tests/Unit/QuicktreeShowTabTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 6e4ffca..31adef1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ --- develop --- - issue: Quicktree continually registers a realm +- issue: Fix quicktree_show_tab() building an invalid `?location=tabtab`/`?location=tabconsole` URL, an undefined-variable text-domain typo in the QuickTree page's "no graph specified" message, and a misspelled `qucktree` text domain - security: Migrate quicktree SQL helpers to prepared statements, normalize location handling, and ensure redirects terminate execution - test: Migrate test suite to Pest with a Cacti-provided test harness and CI workflow diff --git a/locales/po/cacti.pot b/locales/po/cacti.pot index 86f7959..147a562 100644 --- a/locales/po/cacti.pot +++ b/locales/po/cacti.pot @@ -8,7 +8,7 @@ msgid "" msgstr "" "Project-Id-Version: Cacti \n" "Report-Msgid-Bugs-To: developers@cacti.net\n" -"POT-Creation-Date: 2026-09-25 02:50-0400\n" +"POT-Creation-Date: 2026-09-25 02:59-0400\n" "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n" "Last-Translator: Howard Jones >\n" "Language-Team: Howard Jones \n" diff --git a/quicktree.php b/quicktree.php index d8e318e..1c13389 100644 --- a/quicktree.php +++ b/quicktree.php @@ -351,7 +351,7 @@ ['' . __('Clear all graphs', 'quicktree') . '', __('Clear the Graphs on this page from the Graphs Queue so that you have a blank QuickTree ready for new selections', 'quicktree')], ['
' . __('You can manage the individual graphs that appear here by clicking:', 'quicktree')], ['' . __('Add', 'quicktree'), __('This icon is next to a Graph on the %s tab.', '' . __('Graph View Page', 'quicktree') . '', 'quicktree')], - ['' . __('Delete', 'quicktree'), __('This icon next to the Graphs below to remove them from the QuickTree Queue.', 'qucktree')], + ['' . __('Delete', 'quicktree'), __('This icon next to the Graphs below to remove them from the QuickTree Queue.', 'quicktree')], ['
' . __('Note:', 'quicktree') . '
'], [__('Adding, removing or clearing on this page does not affect any other parts of Cacti (only Creating/Saving does)', 'quicktree')] ]; diff --git a/quicktree_security.php b/quicktree_security.php index 90e60e7..011aa36 100644 --- a/quicktree_security.php +++ b/quicktree_security.php @@ -33,7 +33,7 @@ * * @return string Either 'console' or 'tab'. */ -function quicktree_normalize_location(string $value): string { +function quicktree_normalize_location($value): string { if ($value === 'console') { return 'console'; } diff --git a/tests/Unit/QuicktreeShowTabTest.php b/tests/Unit/QuicktreeShowTabTest.php new file mode 100644 index 0000000..e451853 --- /dev/null +++ b/tests/Unit/QuicktreeShowTabTest.php @@ -0,0 +1,69 @@ +toContain('quicktree.php?location=tab"'); + expect($output)->not->toContain('location=tabtab'); + expect($output)->not->toContain('location=tabconsole'); +}); + +it('links to plain ?location=tab when both Tab and Console Menu are enabled', function () { + $GLOBALS['__test_config_options']['quicktree_pagestyle'] = '2'; + + ob_start(); + quicktree_show_tab(); + $output = ob_get_clean(); + + expect($output)->toContain('quicktree.php?location=tab"'); + expect($output)->not->toContain('location=tabtab'); + expect($output)->not->toContain('location=tabconsole'); +}); + +it('prints nothing when the page style is Console Menu only', function () { + $GLOBALS['__test_config_options']['quicktree_pagestyle'] = '1'; + + ob_start(); + quicktree_show_tab(); + $output = ob_get_clean(); + + expect($output)->toBe(''); +}); + +it('prints nothing when the user is not authorized for the quicktree realm', function () { + $GLOBALS['__test_config_options']['quicktree_pagestyle'] = '0'; + $GLOBALS['__test_realm_auth'] = false; + + ob_start(); + quicktree_show_tab(); + $output = ob_get_clean(); + + expect($output)->toBe(''); +}); diff --git a/tests/bootstrap-unit.php b/tests/bootstrap-unit.php index 8a618f3..c9f6541 100644 --- a/tests/bootstrap-unit.php +++ b/tests/bootstrap-unit.php @@ -213,9 +213,12 @@ function is_realm_allowed($realm) { } } -if (!function_exists('raise_message')) { - function raise_message($id, $text = '', $level = 0) { - } +$GLOBALS['__test_realm_auth'] = true; + +if (!function_exists('api_user_realm_auth')) { + function api_user_realm_auth($filename = '') { + return $GLOBALS['__test_realm_auth']; + } } if (!function_exists('get_request_var')) {