cacti vs PHP 7 vs split

[Joel N. Weber II]

[Impact]

 * If the administrator configures cacti to poll data at a higher frequency
than the cron job that runs the poller, a fatal error is emitted.

 * As the highest cron frequency is once per minute, one will have to use this feature if higher resolution is required.

[Test Case]

 * On the one hand, this is a syntactical (non-php7.0 compliant) code change, so the test case feels less necessary.

 * On the other hand, one can configure cacti to poll at a higher than once-per-minute frequency, and the poller will crash with the "undefined function split()".

[Regression Potential]

 * Presuming an administrator does not use a higher than one-minute polling frequency, there is no regression potential, as they are unaffected by this codepath.

 * If the administrator does use a higher polling frequency, they may have already hit this bug and manually changed the source on their system. Ideally, they will have changed it in the same way upstream has, which is the backported fix in this bug -- and so when the update is installed, there should be no change.

 * If the administrator left cacti in the broken state because of this bug, then it will be fixed by this change and there again, is no regression potential, because the feature was broken before.

---

poller.php calls split in two places, which fails with PHP 7. This seems to only actually cause problems if poller.php is configured to poll the snmp data sources more than once per time poller.php is called by cron (which typically happens if 10 second or 30 second polling intervals are requested), but it appears to me that these error messages may appear in the logs even when cacti is configured to only poll once per time it is called by cron.

It appears that using explode instead of split is the fix.

PHP Fatal error: Uncaught Error: Call to undefined function split() in /usr/share/cacti/site/poller.php:454
Stack trace:
#0 {main}
  thrown in /usr/share/cacti/site/poller.php on line 454
PHP Fatal error: Uncaught Error: Call to undefined function split() in /usr/share/cacti/site/poller.php:454
Stack trace:
#0 {main}
  thrown in /usr/share/cacti/site/poller.php on line 454

Description: Ubuntu 16.04.1 LTS
Release: 16.04
cacti:
  Installed: 0.8.8f+ds1-4ubuntu4.16.04.1
  Candidate: 0.8.8f+ds1-4ubuntu4.16.04.1
  Version table:
 *** 0.8.8f+ds1-4ubuntu4.16.04.1 500
        500 http://us.archive.ubuntu.com/ubuntu xenial-updates/universe amd64 Packages
        500 http://us.archive.ubuntu.com/ubuntu xenial-updates/universe i386 Packages
        100 /var/lib/dpkg/status
     0.8.8f+ds1-4ubuntu4 500
        500 http://us.archive.ubuntu.com/ubuntu xenial/universe amd64 Packages
        500 http://us.archive.ubuntu.com/ubuntu xenial/universe i386 Packages

[Paul Gevers (paul-climbing)]

Thanks for reporting this issue.

The code was introduced to fix issue http://bugs.cacti.net/view.php?id=2446 in 0.8.8c.

Looks like already before that time Debian and Ubuntu replaced split() by explode(), so that's why this one was missed.

Code is not present in the recently released upstream version 1.0.0, so this bug will be fixed once 1.0.0 or higher is packaged for Debian and synced to Ubuntu.

[Nish Aravamudan (nacc)]

@Paul,

I do see at least 6b5931130770cdfa5baa68c43a913dcabf1c3970 upstream that fixes the split() calls in question. Is it suitable to just make those changes in the versions in Ubuntu. Specifically:

--- cacti-0.8.8f+ds1.orig/poller.php
+++ cacti-0.8.8f+ds1/poller.php
@@ -451,11 +451,9 @@ while ($poller_runs_completed < $poller_

                /* sleep the appripriate amount of time */
                if ($poller_runs_completed < $poller_runs) {
- list($micro, $seconds) = split(' ', microtime());
- $plugin_start = $seconds + $micro;
+ $plugin_start = microtime(true);
                        api_plugin_hook('poller_bottom');
- list($micro, $seconds) = split(' ', microtime());
- $plugin_end = $seconds + $micro;
+ $plugin_end = microtime(true);
                        if (($sleep_time - ($plugin_end - $plugin_start)) > 0) {
                                usleep(($sleep_time - ($plugin_end - $plugin_start)) * 1000000);
                        }

Probably adjusted a bit for the various versions in the releases? Or are more fixes from upstream needed?

[Paul Gevers (paul-climbing)]

Ouch, I forgot about this issue.

@Nish: It looks like that indeed. Are you asking the last questions for a specific reason? It is the first time I see somebody ask me such a question from the Ubuntu side.

I must admit that I underestimated the impact of this bug, please go ahead.

[Nish Aravamudan (nacc)]

@Paul: I was asking mostly because the upstream changes don't necessarily apply cleanly (the whole commit) because there seems to have been a global move to microtime() in the source. Is that something I should look at SRUing with this change? Or is it sufficient, in your opinion, to just backport the diff in c#2 as a fix for this particular issue?

[Paul Gevers (paul-climbing)]

Hmm, I must also admit that I don't fully understand what this feature is for (it doesn't seem to do what I expected).

Anybody that does now, can you explain why this should be fixed in older releases?

[Paul Gevers (paul-climbing)]

@Nish, if you want to fix this particular issue, I don't think you need to fix the other uses of explode. I think you could also use explode instead of split.

[Paul Gevers (paul-climbing)]

Sleeping on it another night makes sense. I understand what I was doing wrong yesterday, I forgot to change the rrd file definition when changing the poller interval.

Indeed, if someone wants to increase the frequency to something higher than once per minute, cacti fails to support this currently due to this bug.

I'll file a bug in Debian as well and will try to get a release exception.

[Paul Gevers (paul-climbing)]

Bug is now fixed in Debian and unblocked for the upcoming Stretch release.

[Nish Aravamudan (nacc)]

I will upload the same fix to Ubuntu tonight or tomorrow. Thanks Paul!

[Nish Aravamudan (nacc)]

Once we are able to sync the package from Debian in AA, Ubuntu will have the fix and we can SRU.

[Nish Aravamudan (nacc)]

I'm uploading the fix for x, y and z now. We can sync in aa once it's open.

[Adam Conrad (adconrad)]

Hello Joel, or anyone else affected,

Accepted cacti into zesty-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/cacti/0.8.8h+ds1-8ubuntu0.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed.Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Paul Gevers (paul-climbing)]

@Nish, why didn't you (fake-) synced cacti in Zesty, there were no other delta's?

[Nish Aravamudan (nacc)]

@Paul, because I didn't think of that at the time :) Sorry!

[Adam Conrad (adconrad)]

Hello Joel, or anyone else affected,

Accepted cacti into yakkety-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/cacti/0.8.8h+ds1-5ubuntu0.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed.Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Adam Conrad (adconrad)]

Hello Joel, or anyone else affected,

Accepted cacti into xenial-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/cacti/0.8.8f+ds1-4ubuntu4.16.04.3 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed.Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

[Launchpad Janitor (janitor)]

This bug was fixed in the package cacti - 0.8.8h+ds1-8ubuntu0.1

---------------
cacti (0.8.8h+ds1-8ubuntu0.1) zesty; urgency=medium

  * debian/patches/enable_faster_polling_than_cron.patch: split() is
    deprecated in PHP 7, causing the code to fail. Closes: #860271,
    LP: #1662027. Thanks to Paul Gevers <email address hidden>.

 -- Nishanth Aravamudan <email address hidden> Fri, 14 Apr 2017 21:26:51 -0700

[Łukasz Zemczak (sil2100)]

As part of a recent change in the Stable Release Update verification policy we would like to inform that for a bug to be considered verified for a given release a verification-done-$RELEASE tag needs to be added to the bug where $RELEASE is the name of the series the package that was tested (e.g. verification-done-xenial). Please note that the global 'verification-done' tag can no longer be used for this purpose.

Thank you!

[Ubuntu Foundations Team Bug Bot (crichton)]

The fix for this bug has been awaiting testing feedback in the -proposed repository for zesty for more than 90 days. Please test this fix and update the bug appropriately with the results. In the event that the fix for this bug is still not verified 15 days from now, the package will be removed from the -proposed repository.

[Paul Gevers (paul-climbing)]

To all those involved. I am not in the position to test Ubuntu packages as I don't run that myself and my computer doesn't support hardware virtualization.

[Łukasz Zemczak (sil2100)]

The version of cacti in the proposed pocket of Xenial that was purported to fix this bug report has been removed because the bugs that were to be fixed by the upload were not verified in a timely (105 days) fashion.