Skip to content

Commit b177d74

Browse files
prateekbhujelprateekbhujel
committed
ext/snmp: include argument numbers in ValueError messages
1 parent 2f19d30 commit b177d74

File tree

6 files changed

+51
-37
lines changed

6 files changed

+51
-37
lines changed

‎ext/snmp/snmp.c‎

Lines changed: 37 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;
@@ -1122,13 +1123,13 @@ static bool snmp_session_gen_sec_key(struct snmp_session *s, zend_string *pass)
11221123
/* }}} */
11231124

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

11301131
if (!snmp_hex_to_binary(&ebuf, &ebuf_len, &eout_len, 1, ZSTR_VAL(contextEngineID))) {
1131-
zend_value_error("Bad engine ID value '%s'", ZSTR_VAL(contextEngineID));
1132+
zend_argument_value_error(contextEngineID_argument_offset, "must be a valid context engine ID");
11321133
efree(ebuf);
11331134
return false;
11341135
}
@@ -1146,7 +1147,8 @@ static bool snmp_session_set_contextEngineID(struct snmp_session *s, zend_string
11461147
/* {{{ Set all snmpv3-related security options */
11471148
static bool snmp_session_set_security(struct snmp_session *session, zend_string *sec_level,
11481149
zend_string *auth_protocol, zend_string *auth_passphrase, zend_string *priv_protocol,
1149-
zend_string *priv_passphrase, zend_string *contextName, zend_string *contextEngineID)
1150+
zend_string *priv_passphrase, zend_string *contextName, zend_string *contextEngineID,
1151+
uint32_t contextEngineID_argument_offset)
11501152
{
11511153

11521154
/* Setting the security level. */
@@ -1191,7 +1193,7 @@ static bool snmp_session_set_security(struct snmp_session *session, zend_string
11911193
}
11921194

11931195
/* Setting contextEngineIS if specified */
1194-
if (contextEngineID && ZSTR_LEN(contextEngineID) && !snmp_session_set_contextEngineID(session, contextEngineID)) {
1196+
if (contextEngineID && ZSTR_LEN(contextEngineID) && !snmp_session_set_contextEngineID(session, contextEngineID, contextEngineID_argument_offset)) {
11951197
/* Warning message sent already, just bail out */
11961198
return false;
11971199
}
@@ -1219,6 +1221,7 @@ static void php_snmp(INTERNAL_FUNCTION_PARAMETERS, int st, int version)
12191221
php_snmp_session *session;
12201222
int session_less_mode = (getThis() == NULL);
12211223
int timeout_argument_offset = -1;
1224+
uint32_t oid_argument_offset = 1, type_argument_offset = 0, value_argument_offset = 0;
12221225
php_snmp_object *snmp_object;
12231226
php_snmp_object glob_snmp_object;
12241227

@@ -1247,6 +1250,9 @@ static void php_snmp(INTERNAL_FUNCTION_PARAMETERS, int st, int version)
12471250
ZEND_PARSE_PARAMETERS_END();
12481251

12491252
timeout_argument_offset = 10;
1253+
oid_argument_offset = 8;
1254+
type_argument_offset = 9;
1255+
value_argument_offset = 10;
12501256
} else {
12511257
/* SNMP_CMD_GET
12521258
* SNMP_CMD_GETNEXT
@@ -1267,6 +1273,7 @@ static void php_snmp(INTERNAL_FUNCTION_PARAMETERS, int st, int version)
12671273
ZEND_PARSE_PARAMETERS_END();
12681274

12691275
timeout_argument_offset = 9;
1276+
oid_argument_offset = 8;
12701277
}
12711278
} else {
12721279
if (st & SNMP_CMD_SET) {
@@ -1282,6 +1289,9 @@ static void php_snmp(INTERNAL_FUNCTION_PARAMETERS, int st, int version)
12821289
ZEND_PARSE_PARAMETERS_END();
12831290

12841291
timeout_argument_offset = 6;
1292+
oid_argument_offset = 3;
1293+
type_argument_offset = 4;
1294+
value_argument_offset = 5;
12851295
} else {
12861296
/* SNMP_CMD_GET
12871297
* SNMP_CMD_GETNEXT
@@ -1297,6 +1307,7 @@ static void php_snmp(INTERNAL_FUNCTION_PARAMETERS, int st, int version)
12971307
ZEND_PARSE_PARAMETERS_END();
12981308

12991309
timeout_argument_offset = 4;
1310+
oid_argument_offset = 3;
13001311
}
13011312
}
13021313
} else {
@@ -1306,6 +1317,8 @@ static void php_snmp(INTERNAL_FUNCTION_PARAMETERS, int st, int version)
13061317
Z_PARAM_ARRAY_HT_OR_STR(type_ht, type_str)
13071318
Z_PARAM_ARRAY_HT_OR_STR(value_ht, value_str)
13081319
ZEND_PARSE_PARAMETERS_END();
1320+
type_argument_offset = 2;
1321+
value_argument_offset = 3;
13091322
} else if (st & SNMP_CMD_WALK) {
13101323
ZEND_PARSE_PARAMETERS_START(1, 4)
13111324
Z_PARAM_ARRAY_HT_OR_STR(oid_ht, oid_str)
@@ -1335,17 +1348,18 @@ static void php_snmp(INTERNAL_FUNCTION_PARAMETERS, int st, int version)
13351348
}
13361349
}
13371350

1338-
if (!php_snmp_parse_oid(getThis(), st, &objid_query, oid_str, oid_ht, type_str, type_ht, value_str, value_ht)) {
1351+
if (!php_snmp_parse_oid(getThis(), st, &objid_query, oid_str, oid_ht, type_str, type_ht, value_str, value_ht,
1352+
oid_argument_offset, type_argument_offset, value_argument_offset)) {
13391353
RETURN_FALSE;
13401354
}
13411355

13421356
if (session_less_mode) {
1343-
if (!snmp_session_init(&session, version, a1, a2, timeout, retries, timeout_argument_offset)) {
1357+
if (!snmp_session_init(&session, version, a1, a2, timeout, retries, 1, timeout_argument_offset)) {
13441358
php_free_objid_query(&objid_query, oid_ht, value_ht, st);
13451359
snmp_session_free(&session);
13461360
RETURN_FALSE;
13471361
}
1348-
if (version == SNMP_VERSION_3 && !snmp_session_set_security(session, a3, a4, a5, a6, a7, NULL, NULL)) {
1362+
if (version == SNMP_VERSION_3 && !snmp_session_set_security(session, a3, a4, a5, a6, a7, NULL, NULL, 0)) {
13491363
php_free_objid_query(&objid_query, oid_ht, value_ht, st);
13501364
snmp_session_free(&session);
13511365
/* Warning message sent already, just bail out */
@@ -1644,7 +1658,7 @@ PHP_METHOD(SNMP, __construct)
16441658
snmp_session_free(&(snmp_object->session));
16451659
}
16461660

1647-
if (!snmp_session_init(&(snmp_object->session), version, a1, a2, timeout, retries, 4)) {
1661+
if (!snmp_session_init(&(snmp_object->session), version, a1, a2, timeout, retries, 2, 4)) {
16481662
return;
16491663
}
16501664
snmp_object->max_oids = 0;
@@ -1720,7 +1734,7 @@ PHP_METHOD(SNMP, setSecurity)
17201734
RETURN_THROWS();
17211735
}
17221736

1723-
if (!snmp_session_set_security(snmp_object->session, a1, a2, a3, a4, a5, a6, a7)) {
1737+
if (!snmp_session_set_security(snmp_object->session, a1, a2, a3, a4, a5, a6, a7, 7)) {
17241738
/* Warning message sent already, just bail out */
17251739
RETURN_FALSE;
17261740
}

‎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)