Skip to content

Commit ba04f37

Browse files
GirgiasGirgias
committed
Address review comments
1 parent 432131b commit ba04f37

10 files changed

+61
-53
lines changed

‎ext/snmp/snmp.c

Lines changed: 30 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -673,25 +673,33 @@ static int php_snmp_parse_oid(
673673
objid_query->vars = (snmpobjarg *)emalloc(sizeof(snmpobjarg));
674674
objid_query->vars[objid_query->count].oid = ZSTR_VAL(oid_str);
675675
if (st & SNMP_CMD_SET) {
676-
if (type_str && value_str) {
677-
if (ZSTR_LEN(type_str) != 1) {
678-
zend_value_error("Type must be a character");
679-
efree(objid_query->vars);
680-
return FALSE;
681-
}
682-
pptr = ZSTR_VAL(type_str);
683-
objid_query->vars[objid_query->count].type = *pptr;
684-
objid_query->vars[objid_query->count].value = ZSTR_VAL(value_str);
685-
} else {
686-
zend_value_error("Type must be a character when Object ID is a string");
676+
if (type_ht) {
677+
zend_type_error("Type must be of type string when object ID is a string");
678+
efree(objid_query->vars);
679+
return FALSE;
680+
}
681+
if (value_ht) {
682+
zend_type_error("Value must be of type string when object ID is a string");
687683
efree(objid_query->vars);
688684
return FALSE;
689685
}
686+
687+
/* Both type and value must be valid strings */
688+
ZEND_ASSERT(type_str && value_str);
689+
690+
if (ZSTR_LEN(type_str) != 1) {
691+
zend_value_error("Type must be a single character");
692+
efree(objid_query->vars);
693+
return FALSE;
694+
}
695+
pptr = ZSTR_VAL(type_str);
696+
objid_query->vars[objid_query->count].type = *pptr;
697+
objid_query->vars[objid_query->count].value = ZSTR_VAL(value_str);
690698
}
691699
objid_query->count++;
692700
} else if (oid_ht) { /* we got objid array */
693701
if (zend_hash_num_elements(oid_ht) == 0) {
694-
zend_value_error("Object ID array cannot be empty");
702+
zend_value_error("Object ID cannot be an empty array");
695703
return FALSE;
696704
}
697705
objid_query->vars = (snmpobjarg *)safe_emalloc(sizeof(snmpobjarg), zend_hash_num_elements(oid_ht), 0);
@@ -714,7 +722,7 @@ static int php_snmp_parse_oid(
714722
if (idx_type < type_ht->nNumUsed) {
715723
convert_to_string_ex(tmp_type);
716724
if (Z_STRLEN_P(tmp_type) != 1) {
717-
zend_value_error("Type must be a character");
725+
zend_value_error("Type must be a single character");
718726
efree(objid_query->vars);
719727
return FALSE;
720728
}
@@ -915,7 +923,7 @@ static int netsnmp_session_set_sec_level(struct snmp_session *s, char *level)
915923
} else if (!strcasecmp(level, "authPriv") || !strcasecmp(level, "ap")) {
916924
s->securityLevel = SNMP_SEC_LEVEL_AUTHPRIV;
917925
} else {
918-
zend_value_error("Security level must be \"noAuthNoPriv\", \"authNoPriv\", or \"authPriv\"");
926+
zend_value_error("Security level must be one of \"noAuthNoPriv\", \"authNoPriv\", or \"authPriv\"");
919927
return (-1);
920928
}
921929
return (0);
@@ -933,7 +941,7 @@ static int netsnmp_session_set_auth_protocol(struct snmp_session *s, char *prot)
933941
s->securityAuthProto = usmHMACSHA1AuthProtocol;
934942
s->securityAuthProtoLen = USM_AUTH_PROTO_SHA_LEN;
935943
} else {
936-
zend_value_error("Authentication protocol mus be either MD5 or SHA");
944+
zend_value_error("Authentication protocol must be either MD5 or SHA");
937945
return (-1);
938946
}
939947
return (0);
@@ -953,11 +961,11 @@ static int netsnmp_session_set_sec_protocol(struct snmp_session *s, char *prot)
953961
s->securityPrivProtoLen = USM_PRIV_PROTO_AES_LEN;
954962
#endif
955963
} else {
956-
zend_value_error("Security protocol must be DES"
957964
#ifdef HAVE_AES
958-
", AES128, or AES"
965+
zend_value_error("Security protocol must be one of DES, AES128, or AES");
966+
#else
967+
zend_value_error("Security protocol must be DES");
959968
#endif
960-
);
961969
return (-1);
962970
}
963971
return (0);
@@ -1357,7 +1365,7 @@ PHP_FUNCTION(snmp_set_oid_output_format)
13571365
netsnmp_ds_set_int(NETSNMP_DS_LIBRARY_ID, NETSNMP_DS_LIB_OID_OUTPUT_FORMAT, a1);
13581366
RETURN_TRUE;
13591367
default:
1360-
zend_argument_value_error(1, "must be one of SNMP_OID_OUTPUT_*");
1368+
zend_argument_value_error(1, "must be an SNMP_OID_OUTPUT_* constant");
13611369
RETURN_THROWS();
13621370
}
13631371
}
@@ -1446,7 +1454,7 @@ PHP_FUNCTION(snmp_set_valueretrieval)
14461454
SNMP_G(valueretrieval) = method;
14471455
RETURN_TRUE;
14481456
} else {
1449-
zend_argument_value_error(1, "must be SNMP_VALUE_LIBRARY, SNMP_VALUE_PLAIN, or SNMP_VALUE_OBJECT");
1457+
zend_argument_value_error(1, "must be one of SNMP_VALUE_LIBRARY, SNMP_VALUE_PLAIN, or SNMP_VALUE_OBJECT");
14501458
RETURN_THROWS();
14511459
}
14521460
}
@@ -1865,7 +1873,7 @@ static int php_snmp_write_valueretrieval(php_snmp_object *snmp_object, zval *new
18651873
if (lval >= 0 && lval <= (SNMP_VALUE_LIBRARY|SNMP_VALUE_PLAIN|SNMP_VALUE_OBJECT)) {
18661874
snmp_object->valueretrieval = lval;
18671875
} else {
1868-
zend_value_error("SNMP retrieval method must be SNMP_VALUE_LIBRARY, SNMP_VALUE_PLAIN, or SNMP_VALUE_OBJECT");
1876+
zend_value_error("SNMP retrieval method must be one of SNMP_VALUE_LIBRARY, SNMP_VALUE_PLAIN, or SNMP_VALUE_OBJECT");
18691877
return FAILURE;
18701878
}
18711879

@@ -1905,7 +1913,7 @@ static int php_snmp_write_oid_output_format(php_snmp_object *snmp_object, zval *
19051913
snmp_object->oid_output_format = lval;
19061914
return SUCCESS;
19071915
default:
1908-
zend_value_error("SNMP output print format must be one of SNMP_OID_OUTPUT_*");
1916+
zend_value_error("SNMP output print format must be an SNMP_OID_OUTPUT_* constant");
19091917
return FAILURE;
19101918
}
19111919
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ int(32)
9898
string(46) "Invalid object identifier: .1.3.6.1.2.1.1.1..0"
9999
bool(true)
100100
Open normal session
101-
SNMP retrieval method must be SNMP_VALUE_LIBRARY, SNMP_VALUE_PLAIN, or SNMP_VALUE_OBJECT
101+
SNMP retrieval method must be one of SNMP_VALUE_LIBRARY, SNMP_VALUE_PLAIN, or SNMP_VALUE_OBJECT
102102
Closing session
103103
bool(true)
104104
Invalid or uninitialized SNMP object

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ Error handling
190190
Warning: Undefined property: SNMP::$there is no such parameter in %s on line %d
191191
NULL
192192
bool(false)
193-
SNMP retrieval method must be SNMP_VALUE_LIBRARY, SNMP_VALUE_PLAIN, or SNMP_VALUE_OBJECT
194-
SNMP output print format must be one of SNMP_OID_OUTPUT_*
193+
SNMP retrieval method must be one of SNMP_VALUE_LIBRARY, SNMP_VALUE_PLAIN, or SNMP_VALUE_OBJECT
194+
SNMP output print format must be an SNMP_OID_OUTPUT_* constant
195195
info property is read-only
196196
NULL

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -57,17 +57,17 @@ var_dump($session->close());
5757

5858
?>
5959
--EXPECTF--
60-
Security level must be "noAuthNoPriv", "authNoPriv", or "authPriv"
61-
Security level must be "noAuthNoPriv", "authNoPriv", or "authPriv"
62-
Authentication protocol mus be either MD5 or SHA
60+
Security level must be one of "noAuthNoPriv", "authNoPriv", or "authPriv"
61+
Security level must be one of "noAuthNoPriv", "authNoPriv", or "authPriv"
62+
Authentication protocol must be either MD5 or SHA
6363

6464
Warning: SNMP::setSecurity(): Error generating a key for authentication pass phrase '': Generic error (The supplied password length is too short.) in %s on line %d
6565
bool(false)
6666

6767
Warning: SNMP::setSecurity(): Error generating a key for authentication pass phrase 'te': Generic error (The supplied password length is too short.) in %s on line %d
6868
bool(false)
69-
Security protocol must be DES, AES128, or AES
70-
Security protocol must be DES, AES128, or AES
69+
Security protocol must be one of DES, AES128, or AES
70+
Security protocol must be one of DES, AES128, or AES
7171

7272
Warning: SNMP::setSecurity(): Error generating a key for privacy pass phrase '': Generic error (The supplied password length is too short.) in %s on line %d
7373
bool(false)

‎ext/snmp/tests/snmp2_get.phpt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ var_dump(snmp2_get($hostname, $community, array('.1.3.6.1.2.1.1.1.0', '.1.3.6.1.
5151
--EXPECTF--
5252
Checking error handling
5353
Empty OID array
54-
Object ID array cannot be empty
54+
Object ID cannot be an empty array
5555
Checking working
5656
Single OID
5757
string(%d) "%s"

‎ext/snmp/tests/snmp2_set.phpt

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ try {
128128
var_dump($z);
129129
var_dump((snmpget($hostname, $communityWrite, $oid1, $timeout, $retries) === $oldvalue1));
130130
var_dump((snmpget($hostname, $communityWrite, $oid2, $timeout, $retries) === $oldvalue2));
131-
} catch (\ValueError $e) {
131+
} catch (\TypeError $e) {
132132
echo $e->getMessage() . \PHP_EOL;
133133
}
134134
echo "Single OID, single type, single value in array\n";
@@ -137,7 +137,7 @@ try {
137137
var_dump($z);
138138
var_dump((snmpget($hostname, $communityWrite, $oid1, $timeout, $retries) === $oldvalue1));
139139
var_dump((snmpget($hostname, $communityWrite, $oid2, $timeout, $retries) === $oldvalue2));
140-
} catch (\ValueError $e) {
140+
} catch (\TypeError $e) {
141141
echo $e->getMessage() . \PHP_EOL;
142142
}
143143
echo "Multiple OID, 1st wrong type\n";
@@ -216,7 +216,7 @@ try {
216216
--EXPECTF--
217217
Check error handing
218218
No type & no value (timeout & retries instead)
219-
Type must be a character
219+
Type must be a single character
220220
No value (timeout instead), retries instead of timeout
221221

222222
Warning: snmp2_set(): Could not add variable: OID='%s' type='q' value='%i': Bad variable type ("q") in %s on line %d
@@ -257,13 +257,13 @@ bool(true)
257257
bool(true)
258258
More error handing
259259
Single OID, single type in array, single value
260-
Type must be a character when Object ID is a string
260+
Type must be of type string when object ID is a string
261261
Single OID, single type, single value in array
262-
Type must be a character when Object ID is a string
262+
Value must be of type string when object ID is a string
263263
Multiple OID, 1st wrong type
264-
Type must be a character
264+
Type must be a single character
265265
Multiple OID, 2nd wrong type
266-
Type must be a character
266+
Type must be a single character
267267
Multiple OID, single type in array, multiple value
268268

269269
Warning: snmp2_set(): '%s': no type set in %s on line %d

‎ext/snmp/tests/snmp3-error.phpt

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -49,23 +49,23 @@ var_dump(snmp3_set($hostname, $rwuser, 'authPriv', 'MD5', $auth_pass, 'AES', $pr
4949

5050
try {
5151
var_dump(snmp3_set($hostname, $rwuser, 'authPriv', 'MD5', $auth_pass, 'AES', $priv_pass, '.1.3.6.777.7.5.3', array('s'), 'yyy', $timeout, $retries));
52-
} catch (\ValueError $e) {
52+
} catch (\TypeError $e) {
5353
echo $e->getMessage() . \PHP_EOL;
5454
}
5555

5656
?>
5757
--EXPECTF--
5858
Checking error handling
59-
Security level must be "noAuthNoPriv", "authNoPriv", or "authPriv"
60-
Security level must be "noAuthNoPriv", "authNoPriv", or "authPriv"
61-
Authentication protocol mus be either MD5 or SHA
59+
Security level must be one of "noAuthNoPriv", "authNoPriv", or "authPriv"
60+
Security level must be one of "noAuthNoPriv", "authNoPriv", or "authPriv"
61+
Authentication protocol must be either MD5 or SHA
6262

6363
Warning: snmp3_get(): Error generating a key for authentication pass phrase '': Generic error (The supplied password length is too short.) in %s on line %d
6464
bool(false)
6565

6666
Warning: snmp3_get(): Error generating a key for authentication pass phrase 'te': Generic error (The supplied password length is too short.) in %s on line %d
6767
bool(false)
68-
Security protocol must be DES, AES128, or AES
68+
Security protocol must be one of DES, AES128, or AES
6969

7070
Warning: snmp3_get(): Error generating a key for privacy pass phrase '': Generic error (The supplied password length is too short.) in %s on line %d
7171
bool(false)
@@ -78,4 +78,4 @@ bool(false)
7878

7979
Warning: snmp3_set(): Invalid object identifier: .1.3.6.777...7.5.3 in %s on line %d
8080
bool(false)
81-
Type must be a character when Object ID is a string
81+
Type must be of type string when object ID is a string

‎ext/snmp/tests/snmp_get_valueretrieval.phpt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ var_dump(snmp_get_valueretrieval() === (SNMP_VALUE_LIBRARY|SNMP_VALUE_OBJECT));
3333
?>
3434
--EXPECTF--
3535
Checking error handling
36-
snmp_set_valueretrieval(): Argument #1 ($method) must be SNMP_VALUE_LIBRARY, SNMP_VALUE_PLAIN, or SNMP_VALUE_OBJECT
36+
snmp_set_valueretrieval(): Argument #1 ($method) must be one of SNMP_VALUE_LIBRARY, SNMP_VALUE_PLAIN, or SNMP_VALUE_OBJECT
3737
Checking working
3838
int(%d)
3939
bool(true)

‎ext/snmp/tests/snmp_set_oid_output_format.phpt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ var_dump(snmp_set_oid_output_format(SNMP_OID_OUTPUT_NUMERIC));
2424
?>
2525
--EXPECT--
2626
Checking error handling
27-
snmp_set_oid_output_format(): Argument #1 ($oid_format) must be one of SNMP_OID_OUTPUT_*
27+
snmp_set_oid_output_format(): Argument #1 ($oid_format) must be an SNMP_OID_OUTPUT_* constant
2828
Checking working
2929
bool(true)
3030
bool(true)

‎ext/snmp/tests/snmpset.phpt

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ try {
8585
var_dump($z);
8686
var_dump((snmpget($hostname, $communityWrite, $oid1, $timeout, $retries) === $oldvalue1));
8787
var_dump((snmpget($hostname, $communityWrite, $oid2, $timeout, $retries) === $oldvalue2));
88-
} catch (\ValueError $e) {
88+
} catch (\TypeError $e) {
8989
echo $e->getMessage() . \PHP_EOL;
9090
}
9191

@@ -95,7 +95,7 @@ try {
9595
var_dump($z);
9696
var_dump((snmpget($hostname, $communityWrite, $oid1, $timeout, $retries) === $oldvalue1));
9797
var_dump((snmpget($hostname, $communityWrite, $oid2, $timeout, $retries) === $oldvalue2));
98-
} catch (\ValueError $e) {
98+
} catch (\TypeError $e) {
9999
echo $e->getMessage() . \PHP_EOL;
100100
}
101101

@@ -158,7 +158,7 @@ var_dump((snmpget($hostname, $communityWrite, $oid2, $timeout, $retries) === $ol
158158
--EXPECTF--
159159
Check error handing
160160
No type & no value (timeout & retries instead)
161-
Type must be a character
161+
Type must be a single character
162162
No value (timeout instead), retries instead of timeout
163163

164164
Warning: snmpset(): Could not add variable: OID='%s' type='q' value='%i': Bad variable type ("q") in %s on line %d
@@ -196,13 +196,13 @@ bool(true)
196196
bool(true)
197197
More error handing
198198
Single OID, single type in array, single value
199-
Type must be a character when Object ID is a string
199+
Type must be of type string when object ID is a string
200200
Single OID, single type, single value in array
201-
Type must be a character when Object ID is a string
201+
Value must be of type string when object ID is a string
202202
Multiple OID, 1st wrong type
203-
Type must be a character
203+
Type must be a single character
204204
Multiple OID, 2nd wrong type
205-
Type must be a character
205+
Type must be a single character
206206
Multiple OID, single type in array, multiple value
207207

208208
Warning: snmpset(): '%s': no type set in %s on line %d

0 commit comments

Comments
 (0)