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

MIB Parser can sometimes cause errors in later PHP versions #4971

Closed
paulgevers opened this issue Oct 22, 2022 · 11 comments
Closed

MIB Parser can sometimes cause errors in later PHP versions #4971

paulgevers opened this issue Oct 22, 2022 · 11 comments
Assignees
Labels
3rd Party Change Something that Cacti can't fix directly bug Undesired behaviour confirmed Bug is confirm by dev team PHP8 Issue related to PHP8 resolved A fixed issue

Comments

@paulgevers
Copy link
Contributor

Describe the bug

Debian is working on upgrading to php 8.2. Today, I received bug 1022229 in Debian about failing tests with that version of php.

To Reproduce

  1. install php 8.2

  2. Run the test suite

  3. Notice the following (copied from here:

ERROR: Fail Unexpected output in /usr/share/cacti/site/log/cacti.log:
2022-10-17 12:40:41 - ERROR PHP DEPRECATED: Creation of dynamic property MibParser::$mib is deprecated in file: /usr/share/cacti/site/include/vendor/phpsnmp/mib_parser.php  on line: 336
2022-10-17 12:40:41 - CMDPHP PHP ERROR Backtrace:  (/utilities.php[98]:snmpagent_cache_rebuilt(), /lib/snmpagent.php[643]:snmpagent_cache_install(), /lib/snmpagent.php[611]:MibCache->install(), /lib/mib_cache.php[60]:MibParser->add_mib(), /include/vendor/phpsnmp/mib_parser.php[66]:MibParser->parse_mib(), /include/vendor/phpsnmp/mib_parser.php[336]:CactiErrorHandler())
2022-10-17 12:40:41 - ERROR PHP DEPRECATED: Creation of dynamic property MibParser::$mib is deprecated in file: /usr/share/cacti/site/include/vendor/phpsnmp/mib_parser.php  on line: 336
2022-10-17 12:40:41 - CMDPHP PHP ERROR Backtrace:  (/utilities.php[98]:snmpagent_cache_rebuilt(), /lib/snmpagent.php[643]:snmpagent_cache_install(), /lib/snmpagent.php[612]:MibCache->install(), /lib/mib_cache.php[60]:MibParser->add_mib(), /include/vendor/phpsnmp/mib_parser.php[66]:MibParser->parse_mib(), /include/vendor/phpsnmp/mib_parser.php[336]:CactiErrorHandler())
2022-10-17 12:40:41 - ERROR PHP DEPRECATED: Creation of dynamic property MibParser::$mib is deprecated in file: /usr/share/cacti/site/include/vendor/phpsnmp/mib_parser.php  on line: 336
2022-10-17 12:40:41 - CMDPHP PHP ERROR Backtrace:  (/utilities.php[98]:snmpagent_cache_rebuilt(), /lib/snmpagent.php[643]:snmpagent_cache_install(), /lib/snmpagent.php[613]:MibCache->install(), /lib/mib_cache.php[60]:MibParser->add_mib(), /include/vendor/phpsnmp/mib_parser.php[66]:MibParser->parse_mib(), /include/vendor/phpsnmp/mib_parser.php[336]:CactiErrorHandler())

Expected behavior

The test should pass. More importantly there shouldn't be deprecation warnings in the logs when running cacti.

Desktop (please complete the following information)

  • OS: Debian unstable with php-defaults from experimental

  • wget version 1.21.3-1+b2

@paulgevers paulgevers added bug Undesired behaviour unverified Some days we don't have a clue labels Oct 22, 2022
@TheWitness
Copy link
Member

Thanks @paulgevers! On vacation with the boss. So, it'll take a while. Thanks for taking the initiative!

@TheWitness
Copy link
Member

@browniebraun, can you tackle this one?

@browniebraun browniebraun self-assigned this Oct 25, 2022
@browniebraun
Copy link
Member

I have to install 8.2 first ("still" on 8.1), but it is on my plate. 🫡

@netniV netniV added 3rd Party Change Something that Cacti can't fix directly PHP8 Issue related to PHP8 labels Oct 28, 2022
@netniV
Copy link
Member

netniV commented Oct 28, 2022

There's not much in the 8.2 depreciated list that would concern us I think. The main one is going to be ${var} usage and possibly strtolower()/strtoupper() no longer being locale aware (so we should use the mb_ versions of these functions).

The other big one is the warnings that will be thrown if a class doesn't define the property that is being set prior to setting it. Where previously, you could just define the class:

class myclass {
}

Then do something daft like:

$a = new myclass();
$a->undefined = 'defined';

This will now fail. Not sure if any of our code does that, but the libraries we use may also do that so will likely only show up during usage.

@netniV
Copy link
Member

netniV commented Oct 28, 2022

NOTE: Other string functions such as stripos() etc are also affected by the lack of locale awareness change.

NOTE 2: PHP 8.2 isn't even released yet, it's at RC 5 as at the time of writing this so anyone using that is really cutting their teeth at the moment.

@TheWitness
Copy link
Member

Yea got that thanks Mark. They also deprecated strftime() since it's not locale aware, which blew me away, and the replacement is not strait forward and not a direct replacement. PHP team needs to slow down a smidge and think things through a bit more. The strftime() affects Weathermap, but not Cacti.

@paulgevers
Copy link
Contributor Author

@netniV sorry if I confused you. I didn't want to claim that we have php 8.2 already, I just said that we are working on getting ready. The Debian bookworm freeze is on 2023-01-12, so if php 8.2 wants to stand a chance of getting in before that (after that it will have missed the bookworm release), the maintainers need to work on figuring out what breaks. That's what this bug is about.

@netniV
Copy link
Member

netniV commented Oct 28, 2022

No worries Paul, from a lint perspective we appear to have no issues in the core code, one or two in the plugins which I addressed already, but that doesn't stop runtime errors like the dynamic property blocking that they have now introduced.

I have RC4 from the ppa-qa installed but I've yet to rub it with anger.

@browniebraun
Copy link
Member

Mmmhhh... self::method and parent::method seems to have been deprecated as well.

@TheWitness TheWitness changed the title [1.2.22] Deprecation warnings in log files with php 8.2 Deprecation warnings in log files with php 8.2 associate with mib parser Nov 1, 2022
TheWitness added a commit that referenced this issue Nov 1, 2022
Deprecation warnings in log files with php 8.2 associate with mib parser
@TheWitness
Copy link
Member

@browniebraun, that's a bit of a problem too.

@TheWitness TheWitness added resolved A fixed issue confirmed Bug is confirm by dev team and removed unverified Some days we don't have a clue labels Nov 2, 2022
@TheWitness
Copy link
Member

I did a hack job to lib/database.php instead of extending PDO. It get's the job done and less work.

@netniV netniV changed the title Deprecation warnings in log files with php 8.2 associate with mib parser MIB Parser can sometimes cause errors in later PHP versions Dec 31, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Apr 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
3rd Party Change Something that Cacti can't fix directly bug Undesired behaviour confirmed Bug is confirm by dev team PHP8 Issue related to PHP8 resolved A fixed issue
Projects
None yet
Development

No branches or pull requests

4 participants