From 06701870cdc6d5e025b0f0b3f76b023f5df946f4 Mon Sep 17 00:00:00 2001 From: Tony Murray Date: Wed, 10 Nov 2021 20:49:06 -0600 Subject: [PATCH] Change how options are handled for SnmpQuery (#13488) * Change how options are handled for SnmpQuery Fix added backslashes when collecting snmp data. * fix lint --- LibreNMS/Data/Source/NetSnmpQuery.php | 19 +++++++++---------- LibreNMS/Modules/Core.php | 4 ++-- LibreNMS/Util/ModuleTestHelper.php | 26 ++++++++++++++++---------- phpstan-baseline.neon | 10 ---------- tests/Unit/SnmpResponseTest.php | 4 ++++ 5 files changed, 31 insertions(+), 32 deletions(-) diff --git a/LibreNMS/Data/Source/NetSnmpQuery.php b/LibreNMS/Data/Source/NetSnmpQuery.php index 46d534b13c..83e8880f12 100644 --- a/LibreNMS/Data/Source/NetSnmpQuery.php +++ b/LibreNMS/Data/Source/NetSnmpQuery.php @@ -80,11 +80,7 @@ class NetSnmpQuery implements SnmpQueryInterface /** * @var array|string */ - private $options = []; - /** - * @var string[] - */ - private $defaultOptions = ['-OQXUte']; + private $options = ['-OQXUte']; /** * @var \App\Models\Device */ @@ -164,17 +160,20 @@ class NetSnmpQuery implements SnmpQueryInterface } /** - * Set option(s) for net-snmp command line. + * Set option(s) for net-snmp command line. Overrides the default options. * Some options may break parsing, but you can manually parse the raw output if needed. - * This will override other options set such as setting numeric. Call with no options to reset to default. + * This will override other options set such as setting numeric. + * Calling with null will reset to the default options (-OQXUte). * Try to avoid setting options this way to keep the API generic. * - * @param array|string $options + * @param array|string|null $options * @return $this */ public function options($options = []): SnmpQueryInterface { - $this->options = Arr::wrap($options); + $this->options = $options !== null + ? Arr::wrap($options) + : ['-OQXUte']; return $this; } @@ -241,7 +240,7 @@ class NetSnmpQuery implements SnmpQueryInterface // authentication $this->buildAuth($cmd); - $cmd = array_merge($cmd, $this->defaultOptions, $this->options); + $cmd = array_merge($cmd, $this->options); $timeout = $this->device->timeout ?? Config::get('snmp.timeout'); if ($timeout && $timeout !== 1) { diff --git a/LibreNMS/Modules/Core.php b/LibreNMS/Modules/Core.php index 7d817aed41..3608e6f51e 100644 --- a/LibreNMS/Modules/Core.php +++ b/LibreNMS/Modules/Core.php @@ -195,7 +195,7 @@ class Core implements Module } } elseif ($key == 'snmpget') { $get_value = SnmpQuery::device($device) - ->options($value['options'] ?? []) + ->options($value['options'] ?? null) ->mibDir($value['mib_dir'] ?? $mibdir) ->get(isset($value['mib']) ? "{$value['mib']}::{$value['oid']}" : $value['oid']) ->value(); @@ -204,7 +204,7 @@ class Core implements Module } } elseif ($key == 'snmpwalk') { $walk_value = SnmpQuery::device($device) - ->options($value['options'] ?? []) + ->options($value['options'] ?? null) ->mibDir($value['mib_dir'] ?? $mibdir) ->walk(isset($value['mib']) ? "{$value['mib']}::{$value['oid']}" : $value['oid']) ->raw(); diff --git a/LibreNMS/Util/ModuleTestHelper.php b/LibreNMS/Util/ModuleTestHelper.php index 172eed10fc..4d00aab22c 100644 --- a/LibreNMS/Util/ModuleTestHelper.php +++ b/LibreNMS/Util/ModuleTestHelper.php @@ -30,6 +30,7 @@ use Illuminate\Support\Arr; use Illuminate\Support\Str; use LibreNMS\Component; use LibreNMS\Config; +use LibreNMS\Data\Source\SnmpResponse; use LibreNMS\Exceptions\FileNotFoundException; use LibreNMS\Exceptions\InvalidModuleException; use Symfony\Component\Yaml\Yaml; @@ -157,23 +158,22 @@ class ModuleTestHelper $snmp_oids = $this->collectOids($device_id); } - $device = device_by_id_cache($device_id, true); DeviceCache::setPrimary($device_id); $snmprec_data = []; foreach ($snmp_oids as $oid_data) { $this->qPrint(' ' . $oid_data['oid']); - $snmp_options = ['-OUneb', '-Ih']; + $snmp_options = ['-OUneb', '-Ih', '-m', '+' . $oid_data['mib']]; if ($oid_data['method'] == 'walk') { - $data = snmp_walk($device, $oid_data['oid'], $snmp_options, $oid_data['mib'], $oid_data['mibdir']); + $data = \SnmpQuery::options($snmp_options)->mibDir($oid_data['mibdir'])->walk($oid_data['oid']); } elseif ($oid_data['method'] == 'get') { - $data = snmp_get($device, $oid_data['oid'], $snmp_options, $oid_data['mib'], $oid_data['mibdir']); + $data = \SnmpQuery::options($snmp_options)->mibDir($oid_data['mibdir'])->get($oid_data['oid']); } elseif ($oid_data['method'] == 'getnext') { - $data = snmp_getnext($device, $oid_data['oid'], $snmp_options, $oid_data['mib'], $oid_data['mibdir']); + $data = \SnmpQuery::options($snmp_options)->mibDir($oid_data['mibdir'])->next($oid_data['oid']); } - if (isset($data) && $data !== false) { + if (isset($data) && $data->isValid()) { $snmprec_data[] = $this->convertSnmpToSnmprec($data); } } @@ -368,10 +368,10 @@ class ModuleTestHelper } } - private function convertSnmpToSnmprec($snmp_data) + private function convertSnmpToSnmprec(SnmpResponse $snmp_data): array { $result = []; - foreach (explode(PHP_EOL, $snmp_data) as $line) { + foreach (explode(PHP_EOL, $snmp_data->raw()) as $line) { if (empty($line)) { continue; } @@ -381,7 +381,7 @@ class ModuleTestHelper $oid = ltrim($oid, '.'); $raw_data = trim($raw_data); - if (empty($raw_data)) { + if (empty($raw_data) || $raw_data == '""') { $result[] = "$oid|4|"; // empty data, we don't know type, put string } else { [$raw_type, $data] = explode(':', $raw_data, 2); @@ -389,9 +389,15 @@ class ModuleTestHelper // device returned the wrong type, save the wrong type to emulate the device behavior [$raw_type, $data] = explode(':', ltrim($data), 2); } - $data = ltrim($data, ' "'); + $type = $this->getSnmprecType($raw_type); + $data = ltrim($data, ' '); + if (Str::startsWith($data, '"') && Str::endsWith($data, '"')) { + // raw string surrounded by quotes, strip extra escapes + $data = stripslashes(substr($data, 1, -1)); + } + if ($type == '6') { // remove leading . from oid data $data = ltrim($data, '.'); diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 332838691f..9bd99048dd 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -6210,16 +6210,6 @@ parameters: count: 1 path: LibreNMS/Util/ModuleTestHelper.php - - - message: "#^Method LibreNMS\\\\Util\\\\ModuleTestHelper\\:\\:convertSnmpToSnmprec\\(\\) has no return type specified\\.$#" - count: 1 - path: LibreNMS/Util/ModuleTestHelper.php - - - - message: "#^Method LibreNMS\\\\Util\\\\ModuleTestHelper\\:\\:convertSnmpToSnmprec\\(\\) has parameter \\$snmp_data with no type specified\\.$#" - count: 1 - path: LibreNMS/Util/ModuleTestHelper.php - - message: "#^Method LibreNMS\\\\Util\\\\ModuleTestHelper\\:\\:dataIsEmpty\\(\\) has no return type specified\\.$#" count: 1 diff --git a/tests/Unit/SnmpResponseTest.php b/tests/Unit/SnmpResponseTest.php index ecae88747b..496be02dc9 100644 --- a/tests/Unit/SnmpResponseTest.php +++ b/tests/Unit/SnmpResponseTest.php @@ -179,6 +179,10 @@ HOST-RESOURCES-MIB::hrStorageUsed.36 = 127044934 $this->assertTrue($response->isValid()); $this->assertEquals('4958', $response->value()); $this->assertEquals(['.1.3.6.1.2.1.2.2.1.10.1' => '4958', '.1.3.6.1.2.1.2.2.1.10.2' => '349'], $response->values()); + + $response = new SnmpResponse(".1.3.6.1.2.1.31.1.1.1.18.1 = \"internal\\\\backslash\"\n"); + $this->assertTrue($response->isValid()); + $this->assertEquals('internal\\backslash', $response->value()); } public function testErrorHandling(): void