changes from early feedback, new test case

Author: devnexen

ext/snmp/snmp.c

@@ -632,15 +632,29 @@ static void php_snmp_zend_string_release_from_char_pointer(char *ptr) {
 }
 
 static void php_free_objid_query(struct objid_query *objid_query, HashTable* oid_ht, zend_string *value_str, HashTable *value_ht, int st) {
+#define PHP_FREE_OBJID_VAL(arg)									\
+	do {											\
+		if (st & SNMP_CMD_SET) { 							\
+			if (value_str) { 							\
+				php_snmp_zend_string_release_from_char_pointer(arg->value); 	\
+			}									\
+			if (arg->type) {							\
+				php_snmp_zend_string_release_from_char_pointer(&arg->type);	\
+			}									\
+		}										\
+		if (arg->oid) {									\
+			php_snmp_zend_string_release_from_char_pointer(arg->oid);		\
+		}										\
+	} while (0)
+
 	if (oid_ht) {
-		for (int i = 0; i < objid_query->count; i ++) {
-			snmpobjarg *arg = &objid_query->vars[i];
-			if (st & SNMP_CMD_SET) {
-				if (!value_str && value_ht) {
-					php_snmp_zend_string_release_from_char_pointer(arg->value);
-				}
+		snmpobjarg *arg = &objid_query->vars[0];
+		PHP_FREE_OBJID_VAL(arg);
+		if (objid_query->count > 0) {
+			for (int i = 1; i < objid_query->count; i ++) {
+				snmpobjarg *arg = &objid_query->vars[i];
+				PHP_FREE_OBJID_VAL(arg);
 			}
-			php_snmp_zend_string_release_from_char_pointer(arg->oid);
 		}
 	}
 	efree(objid_query->vars);
@@ -696,6 +710,7 @@ static bool php_snmp_parse_oid(
 		objid_query->vars = (snmpobjarg *)safe_emalloc(sizeof(snmpobjarg), zend_hash_num_elements(oid_ht), 0);
 		objid_query->array_output = (st & SNMP_CMD_SET) == 0;
 		ZEND_HASH_FOREACH_VAL(oid_ht, tmp_oid) {
+			objid_query->vars[objid_query->count].oid = NULL;
 			zend_string *tmp = zval_try_get_string(tmp_oid);
 			if (!tmp) {
 				php_free_objid_query(objid_query, oid_ht, value_str, value_ht, st);
@@ -707,6 +722,7 @@ static bool php_snmp_parse_oid(
 					pptr = ZSTR_VAL(type_str);
 					objid_query->vars[objid_query->count].type = *pptr;
 				} else if (type_ht) {
+					objid_query->vars[objid_query->count].type = 0;
 					if (HT_IS_PACKED(type_ht)) {
 						while (idx_type < type_ht->nNumUsed) {
 							tmp_type = &type_ht->arPacked[idx_type];
@@ -725,25 +741,22 @@ static bool php_snmp_parse_oid(
 						}
 					}
 					if (idx_type < type_ht->nNumUsed) {
-						zval new;
-						ZVAL_COPY_VALUE(&new, tmp_type);
-						if (!try_convert_to_string(&new)) {
-							zend_string_release(tmp);
+						zend_string *type = zval_try_get_string(tmp_type);
+						if (!type) {
 							php_free_objid_query(objid_query, oid_ht, value_str, value_ht, st);
 							return false;
 						}
-						if (Z_STRLEN(new) != 1) {
+						if (ZSTR_LEN(type) != 1) {
 							zend_value_error("Type must be a single character");
-							zend_string_release(tmp);
+							zend_string_release(type);
 							php_free_objid_query(objid_query, oid_ht, value_str, value_ht, st);
 							return false;
 						}
-						pptr = Z_STRVAL(new);
+						pptr = ZSTR_VAL(type);
 						objid_query->vars[objid_query->count].type = *pptr;
 						idx_type++;
 					} else {
 						php_error_docref(NULL, E_WARNING, "'%s': no type set", ZSTR_VAL(tmp));
-						zend_string_release(tmp);
 						php_free_objid_query(objid_query, oid_ht, value_str, value_ht, st);
 						return false;
 					}

ext/snmp/tests/gh16959.phpt

@@ -30,6 +30,11 @@ try {
 }
 try {
 	snmp2_set($hostname, $communityWrite, $bad_object_ids, array("toolongtype"), array(null));
+} catch (Throwable $e) {
+	echo $e->getMessage() . PHP_EOL;
+}
+try {
+	snmp2_set($hostname, $communityWrite, $bad_object_ids, array(str_repeat("onetoomuch", random_int(1, 1))), array(null));
 } catch (Throwable $e) {
 	echo $e->getMessage();
 }
@@ -61,3 +66,4 @@ array(4) {
 Object of class stdClass could not be converted to string
 Object of class stdClass could not be converted to string
 Type must be a single character
+Type must be a single character