Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Whilst upgrading, errors in upgrade scripts prevent properly execution #4284

Closed
mchorsley opened this issue May 28, 2021 · 4 comments
Closed
Labels
bug Undesired behaviour installer Installation issue resolved A fixed issue
Milestone

Comments

@mchorsley
Copy link
Contributor

Describe the bug

When upgrading database to 1.2.17, upgrade script 1_2_17.php fails due to 3 parameters passed to array_key_exists function.

To Reproduce

$ php cli/upgrade_database.php --debug

+++PHP Fatal error:  Uncaught ArgumentCountError: array_key_exists() expects exactly 2 arguments, 3 given in /usr/share/cacti/install/upgrades/1_2_17.php:176
Stack trace:
#0 /usr/share/cacti/install/upgrades/1_2_17.php(176): array_key_exists('aggregate_graph...', Array, true)
#1 /usr/share/cacti/install/upgrades/1_2_17.php(54): database_fix_mediumint_columns()
#2 /var/lib/cacti/cli/upgrade_database.php(129): upgrade_to_1_2_17()
#3 {main}
  thrown in /usr/share/cacti/install/upgrades/1_2_17.php on line 176

Expected behavior

Database upgrade completes without error.

Fix

Update: install/upgrades/1_2_17.php:176
From:
if (!array_key_exists($table, $tables, true)) {
To:
if (!array_key_exists($table, $tables)) {

Update: install/upgrades/1_2_17.php:183
From:
if (array_key_exists($field, $known_columns, true)) {
To:
if (array_key_exists($field, $known_columns)) {

Additional Context

-PHP Version: 8.0.6
-OS: CentOS 8.3.2011

@mchorsley mchorsley added bug Undesired behaviour unverified Some days we don't have a clue labels May 28, 2021
@netniV
Copy link
Member

netniV commented May 28, 2021

I would concur with this finding. The function array_key_exists has always had just two parameters. The fix definitely seems reasonable, will you be supplying a PR ?

mchorsley added a commit to mchorsley/cacti that referenced this issue May 29, 2021
@mchorsley
Copy link
Contributor Author

Submitted a pull request. Hope I've done it right!

@netniV
Copy link
Member

netniV commented May 30, 2021

I added a comment regarding changes to the pull request. Just minor bits to keep things in line with what we ask others to do.

@netniV netniV added confirmed Bug is confirm by dev team installer Installation issue and removed unverified Some days we don't have a clue labels May 30, 2021
@netniV netniV added this to the 1.2.18 milestone May 30, 2021
mchorsley added a commit to mchorsley/cacti that referenced this issue May 30, 2021
@mchorsley
Copy link
Contributor Author

Thanks for your help, I've updated the change log as requested. Hope this is suitable. Please let me know if you have any further suggestions.

mchorsley added a commit to mchorsley/cacti that referenced this issue Jun 3, 2021
netniV added a commit that referenced this issue Jun 5, 2021
…e) (#4285)

* Fixing #4284 - Parameters in array_key_exists

* Fixing issue #4284 - Database upgrade can fail - Uncaught argument count error

* Fixing issue #4284 - Database upgrade can fail - Uncaught argument count error

Co-authored-by: Mark Brugnoli-Vinten <netniv@hotmail.com>
@netniV netniV added resolved A fixed issue and removed confirmed Bug is confirm by dev team labels Jun 9, 2021
@netniV netniV closed this as completed Jun 9, 2021
@netniV netniV changed the title Database upgrade (1.2.16 -> 1.2.17) fails due to array_key_exists parameters Whilst upgrading, errors in upgrade scripts prevent properly execution Jul 4, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Oct 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Undesired behaviour installer Installation issue resolved A fixed issue
Projects
None yet
Development

No branches or pull requests

2 participants