reformat OID parsing procedure,

fail whole SNMP query on single OID parsing failure

3 files changed, 28 insertions, 30 deletions

diff --git a/ext/snmp/snmp.c b/ext/snmp/snmp.c
index c0ab7f5f6a..e2a1b94803 100644
--- a/ext/snmp/snmp.c
+++ b/ext/snmp/snmp.c
@@ -744,24 +744,28 @@ static void php_snmp_internal(INTERNAL_FUNCTION_PARAMETERS, int st,
 
 	while (keepwalking) {
 		keepwalking = 0;
-		if (st & (SNMP_CMD_GET | SNMP_CMD_GETNEXT)) {
-			pdu = snmp_pdu_create((st & SNMP_CMD_GET) ? SNMP_MSG_GET : SNMP_MSG_GETNEXT);
-			for (count = 0; objid_query->offset < objid_query->count && count < objid_query->step; objid_query->offset++, count++){
-				objid_query->vars[objid_query->offset].name_length = MAX_OID_LEN;
-				if (!snmp_parse_oid(objid_query->vars[objid_query->offset].oid, objid_query->vars[objid_query->offset].name, &(objid_query->vars[objid_query->offset].name_length))) {
-					php_error_docref(NULL TSRMLS_CC, E_WARNING, "Invalid object identifier: %s", objid_query->vars[objid_query->offset].oid);
-				} else {
-					snmp_add_null_var(pdu, objid_query->vars[objid_query->offset].name, objid_query->vars[objid_query->offset].name_length);
-				}
+		if (st & SNMP_CMD_WALK) {
+			if (session->version == SNMP_VERSION_1) {
+				pdu = snmp_pdu_create(SNMP_MSG_GETNEXT);
+			} else {
+				pdu = snmp_pdu_create(SNMP_MSG_GETBULK);
+				pdu->non_repeaters = objid_query->non_repeaters;
+				pdu->max_repetitions = objid_query->max_repetitions;
 			}
-			if(pdu->variables == NULL){
-				snmp_free_pdu(pdu);
+			snmp_add_null_var(pdu, name, name_length);
+		} else {
+			if (st & SNMP_CMD_GET) {
+				pdu = snmp_pdu_create(SNMP_MSG_GET);
+			} else if (st & SNMP_CMD_GETNEXT) {
+				pdu = snmp_pdu_create(SNMP_MSG_GETNEXT);
+			} else if (st & SNMP_CMD_SET) {
+				pdu = snmp_pdu_create(SNMP_MSG_SET);
+			} else {
 				snmp_close(ss);
+				php_error_docref(NULL TSRMLS_CC, E_ERROR, "Unknown SNMP command (internals)");
 				RETVAL_FALSE;
 				return;
 			}
-		} else if (st & SNMP_CMD_SET) {
-			pdu = snmp_pdu_create(SNMP_MSG_SET);
 			for (count = 0; objid_query->offset < objid_query->count && count < objid_query->step; objid_query->offset++, count++){
 				objid_query->vars[objid_query->offset].name_length = MAX_OID_LEN;
 				if (!snmp_parse_oid(objid_query->vars[objid_query->offset].oid, objid_query->vars[objid_query->offset].name, &(objid_query->vars[objid_query->offset].name_length))) {
@@ -770,7 +774,8 @@ static void php_snmp_internal(INTERNAL_FUNCTION_PARAMETERS, int st,
 					snmp_close(ss);
 					RETVAL_FALSE;
 					return;
-				} else {
+				}
+				if (st & SNMP_CMD_SET) {
 					if ((snmp_errno = snmp_add_var(pdu, objid_query->vars[objid_query->offset].name, objid_query->vars[objid_query->offset].name_length, objid_query->vars[objid_query->offset].type, objid_query->vars[objid_query->offset].value))) {
 						snprint_objid(buf, sizeof(buf), objid_query->vars[objid_query->offset].name, objid_query->vars[objid_query->offset].name_length);
 						php_error_docref(NULL TSRMLS_CC, E_WARNING, "Could not add variable: OID='%s' type='%c' value='%s': %s", buf, objid_query->vars[objid_query->offset].type, objid_query->vars[objid_query->offset].value, snmp_api_errstring(snmp_errno));
@@ -779,17 +784,16 @@ static void php_snmp_internal(INTERNAL_FUNCTION_PARAMETERS, int st,
 						RETVAL_FALSE;
 						return;
 					}
+				} else {
+					snmp_add_null_var(pdu, objid_query->vars[objid_query->offset].name, objid_query->vars[objid_query->offset].name_length);
 				}
 			}
-		} else if (st & SNMP_CMD_WALK) {
-			if (session->version == SNMP_VERSION_1) {
-				pdu = snmp_pdu_create(SNMP_MSG_GETNEXT);
-			} else {
-				pdu = snmp_pdu_create(SNMP_MSG_GETBULK);
-				pdu->non_repeaters = objid_query->non_repeaters;
-				pdu->max_repetitions = objid_query->max_repetitions;
+			if(pdu->variables == NULL){
+				snmp_free_pdu(pdu);
+				snmp_close(ss);
+				RETVAL_FALSE;
+				return;
 			}
-			snmp_add_null_var(pdu, name, name_length);
 		}
 
 retry:
diff --git a/ext/snmp/tests/snmp2_get.phpt b/ext/snmp/tests/snmp2_get.phpt
index cc192c9b5d..b6b50cea90 100644
--- a/ext/snmp/tests/snmp2_get.phpt
+++ b/ext/snmp/tests/snmp2_get.phpt
@@ -85,10 +85,7 @@ bool(false)
 Multiple OID
 
 Warning: snmp2_get(): Invalid object identifier: .1.3.6.1.2.1...1.1.0 in %s on line %d
-array(1) {
-  ["%s"]=>
-  %unicode|string%(%d) "%d"
-}
+bool(false)
 noSuchName checks
 Single OID
 
diff --git a/ext/snmp/tests/snmpget.phpt b/ext/snmp/tests/snmpget.phpt
index c2fca2dce2..e4514a2538 100644
--- a/ext/snmp/tests/snmpget.phpt
+++ b/ext/snmp/tests/snmpget.phpt
@@ -87,10 +87,7 @@ bool(false)
 Multiple OID
 
 Warning: snmpget(): Invalid object identifier: .1.3.6.1.2.1...1.1.0 in %s on line %d
-array(1) {
-  ["%s"]=>
-  %unicode|string%(%d) "%d"
-}
+bool(false)
 noSuchName checks
 Single OID