Skip to content

Commit 4a448a7

Browse files
prateekbhujelprateekbhujel
committed
ext/snmp: include argument numbers in ValueError messages
1 parent b6965c9 commit 4a448a7

File tree

6 files changed

+50
-37
lines changed

6 files changed

+50
-37
lines changed

‎ext/snmp/snmp.c‎

Lines changed: 36 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -655,7 +655,8 @@ static void php_free_objid_query(struct objid_query *objid_query, HashTable* oid
655655
*/
656656
static bool php_snmp_parse_oid(
657657
zval *object, int st, struct objid_query *objid_query, zend_string *oid_str, HashTable *oid_ht,
658-
zend_string *type_str, HashTable *type_ht, zend_string *value_str, HashTable *value_ht
658+
zend_string *type_str, HashTable *type_ht, zend_string *value_str, HashTable *value_ht,
659+
uint32_t oid_argument_offset, uint32_t type_argument_offset, uint32_t value_argument_offset
659660
) {
660661
char *pptr;
661662
uint32_t idx_type = 0, idx_value = 0;
@@ -682,7 +683,7 @@ static bool php_snmp_parse_oid(
682683
ZEND_ASSERT(type_str && value_str);
683684

684685
if (ZSTR_LEN(type_str) != 1) {
685-
zend_value_error("Type must be a single character");
686+
zend_argument_value_error(type_argument_offset, "must be a single character");
686687
efree(objid_query->vars);
687688
return false;
688689
}
@@ -693,7 +694,7 @@ static bool php_snmp_parse_oid(
693694
objid_query->count++;
694695
} else if (oid_ht) { /* we got objid array */
695696
if (zend_hash_num_elements(oid_ht) == 0) {
696-
zend_value_error("Array of object IDs must not be empty");
697+
zend_argument_value_error(oid_argument_offset, "must not be empty when passed as an array");
697698
return false;
698699
}
699700
objid_query->vars = (snmpobjarg *)safe_emalloc(sizeof(snmpobjarg), zend_hash_num_elements(oid_ht), 0);
@@ -738,14 +739,14 @@ static bool php_snmp_parse_oid(
738739
char ptype = *ZSTR_VAL(type);
739740
zend_string_release(type);
740741
if (len != 1) {
741-
zend_value_error("Type must be a single character");
742+
zend_argument_value_error(type_argument_offset, "must be a single character");
742743
php_free_objid_query(objid_query, oid_ht, value_ht, st);
743744
return false;
744745
}
745746
objid_query->vars[objid_query->count].type = ptype;
746747
idx_type++;
747748
} else {
748-
zend_value_error("'%s': no type set", ZSTR_VAL(tmp));
749+
zend_argument_value_error(type_argument_offset, "must contain a type for object ID '%s'", ZSTR_VAL(tmp));
749750
php_free_objid_query(objid_query, oid_ht, value_ht, st);
750751
return false;
751752
}
@@ -780,7 +781,7 @@ static bool php_snmp_parse_oid(
780781
objid_query->vars[objid_query->count].value = ZSTR_VAL(tmp);
781782
idx_value++;
782783
} else {
783-
zend_value_error("'%s': no value set", ZSTR_VAL(tmp));
784+
zend_argument_value_error(value_argument_offset, "must contain a value for object ID '%s'", ZSTR_VAL(tmp));
784785
php_free_objid_query(objid_query, oid_ht, value_ht, st);
785786
return false;
786787
}
@@ -827,7 +828,7 @@ static bool php_snmp_parse_oid(
827828
/* {{{ snmp_session_init
828829
allocates memory for session and session->peername, caller should free it manually using snmp_session_free() and efree()
829830
*/
830-
static bool snmp_session_init(php_snmp_session **session_p, int version, zend_string *hostname, zend_string *community, zend_long timeout, zend_long retries, int timeout_argument_offset)
831+
static bool snmp_session_init(php_snmp_session **session_p, int version, zend_string *hostname, zend_string *community, zend_long timeout, zend_long retries, int hostname_argument_offset, int timeout_argument_offset)
831832
{
832833
php_snmp_session *session;
833834
char *pptr, *host_ptr;
@@ -842,22 +843,22 @@ static bool snmp_session_init(php_snmp_session **session_p, int version, zend_st
842843
ZEND_ASSERT(community != NULL);
843844

844845
if (zend_str_has_nul_byte(hostname)) {
845-
zend_argument_value_error(2, "must not contain any null bytes");
846+
zend_argument_value_error(hostname_argument_offset, "must not contain any null bytes");
846847
return false;
847848
}
848849

849850
if (ZSTR_LEN(hostname) >= MAX_NAME_LEN) {
850-
zend_argument_value_error(2, "length must be lower than %d", MAX_NAME_LEN);
851+
zend_argument_value_error(hostname_argument_offset, "length must be lower than %d", MAX_NAME_LEN);
851852
return false;
852853
}
853854

854855
if (zend_str_has_nul_byte(community)) {
855-
zend_argument_value_error(3, "must not contain any null bytes");
856+
zend_argument_value_error(hostname_argument_offset + 1, "must not contain any null bytes");
856857
return false;
857858
}
858859

859860
if (ZSTR_LEN(community) == 0) {
860-
zend_argument_must_not_be_empty_error(3);
861+
zend_argument_must_not_be_empty_error(hostname_argument_offset + 1);
861862
return false;
862863
}
863864

@@ -899,22 +900,22 @@ static bool snmp_session_init(php_snmp_session **session_p, int version, zend_st
899900
char *pport = pptr + 2;
900901
tmp_port = atoi(pport);
901902
if (tmp_port < 0 || tmp_port > USHRT_MAX) {
902-
zend_argument_value_error(2, "remote port must be between 0 and %u", USHRT_MAX);
903+
zend_argument_value_error(hostname_argument_offset, "remote port must be between 0 and %u", USHRT_MAX);
903904
return false;
904905
}
905906
remote_port = (unsigned short)tmp_port;
906907
}
907908
*pptr = '\0';
908909
} else {
909-
zend_value_error("Malformed IPv6 address, closing square bracket missing");
910+
zend_argument_value_error(hostname_argument_offset, "has a malformed IPv6 address, closing square bracket missing");
910911
return false;
911912
}
912913
} else { /* IPv4 address */
913914
if ((pptr = strchr(host_ptr, ':'))) {
914915
char *pport = pptr + 1;
915916
tmp_port = atoi(pport);
916917
if (tmp_port < 0 || tmp_port > USHRT_MAX) {
917-
zend_argument_value_error(2, "remote port must be between 0 and %u", USHRT_MAX);
918+
zend_argument_value_error(hostname_argument_offset, "remote port must be between 0 and %u", USHRT_MAX);
918919
return false;
919920
}
920921
remote_port = (unsigned short)tmp_port;
@@ -1123,13 +1124,13 @@ static ZEND_ATTRIBUTE_NONNULL bool snmp_session_gen_sec_key(struct snmp_session
11231124
/* }}} */
11241125

11251126
/* {{{ Set context Engine Id in the snmpv3 session */
1126-
static bool snmp_session_set_contextEngineID(struct snmp_session *s, zend_string * contextEngineID)
1127+
static bool snmp_session_set_contextEngineID(struct snmp_session *s, zend_string * contextEngineID, uint32_t contextEngineID_argument_offset)
11271128
{
11281129
size_t ebuf_len = 32, eout_len = 0;
11291130
uint8_t *ebuf = (uint8_t *) emalloc(ebuf_len);
11301131

11311132
if (!snmp_hex_to_binary(&ebuf, &ebuf_len, &eout_len, 1, ZSTR_VAL(contextEngineID))) {
1132-
zend_value_error("Bad engine ID value '%s'", ZSTR_VAL(contextEngineID));
1133+
zend_argument_value_error(contextEngineID_argument_offset, "must be a valid context engine ID");
11331134
efree(ebuf);
11341135
return false;
11351136
}
@@ -1148,7 +1149,7 @@ static bool snmp_session_set_contextEngineID(struct snmp_session *s, zend_string
11481149
static ZEND_ATTRIBUTE_NONNULL_ARGS(2) bool snmp_session_set_security(struct snmp_session *session, zend_string *sec_level,
11491150
zend_string *auth_protocol, zend_string *auth_passphrase, zend_string *priv_protocol,
11501151
zend_string *priv_passphrase, zend_string *contextName, zend_string *contextEngineID,
1151-
uint32_t auth_protocol_argnum)
1152+
uint32_t auth_protocol_argnum, uint32_t contextEngineID_argument_offset)
11521153
{
11531154

11541155
/* Setting the security level. */
@@ -1214,7 +1215,7 @@ static ZEND_ATTRIBUTE_NONNULL_ARGS(2) bool snmp_session_set_security(struct snmp
12141215
}
12151216

12161217
/* Setting contextEngineIS if specified */
1217-
if (contextEngineID && ZSTR_LEN(contextEngineID) && !snmp_session_set_contextEngineID(session, contextEngineID)) {
1218+
if (contextEngineID && ZSTR_LEN(contextEngineID) && !snmp_session_set_contextEngineID(session, contextEngineID, contextEngineID_argument_offset)) {
12181219
/* Warning message sent already, just bail out */
12191220
return false;
12201221
}
@@ -1242,6 +1243,7 @@ static void php_snmp(INTERNAL_FUNCTION_PARAMETERS, int st, int version)
12421243
php_snmp_session *session;
12431244
int session_less_mode = (getThis() == NULL);
12441245
int timeout_argument_offset = -1;
1246+
uint32_t oid_argument_offset = 1, type_argument_offset = 0, value_argument_offset = 0;
12451247
php_snmp_object *snmp_object;
12461248
php_snmp_object glob_snmp_object;
12471249

@@ -1270,6 +1272,9 @@ static void php_snmp(INTERNAL_FUNCTION_PARAMETERS, int st, int version)
12701272
ZEND_PARSE_PARAMETERS_END();
12711273

12721274
timeout_argument_offset = 10;
1275+
oid_argument_offset = 8;
1276+
type_argument_offset = 9;
1277+
value_argument_offset = 10;
12731278
} else {
12741279
/* SNMP_CMD_GET
12751280
* SNMP_CMD_GETNEXT
@@ -1290,6 +1295,7 @@ static void php_snmp(INTERNAL_FUNCTION_PARAMETERS, int st, int version)
12901295
ZEND_PARSE_PARAMETERS_END();
12911296

12921297
timeout_argument_offset = 9;
1298+
oid_argument_offset = 8;
12931299
}
12941300
} else {
12951301
if (st & SNMP_CMD_SET) {
@@ -1305,6 +1311,9 @@ static void php_snmp(INTERNAL_FUNCTION_PARAMETERS, int st, int version)
13051311
ZEND_PARSE_PARAMETERS_END();
13061312

13071313
timeout_argument_offset = 6;
1314+
oid_argument_offset = 3;
1315+
type_argument_offset = 4;
1316+
value_argument_offset = 5;
13081317
} else {
13091318
/* SNMP_CMD_GET
13101319
* SNMP_CMD_GETNEXT
@@ -1320,6 +1329,7 @@ static void php_snmp(INTERNAL_FUNCTION_PARAMETERS, int st, int version)
13201329
ZEND_PARSE_PARAMETERS_END();
13211330

13221331
timeout_argument_offset = 4;
1332+
oid_argument_offset = 3;
13231333
}
13241334
}
13251335
} else {
@@ -1329,6 +1339,8 @@ static void php_snmp(INTERNAL_FUNCTION_PARAMETERS, int st, int version)
13291339
Z_PARAM_ARRAY_HT_OR_STR(type_ht, type_str)
13301340
Z_PARAM_ARRAY_HT_OR_STR(value_ht, value_str)
13311341
ZEND_PARSE_PARAMETERS_END();
1342+
type_argument_offset = 2;
1343+
value_argument_offset = 3;
13321344
} else if (st & SNMP_CMD_WALK) {
13331345
ZEND_PARSE_PARAMETERS_START(1, 4)
13341346
Z_PARAM_ARRAY_HT_OR_STR(oid_ht, oid_str)
@@ -1358,17 +1370,18 @@ static void php_snmp(INTERNAL_FUNCTION_PARAMETERS, int st, int version)
13581370
}
13591371
}
13601372

1361-
if (!php_snmp_parse_oid(getThis(), st, &objid_query, oid_str, oid_ht, type_str, type_ht, value_str, value_ht)) {
1373+
if (!php_snmp_parse_oid(getThis(), st, &objid_query, oid_str, oid_ht, type_str, type_ht, value_str, value_ht,
1374+
oid_argument_offset, type_argument_offset, value_argument_offset)) {
13621375
RETURN_FALSE;
13631376
}
13641377

13651378
if (session_less_mode) {
1366-
if (!snmp_session_init(&session, version, a1, a2, timeout, retries, timeout_argument_offset)) {
1379+
if (!snmp_session_init(&session, version, a1, a2, timeout, retries, 1, timeout_argument_offset)) {
13671380
php_free_objid_query(&objid_query, oid_ht, value_ht, st);
13681381
snmp_session_free(&session);
13691382
RETURN_FALSE;
13701383
}
1371-
if (version == SNMP_VERSION_3 && !snmp_session_set_security(session, a3, a4, a5, a6, a7, NULL, NULL, 4)) {
1384+
if (version == SNMP_VERSION_3 && !snmp_session_set_security(session, a3, a4, a5, a6, a7, NULL, NULL, 4, 0)) {
13721385
php_free_objid_query(&objid_query, oid_ht, value_ht, st);
13731386
snmp_session_free(&session);
13741387
/* Warning message sent already, just bail out */
@@ -1663,7 +1676,7 @@ PHP_METHOD(SNMP, __construct)
16631676
snmp_session_free(&(snmp_object->session));
16641677
}
16651678

1666-
if (!snmp_session_init(&(snmp_object->session), version, a1, a2, timeout, retries, 4)) {
1679+
if (!snmp_session_init(&(snmp_object->session), version, a1, a2, timeout, retries, 2, 4)) {
16671680
return;
16681681
}
16691682
snmp_object->max_oids = 0;
@@ -1737,7 +1750,7 @@ PHP_METHOD(SNMP, setSecurity)
17371750
RETURN_THROWS();
17381751
}
17391752

1740-
if (!snmp_session_set_security(snmp_object->session, a1, a2, a3, a4, a5, a6, a7, 2)) {
1753+
if (!snmp_session_set_security(snmp_object->session, a1, a2, a3, a4, a5, a6, a7, 2, 7)) {
17411754
/* Warning message sent already, just bail out */
17421755
RETURN_FALSE;
17431756
}

‎ext/snmp/tests/gh16959.phpt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,5 +66,5 @@ array(4) {
6666
}
6767
Object of class stdClass could not be converted to string
6868
Object of class stdClass could not be converted to string
69-
Type must be a single character
70-
Type must be a single character
69+
snmp2_set(): Argument #4 ($type) must be a single character
70+
snmp2_set(): Argument #4 ($type) must be a single character

‎ext/snmp/tests/ipv6.phpt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,4 +30,4 @@ try {
3030
?>
3131
--EXPECTF--
3232
string(%d) "%s"
33-
Malformed IPv6 address, closing square bracket missing
33+
snmpget(): Argument #1 ($hostname) has a malformed IPv6 address, closing square bracket missing

‎ext/snmp/tests/snmp-object-setSecurity_error.phpt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,5 +80,5 @@ bool(false)
8080

8181
Warning: SNMP::setSecurity(): Error generating a key for privacy pass phrase 'ty': Generic error (The supplied password length is too short.) in %s on line %d
8282
bool(false)
83-
Bad engine ID value 'dsa'
83+
SNMP::setSecurity(): Argument #7 ($contextEngineId) must be a valid context engine ID
8484
bool(true)

‎ext/snmp/tests/snmp2_set.phpt‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ var_dump((snmpget($hostname, $communityWrite, $oid2, $timeout, $retries) === $ol
175175
--EXPECTF--
176176
Check error handing
177177
No type & no value (timeout & retries instead)
178-
Type must be a single character
178+
snmp2_set(): Argument #4 ($type) must be a single character
179179
No value (timeout instead), retries instead of timeout
180180

181181
Warning: snmp2_set(): Could not add variable: OID='%s' type='q' value='%i': Bad variable type ("q") in %s on line %d
@@ -224,19 +224,19 @@ Value must be of type string when object ID is a string
224224
bool(true)
225225
bool(true)
226226
Multiple OID, 1st wrong type
227-
Type must be a single character
227+
snmp2_set(): Argument #4 ($type) must be a single character
228228
bool(true)
229229
bool(true)
230230
Multiple OID, 2nd wrong type
231-
Type must be a single character
231+
snmp2_set(): Argument #4 ($type) must be a single character
232232
bool(true)
233233
bool(true)
234234
Multiple OID, single type in array, multiple value
235-
'SNMPv2-MIB::sysLocation.0': no type set
235+
snmp2_set(): Argument #4 ($type) must contain a type for object ID 'SNMPv2-MIB::sysLocation.0'
236236
bool(true)
237237
bool(true)
238238
Multiple OID & type, single value in array
239-
'SNMPv2-MIB::sysLocation.0': no value set
239+
snmp2_set(): Argument #5 ($value) must contain a value for object ID 'SNMPv2-MIB::sysLocation.0'
240240
bool(true)
241241
bool(true)
242242
Multiple OID, 1st bogus, single type, multiple value

‎ext/snmp/tests/snmpset.phpt‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,7 @@ var_dump((snmpget($hostname, $communityWrite, $oid2, $timeout, $retries) === $ol
169169
--EXPECTF--
170170
Check error handing
171171
No type & no value (timeout & retries instead)
172-
Type must be a single character
172+
snmpset(): Argument #4 ($type) must be a single character
173173
No value (timeout instead), retries instead of timeout
174174

175175
Warning: snmpset(): Could not add variable: OID='%s' type='q' value='%i': Bad variable type ("q") in %s on line %d
@@ -215,19 +215,19 @@ Value must be of type string when object ID is a string
215215
bool(true)
216216
bool(true)
217217
Multiple OID, 1st wrong type
218-
Type must be a single character
218+
snmpset(): Argument #4 ($type) must be a single character
219219
bool(true)
220220
bool(true)
221221
Multiple OID, 2nd wrong type
222-
Type must be a single character
222+
snmpset(): Argument #4 ($type) must be a single character
223223
bool(true)
224224
bool(true)
225225
Multiple OID, single type in array, multiple value
226-
'SNMPv2-MIB::sysLocation.0': no type set
226+
snmpset(): Argument #4 ($type) must contain a type for object ID 'SNMPv2-MIB::sysLocation.0'
227227
bool(true)
228228
bool(true)
229229
Multiple OID & type, single value in array
230-
'SNMPv2-MIB::sysLocation.0': no value set
230+
snmpset(): Argument #5 ($value) must contain a value for object ID 'SNMPv2-MIB::sysLocation.0'
231231
bool(true)
232232
bool(true)
233233
Multiple OID, 1st bogus, single type, multiple value

0 commit comments

Comments
 (0)