Source: net-snmp
Source-Version: 5.9+dfsg-3
Severity: wishlist
Tags: patch
Hi!
The code is currently very strict, and does not accept several internal
perl representations for integer scalars. This is a problem when writing
SNMP agents in perl as the values might come from external sources and
might be represented internally in multiple ways, which might change
over the course of the execution.
In addition, improve the logging output to make it easier to diagnose why
a scalar variable could not be set.
These two patches were merged upstream:
https://github.com/net-snmp/net-snmp/pull/153
but I don't expect a new release soon. And it would be nice to merge
these in Debian before the bullseye release. This would be helpful as
these are the only two patches we need a fork for at $work. :)
I'm attaching a patch adding these two patches.
Thanks,
Guillem
diff --git c/debian/patches/0001-Perl-agent-Print-the-perl-scalar-flags-on-type-misma.patch i/debian/patches/0001-Perl-agent-Print-the-perl-scalar-flags-on-type-misma.patch
new file mode 100644
index 0000000..c4b4a33
--- /dev/null
+++ i/debian/patches/0001-Perl-agent-Print-the-perl-scalar-flags-on-type-misma.patch
@@ -0,0 +1,85 @@
+From d59ae433af8b6d9051d149ca2a745fc845e7c82d Mon Sep 17 00:00:00 2001
+From: Guillem Jover <gjo...@sipwise.com>
+Date: Fri, 22 Apr 2016 15:45:07 +0200
+Subject: [PATCH 1/2] Perl, agent: Print the perl scalar flags on type mismatch
+ in setValue
+
+This makes it easier to debug why a type mismatch happened, as we can
+see how the type is represented internally by perl.
+---
+ perl/agent/agent.xs | 24 ++++++++++++------------
+ 1 file changed, 12 insertions(+), 12 deletions(-)
+
+diff --git a/perl/agent/agent.xs b/perl/agent/agent.xs
+index 163a3ba81a..ed8a56c9fb 100644
+--- a/perl/agent/agent.xs
++++ b/perl/agent/agent.xs
+@@ -646,8 +646,8 @@ nari_setValue(me, type, value)
+ break;
+ }
+ else {
+- snmp_log(LOG_ERR, "Non-integer value passed to setValue with ASN_INTEGER: type was %lu\n",
+- (unsigned long)SvTYPE(value));
++ snmp_log(LOG_ERR, "Non-integer value passed to setValue with ASN_INTEGER: type was %lu flags %#lx\n",
++ (unsigned long)SvTYPE(value), (unsigned long)SvFLAGS(value));
+ RETVAL = 0;
+ break;
+ }
+@@ -682,8 +682,8 @@ nari_setValue(me, type, value)
+ break;
+ }
+ else {
+- snmp_log(LOG_ERR, "Non-unsigned-integer value passed to setValue with ASN_UNSIGNED/ASN_COUNTER/ASN_TIMETICKS: type was %lu\n",
+- (unsigned long)SvTYPE(value));
++ snmp_log(LOG_ERR, "Non-unsigned-integer value passed to setValue with ASN_UNSIGNED/ASN_COUNTER/ASN_TIMETICKS: type was %lu flags %#lx\n",
++ (unsigned long)SvTYPE(value), (unsigned long)SvFLAGS(value));
+ RETVAL = 0;
+ break;
+ }
+@@ -708,8 +708,8 @@ nari_setValue(me, type, value)
+ RETVAL = 1;
+ }
+ else {
+- snmp_log(LOG_ERR, "Non-unsigned-integer value passed to setValue with ASN_COUNTER64: type was %lu\n",
+- (unsigned long)SvTYPE(value));
++ snmp_log(LOG_ERR, "Non-unsigned-integer value passed to setValue with ASN_COUNTER64: type was %lu flags %#lx\n",
++ (unsigned long)SvTYPE(value), (unsigned long)SvFLAGS(value));
+ RETVAL = 0;
+ }
+ if (RETVAL) {
+@@ -725,8 +725,8 @@ nari_setValue(me, type, value)
+ case ASN_OPAQUE:
+ /* Check that we have been passed something with a string value (or a blessed scalar) */
+ if (!SvPOKp(value) && (SvTYPE(value) != SVt_PVMG)) {
+- snmp_log(LOG_ERR, "Non-string value passed to setValue with ASN_OCTET_STR/ASN_BIT_STR: type was %lu\n",
+- (unsigned long)SvTYPE(value));
++ snmp_log(LOG_ERR, "Non-string value passed to setValue with ASN_OCTET_STR/ASN_BIT_STR: type was %lu flags %#lx\n",
++ (unsigned long)SvTYPE(value), (unsigned long)SvFLAGS(value));
+ RETVAL = 0;
+ break;
+ }
+@@ -752,8 +752,8 @@ nari_setValue(me, type, value)
+
+ /* Check that we have been passed something with a string value (or a blessed scalar) */
+ if (!SvPOKp(value) && (SvTYPE(value) != SVt_PVMG)) {
+- snmp_log(LOG_ERR, "Non-string value passed to setValue with ASN_IPADDRESS: type was %lu\n",
+- (unsigned long)SvTYPE(value));
++ snmp_log(LOG_ERR, "Non-string value passed to setValue with ASN_IPADDRESS: type was %lu flags %#lx\n",
++ (unsigned long)SvTYPE(value), (unsigned long)SvFLAGS(value));
+ RETVAL = 0;
+ break;
+ }
+@@ -779,8 +779,8 @@ nari_setValue(me, type, value)
+ case ASN_OBJECT_ID:
+ /* Check that we have been passed something with a string value (or a blessed scalar) */
+ if (!SvPOKp(value) && (SvTYPE(value) != SVt_PVMG)) {
+- snmp_log(LOG_ERR, "Non-string value passed to setValue with ASN_OBJECT_ID: type was %lu\n",
+- (unsigned long)SvTYPE(value));
++ snmp_log(LOG_ERR, "Non-string value passed to setValue with ASN_OBJECT_ID: type was %lu flags %#lx\n",
++ (unsigned long)SvTYPE(value), (unsigned long)SvFLAGS(value));
+ RETVAL = 0;
+ break;
+ }
+--
+2.29.2
+
diff --git c/debian/patches/0002-Perl-agent-Make-the-perl-agent-code-more-tolerant-of.patch i/debian/patches/0002-Perl-agent-Make-the-perl-agent-code-more-tolerant-of.patch
new file mode 100644
index 0000000..1169ed6
--- /dev/null
+++ i/debian/patches/0002-Perl-agent-Make-the-perl-agent-code-more-tolerant-of.patch
@@ -0,0 +1,127 @@
+From 5adc5bbcec46eacf654ed92ed2431d0965076fa9 Mon Sep 17 00:00:00 2001
+From: Guillem Jover <gjo...@sipwise.com>
+Date: Fri, 22 Apr 2016 15:45:07 +0200
+Subject: [PATCH 2/2] Perl, agent: Make the perl agent code more tolerant of
+ perl types
+
+The code is currently very strict, and does not accept several internal
+perl representations for integer scalars.
+
+First we should use SvPOK instead of SvPOKp to accept anything that
+looks like a string. And then we should also accept floating point
+numbers because they might store integer or unsigned values.
+---
+ perl/agent/agent.xs | 55 ++++++++++++++++++++++++++++++++++++++++-----
+ 1 file changed, 49 insertions(+), 6 deletions(-)
+
+diff --git a/perl/agent/agent.xs b/perl/agent/agent.xs
+index ed8a56c9fb..06f878e718 100644
+--- a/perl/agent/agent.xs
++++ b/perl/agent/agent.xs
+@@ -598,6 +598,7 @@ nari_setValue(me, type, value)
+ netsnmp_request_info *request;
+ u_long utmp;
+ long ltmp;
++ double dtmp;
+ uint64_t ulltmp;
+ struct counter64 c64;
+ oid myoid[MAX_OID_LEN];
+@@ -630,7 +631,21 @@ nari_setValue(me, type, value)
+ RETVAL = 1;
+ break;
+ }
+- else if (SvPOKp(value)) {
++ else if (SvTYPE(value) == SVt_NV || SvNOK(value)) {
++ /* Might be ok - got a double that might be an actual integer */
++ dtmp = SvNVX(value);
++ ltmp = SvIV(value);
++ if (dtmp != ltmp) {
++ snmp_log(LOG_ERR, "Could not convert double to integer in setValue: '%f'", dtmp);
++ RETVAL = 0;
++ break;
++ }
++ snmp_set_var_typed_value(request->requestvb, (u_char)type,
++ (u_char *) <mp, sizeof(ltmp));
++ RETVAL = 1;
++ break;
++ }
++ else if (SvPOK(value)) {
+ /* Might be OK - got a string, so try to convert it, allowing base 10, octal, and hex forms */
+ stringptr = SvPV(value, stringlen);
+ ltmp = strtol( stringptr, NULL, 0 );
+@@ -666,7 +681,21 @@ nari_setValue(me, type, value)
+ RETVAL = 1;
+ break;
+ }
+- else if (SvPOKp(value)) {
++ else if (SvTYPE(value) == SVt_NV || SvNOK(value)) {
++ /* Might be ok - got a double that might be an actual unsigned */
++ dtmp = SvNVX(value);
++ utmp = SvIV(value);
++ if (dtmp != utmp) {
++ snmp_log(LOG_ERR, "Could not convert double to unsigned in setValue: '%f'", dtmp);
++ RETVAL = 0;
++ break;
++ }
++ snmp_set_var_typed_value(request->requestvb, (u_char)type,
++ (u_char *) &utmp, sizeof(utmp));
++ RETVAL = 1;
++ break;
++ }
++ else if (SvPOK(value)) {
+ /* Might be OK - got a string, so try to convert it, allowing base 10, octal, and hex forms */
+ stringptr = SvPV(value, stringlen);
+ utmp = strtoul( stringptr, NULL, 0 );
+@@ -695,7 +724,21 @@ nari_setValue(me, type, value)
+ ulltmp = SvIV(value);
+ RETVAL = 1;
+ }
+- else if (SvPOKp(value)) {
++ else if (SvTYPE(value) == SVt_NV || SvNOK(value)) {
++ /* Might be ok - got a double that might be an actual unsigned */
++ dtmp = SvNVX(value);
++ ulltmp = SvIV(value);
++ if (dtmp != ulltmp) {
++ snmp_log(LOG_ERR, "Could not convert double to unsigned in setValue: '%f'", dtmp);
++ RETVAL = 0;
++ break;
++ }
++ snmp_set_var_typed_value(request->requestvb, (u_char)type,
++ (u_char *) &ulltmp, sizeof(ulltmp));
++ RETVAL = 1;
++ break;
++ }
++ else if (SvPOK(value)) {
+ /* Might be OK - got a string, so try to convert it, allowing base 10, octal, and hex forms */
+ stringptr = SvPV(value, stringlen);
+ errno = 0;
+@@ -724,7 +767,7 @@ nari_setValue(me, type, value)
+ case ASN_BIT_STR:
+ case ASN_OPAQUE:
+ /* Check that we have been passed something with a string value (or a blessed scalar) */
+- if (!SvPOKp(value) && (SvTYPE(value) != SVt_PVMG)) {
++ if (!SvPOK(value) && (SvTYPE(value) != SVt_PVMG)) {
+ snmp_log(LOG_ERR, "Non-string value passed to setValue with ASN_OCTET_STR/ASN_BIT_STR: type was %lu flags %#lx\n",
+ (unsigned long)SvTYPE(value), (unsigned long)SvFLAGS(value));
+ RETVAL = 0;
+@@ -751,7 +794,7 @@ nari_setValue(me, type, value)
+ */
+
+ /* Check that we have been passed something with a string value (or a blessed scalar) */
+- if (!SvPOKp(value) && (SvTYPE(value) != SVt_PVMG)) {
++ if (!SvPOK(value) && (SvTYPE(value) != SVt_PVMG)) {
+ snmp_log(LOG_ERR, "Non-string value passed to setValue with ASN_IPADDRESS: type was %lu flags %#lx\n",
+ (unsigned long)SvTYPE(value), (unsigned long)SvFLAGS(value));
+ RETVAL = 0;
+@@ -778,7 +821,7 @@ nari_setValue(me, type, value)
+
+ case ASN_OBJECT_ID:
+ /* Check that we have been passed something with a string value (or a blessed scalar) */
+- if (!SvPOKp(value) && (SvTYPE(value) != SVt_PVMG)) {
++ if (!SvPOK(value) && (SvTYPE(value) != SVt_PVMG)) {
+ snmp_log(LOG_ERR, "Non-string value passed to setValue with ASN_OBJECT_ID: type was %lu flags %#lx\n",
+ (unsigned long)SvTYPE(value), (unsigned long)SvFLAGS(value));
+ RETVAL = 0;
+--
+2.29.2
+
diff --git c/debian/patches/series i/debian/patches/series
index 13e1907..cd98207 100644
--- c/debian/patches/series
+++ i/debian/patches/series
@@ -37,3 +37,5 @@ snmpd_conf_5_ro_extend
Link-libnetsnmptrapd-against-MYSQL_LIBS.patch
pkgconfig_install_mode
perl_makefile_man3pods
+0001-Perl-agent-Print-the-perl-scalar-flags-on-type-misma.patch
+0002-Perl-agent-Make-the-perl-agent-code-more-tolerant-of.patch
