Cacti / cacti

Cacti ™
http://www.cacti.net
GNU General Public License v2.0
1.63k stars 404 forks source link

Index incorrectly changed to 1 if the index is alphanumeric when <source> OID/REGEXP: or OIDVALUE/REGEXP: #2449

Closed tbilan closed 5 years ago

tbilan commented 5 years ago

Bug: When is an OID/REGEXP: or OIDVALUE/REGEXP: and the index is alphanumeric an extra check needs to be made before changing $snmp_index to 1. I'm running version 1.2.1.

Example:

Click to show Data Query output for field 'ExtractRoot'
--
Total: 0.510000, Delta: 0.000000, Located input field 'ExtractRoot' [walk]
Total: 0.660000, Delta: 0.150000, Executing SNMP walk for data @ '.1.3.6.1.4.1.29671.1.1.5.1.3'
Found item [ExtractRoot='.0.24.10.10.169.152'] index: 1 [from regexp oid parse]
Found item [ExtractRoot='.0.24.10.10.169.152'] index: 1 [from regexp oid parse]
Found item [ExtractRoot='.0.24.10.10.169.152'] index: 1 [from regexp oid parse]
Found item [ExtractRoot='.0.24.10.10.169.152'] index: 1 [from regexp oid parse]

  The following code in data_query.php fixes the issue:

1072c1072
<                                               if ($snmp_index == 0) {
---
>                                               if (($snmp_index == 0) && is_numeric($snmp_index)) {
1121c1121
<                                               if ($snmp_index == 0) {
---
>                                               if (($snmp_index == 0) && is_numeric($snmp_index)) {

Example:

Click to show Data Query output for field 'ExtractRoot'
--
Total: 0.500000, Delta: 0.000000, Located input field 'ExtractRoot' [walk]
Total: 0.640000, Delta: 0.140000, Executing SNMP walk for data @ '.1.3.6.1.4.1.29671.1.1.5.1.3'
Found item [ExtractRoot='.0.24.10.10.169.152'] index: .0.24.10.10.169.152.0 [from regexp oid parse]
Found item [ExtractRoot='.0.24.10.10.169.152'] index: .0.24.10.10.169.152.1 [from regexp oid parse]
Found item [ExtractRoot='.0.24.10.10.169.152'] index: .0.24.10.10.169.152.2 [from regexp oid parse]
Found item [ExtractRoot='.0.24.10.10.169.152'] index: .0.24.10.10.169.152.3 [from regexp oid parse]
cigamit commented 5 years ago

Please update to the latest develop version and then re-index your devices. This is a known issue and already resolved in develop. We should be releasing 1.2.2 tomorrow.

tbilan commented 5 years ago

I just reviewed the latest develop code and this and the other issue I brought up isn't resolved in that code. The original code still exists.

netniV commented 5 years ago

The check above makes no sense. If $snmp_index is 0, then it will be true for is_numeric.

php > $snmp_index = 0;
php > echo $snmp_index . ' ' . is_numeric($snmp_index);
0 1
php > $snnp_index = '0';
php > echo $snmp_index . ' ' . is_numeric($snmp_index);
0 1
tbilan commented 5 years ago

Try running that code with $snmp_index = ".1.2.3.4". $snmp_index = 0 is true even though $snmp_index is actually ".1.2.3.4" and "1.2.3.4" is not numeric. It's just how php evaluates the expression.

".1.2.3.4" is a valid index and it's changing it to 1 when it shouldn't. If you check to make sure the index is a number and also check if that number is 0 then it solves the problem.

Check my original output from the verbose query.

netniV commented 5 years ago

Yes, .1.2.3.4 is a valid index.

No, adding is_numeric() will not make a difference to the line of code you have mentioned:

php > $snmp_indexes = array(0, '0',1, '1', .1, '.1','.1.2.3.4'); foreach ($snmp_indexes as $snmp_index) { printf("Index: %10s, Equals Zero: %1d, Is Numeric: %1d%s", $snmp_index, ($snmp_index == 0), is_numeric($snmp_index), PHP_EOL); };
Index:          0, Equals Zero: 1, Is Numeric: 1
Index:          0, Equals Zero: 1, Is Numeric: 1
Index:          1, Equals Zero: 0, Is Numeric: 1
Index:          1, Equals Zero: 0, Is Numeric: 1
Index:        0.1, Equals Zero: 0, Is Numeric: 1
Index:         .1, Equals Zero: 0, Is Numeric: 1
Index:   .1.2.3.4, Equals Zero: 0, Is Numeric: 0
tbilan commented 5 years ago

Try it with a larger string and it fails thus the safety is_numeric check is helpful.

[root@localhost ~]# cat a.php
#!/usr/bin/php -q
<?php

$snmp_indexes = array(0, '0',1, '1', .1, '.1', '1.2.3.4', '.0.24.10.10.169.152.0'); 
foreach ($snmp_indexes as $snmp_index) 
   { printf("Index: %25s, Equals Zero: %1d, Is Numeric: %1d%s", $snmp_index, ($snmp_index == 0), is_numeric($snmp_index), PHP_EOL); };

[root@localhost ~]# php a.php
Index:                         0, Equals Zero: 1, Is Numeric: 1
Index:                         0, Equals Zero: 1, Is Numeric: 1
Index:                         1, Equals Zero: 0, Is Numeric: 1
Index:                         1, Equals Zero: 0, Is Numeric: 1
Index:                       0.1, Equals Zero: 0, Is Numeric: 1
Index:                        .1, Equals Zero: 0, Is Numeric: 1
Index:                   1.2.3.4, Equals Zero: 0, Is Numeric: 0
Index:     .0.24.10.10.169.152.0, Equals Zero: 1, Is Numeric: 0
[root@localhost ~]# 
netniV commented 5 years ago

Then that is a bug with PHP. What PHP version are you running?

netniV commented 5 years ago

Strike that, I did a search and found that PHP will indeed make the stupid comparison of a non-numeric string to be zero. And if we attempt to use the === operator then it will fail again because it could be a string zero.

I really don't understand why your longer version makes it zero when the other invalid strings don't. 1.2.3.4 should have equalled zero too. In fact, running:

<?php
function f($x='surprise')
    {
    if ($x == 'surprise')
        return 'A:'.$x;
    if ($x === 'surprise')
        return 'B:'.$x;
    return 'C:'.$x;
    }

echo "\n".f(0); // A:0 !!
echo "\n".f(0.0); // A:0 !!
echo "\n".f(NULL); // C: !!
echo "\n".f(FALSE); // C:
echo "\n".f(); // A:surprise
echo "\n".f(''); // C:
echo "\n".f((integer)0); // A:0
echo "\n".f((string)0); // C:0
echo "\n".f('0'); // C:0
echo "\n".f(0.1); // C:0.1
echo "\n".f(array()); // C:Array
echo "\n".f('surprise'); // A:surprise
?>

Produces the results:

php -q /tmp/t.php

A:0
A:0
C:
C:
A:surprise
C:
A:0
C:0
C:0
C:0.1PHP Notice:  Array to string conversion in /tmp/t.php on line 8

C:Array
A:surprise

And here is another crazy situation which I hadn't seen before:

<?php
$x = "1";
$y = "+1";
var_dump($x == $x);
?>

Produces

php /tmp/t1.php
bool(true)

These were all code examples found on http://php.net/manual/en/types.comparisons.php

cigamit commented 5 years ago

@tbilan, Please upload your:

1) resource xml file, 2) your Device Template, 3) raw snmpwalks of all the items in the resource xml file using the -On option 4) Your PHP version and whether or not php-snmp is enabled 5) What version of Cacti this last worked in

This will help us with a few things:

1) Reproducing your problem 2) Prevent it from happening again

Make sure that you walk the items using '-On' so that we get numeric OID's. Lastly, check the resource xml for 'index_order_type', ensure that it's alphanumeric. Seeing the comment block around this piece of code complaining about EMC Cellera, I have a hunch this hack may be quire wrong.

cigamit commented 5 years ago

One more thing, now that I'm opening this can of worms. Can you, before the $snmp_index == 0 check place a log entry as follows:

cacti_log('The snmp_index was "' . $snmp_index . '"');

Post the output from the log. This is actually of higher importance than my former request, but please do answer that as well.

tbilan commented 5 years ago

snmpwalk.txt

PHP 5.4.16 (cli) (built: Oct 30 2018 19:30:51) Copyright (c) 1997-2013 The PHP Group Zend Engine v2.4.0, Copyright (c) 1998-2013 Zend Technologies

2019/02/27 19:49:51 - CMDPHP The snmp_index was ".0.24.10.135.91.90.0" 2019/02/27 19:49:51 - CMDPHP The snmp_index was ".0.24.10.59.225.198.5" 2019/02/27 19:49:51 - CMDPHP The snmp_index was ".0.24.10.59.225.198.4" 2019/02/27 19:49:51 - CMDPHP The snmp_index was ".0.24.10.59.225.198.3" 2019/02/27 19:49:51 - CMDPHP The snmp_index was ".0.24.10.59.225.198.2" 2019/02/27 19:49:51 - CMDPHP The snmp_index was ".0.24.10.59.225.198.1" 2019/02/27 19:49:51 - CMDPHP The snmp_index was ".0.24.10.59.225.198.0" 2019/02/27 19:49:51 - CMDPHP The snmp_index was ".0.24.10.10.169.152.6" 2019/02/27 19:49:51 - CMDPHP The snmp_index was ".0.24.10.10.169.152.5" 2019/02/27 19:49:51 - CMDPHP The snmp_index was ".0.24.10.10.169.152.4" 2019/02/27 19:49:51 - CMDPHP The snmp_index was ".0.24.10.10.169.152.3" 2019/02/27 19:49:51 - CMDPHP The snmp_index was ".0.24.10.10.169.152.2" 2019/02/27 19:49:51 - CMDPHP The snmp_index was ".0.24.10.10.169.152.1" 2019/02/27 19:49:51 - CMDPHP The snmp_index was ".0.24.10.10.169.152.0"

[root@localhost lib]# php -m | grep snmp snmp [root@localhost lib]#

This is a new device so I'm just building the snmp_query from the ground up and learning as I go. I hope my analysis and suggestion helps the community somehow.

tbilan commented 5 years ago

meraki.xml.txt

cigamit commented 5 years ago

Did you add those Cacti.log statements as I asked? What is the output. Also, you need to upload the Device Template from the Import/Export menu.

So, do these three things: 1) add the Cacti log statement, and reindex. Send me the output of the Cacti.log 2) remove the checks in the file that were there fore EMC Cellera (way out of service these days), re-index and see if it's fixed 3) Upload your Device Template export here.

tbilan commented 5 years ago

I did add the code and posted as asked. It's pasted in my post below the php version. Look above.

Yes, removing the part where it changes the snmp_index to 1 definitely fixes the problem. That is an easier solution than coding around the Celera. I really don't care which route is chosen now that it works.

I've tried using the Cisco Router device template or having no template at all selected therefore there is nothing to upload.

I don't have a single graph working yet for this device because even though my fixes to data_query.xml and my snmp_query get me past the verbose query successfully I get this error in my log and I'm trying to hunt down the php code that generates it so I can debug it. Other devices poll just fine when they're enabled. tcpdumping snmp shows only the query for uptime and the device is showing as up. 2019/02/27 21:50:02 - POLLER: Poller[Main Poller] WARNING: Invalid Response(s), Errors[1] Device[Meraki Cloud] Thread[1] DS[Meraki Cloud - Traffic] Graphs[wan1]

It's been a pain trying to get this complex snmp_query working. It's now on the level of personal challenge.

netniV commented 5 years ago

I think the problem here is that if Cacti thinks that it is a numeric index, it expects that index to be positive (>0). However, with alpha numeric indexes, that isn't possible since it's not a single value index but an entire string.

That gets complicated by the fact that PHP is dumb when it comes to comparing a string to an integer. It effectively calls intval() and that says well there's no number so it's zero. When we are using that in an IF statement, it breaks common sense logic because

if (".0.24.10.10.169.152.0" == 0) {

actually becomes

if (0 == 0) {

What I'm not sure about is why we are forcing the index to be 1? Which scenario requires that?

tbilan commented 5 years ago

There is a little comment in the code saying that the scenario is that it fixes an EMC Celerra bug. Since the Celerra came out in 1996 it may be safe now to just remove that little piece of logic and call it a day. Plan B could be to implement my is_numeric check. Whatever you guys think is best.

cigamit commented 5 years ago

I've taken that crap out. Done fixed, move on. Thanks for sticking with this one. You and others that help us work through the crazy world of snmp agents make the tool a better one.

cigamit commented 5 years ago

Close if this change is acceptable.