Skip to content

Commit cf043af

Browse files
GirgiasGirgias
committed
ext/ldap: Refactor verification of modification entry
1 parent 3af914d commit cf043af

File tree

4 files changed

+85
-133
lines changed

4 files changed

+85
-133
lines changed

‎ext/ldap/ldap.c

Lines changed: 72 additions & 120 deletions
Original file line numberDiff line numberDiff line change
@@ -2491,24 +2491,6 @@ PHP_FUNCTION(ldap_delete_ext)
24912491
}
24922492
/* }}} */
24932493

2494-
/* {{{ _ldap_str_equal_to_const */
2495-
static size_t _ldap_str_equal_to_const(const char *str, size_t str_len, const char *cstr)
2496-
{
2497-
size_t i;
2498-
2499-
if (strlen(cstr) != str_len)
2500-
return 0;
2501-
2502-
for (i = 0; i < str_len; ++i) {
2503-
if (str[i] != cstr[i]) {
2504-
return 0;
2505-
}
2506-
}
2507-
2508-
return 1;
2509-
}
2510-
/* }}} */
2511-
25122494
/* {{{ Perform multiple modifications as part of one operation */
25132495
PHP_FUNCTION(ldap_modify_batch)
25142496
{
@@ -2554,10 +2536,6 @@ PHP_FUNCTION(ldap_modify_batch)
25542536

25552537
/* perform validation */
25562538
{
2557-
zend_string *modkey;
2558-
/* to store the wrongly-typed keys */
2559-
zend_ulong tmpUlong;
2560-
25612539
/* make sure the DN contains no NUL bytes */
25622540
if (zend_char_has_nul_byte(dn, dn_len)) {
25632541
zend_argument_value_error(2, "must not contain null bytes");
@@ -2579,117 +2557,91 @@ PHP_FUNCTION(ldap_modify_batch)
25792557
zend_argument_value_error(3, "must have consecutive integer indices starting from 0");
25802558
RETURN_THROWS();
25812559
}
2582-
mod = fetched;
2560+
zval *modification_zv = fetched;
25832561

2584-
/* is it an array? */
2585-
if (Z_TYPE_P(mod) != IS_ARRAY) {
2586-
zend_argument_value_error(3, "must only contain arrays");
2562+
if (Z_TYPE_P(modification_zv) != IS_ARRAY) {
2563+
zend_argument_type_error(3, "must only contain arrays");
25872564
RETURN_THROWS();
25882565
}
25892566

2590-
SEPARATE_ARRAY(mod);
2591-
/* for the modification hashtable... */
2592-
zend_hash_internal_pointer_reset(Z_ARRVAL_P(mod));
2593-
uint32_t num_modprops = zend_hash_num_elements(Z_ARRVAL_P(mod));
2594-
bool has_attrib_key = false;
2595-
bool has_modtype_key = false;
2596-
2597-
for (j = 0; j < num_modprops; j++) {
2598-
2599-
/* are the keys strings? */
2600-
if (zend_hash_get_current_key(Z_ARRVAL_P(mod), &modkey, &tmpUlong) != HASH_KEY_IS_STRING) {
2601-
zend_argument_type_error(3, "must only contain string-indexed arrays");
2602-
RETURN_THROWS();
2603-
}
2604-
2605-
/* is this a valid entry? */
2606-
if (
2607-
!_ldap_str_equal_to_const(ZSTR_VAL(modkey), ZSTR_LEN(modkey), LDAP_MODIFY_BATCH_ATTRIB) &&
2608-
!_ldap_str_equal_to_const(ZSTR_VAL(modkey), ZSTR_LEN(modkey), LDAP_MODIFY_BATCH_MODTYPE) &&
2609-
!_ldap_str_equal_to_const(ZSTR_VAL(modkey), ZSTR_LEN(modkey), LDAP_MODIFY_BATCH_VALUES)
2610-
) {
2611-
zend_argument_value_error(3, "must contain arrays only containing the \"" LDAP_MODIFY_BATCH_ATTRIB "\", \"" LDAP_MODIFY_BATCH_MODTYPE "\" and \"" LDAP_MODIFY_BATCH_VALUES "\" keys");
2612-
RETURN_THROWS();
2613-
}
2614-
2615-
fetched = zend_hash_get_current_data(Z_ARRVAL_P(mod));
2616-
zval *modinfo = fetched;
2617-
2618-
/* does the value type match the key? */
2619-
if (_ldap_str_equal_to_const(ZSTR_VAL(modkey), ZSTR_LEN(modkey), LDAP_MODIFY_BATCH_ATTRIB)) {
2620-
has_attrib_key = true;
2621-
if (Z_TYPE_P(modinfo) != IS_STRING) {
2622-
zend_type_error("%s(): Option \"" LDAP_MODIFY_BATCH_ATTRIB "\" must be of type string, %s given", get_active_function_name(), zend_zval_value_name(modinfo));
2623-
RETURN_THROWS();
2624-
}
2567+
SEPARATE_ARRAY(modification_zv);
2568+
const HashTable *modification = Z_ARRVAL_P(modification_zv);
2569+
uint32_t modification_size = zend_hash_num_elements(modification);
26252570

2626-
if (zend_str_has_nul_byte(Z_STR_P(modinfo))) {
2627-
zend_argument_value_error(3, "the value for option \"" LDAP_MODIFY_BATCH_ATTRIB "\" must not contain null bytes");
2628-
RETURN_THROWS();
2629-
}
2630-
}
2631-
else if (_ldap_str_equal_to_const(ZSTR_VAL(modkey), ZSTR_LEN(modkey), LDAP_MODIFY_BATCH_MODTYPE)) {
2632-
has_modtype_key = true;
2633-
if (Z_TYPE_P(modinfo) != IS_LONG) {
2634-
zend_type_error("%s(): Option \"" LDAP_MODIFY_BATCH_MODTYPE "\" must be of type int, %s given", get_active_function_name(), zend_zval_value_name(modinfo));
2635-
RETURN_THROWS();
2636-
}
2571+
if (modification_size != 2 && modification_size != 3) {
2572+
zend_argument_value_error(3, "a modification entry must only contain the keys "
2573+
"\"" LDAP_MODIFY_BATCH_ATTRIB "\", \"" LDAP_MODIFY_BATCH_MODTYPE "\", and \"" LDAP_MODIFY_BATCH_VALUES "\"");
2574+
RETURN_THROWS();
2575+
}
26372576

2638-
/* is the value in range? */
2639-
zend_long modtype = Z_LVAL_P(modinfo);
2640-
if (
2641-
modtype != LDAP_MODIFY_BATCH_ADD &&
2642-
modtype != LDAP_MODIFY_BATCH_REMOVE &&
2643-
modtype != LDAP_MODIFY_BATCH_REPLACE &&
2644-
modtype != LDAP_MODIFY_BATCH_REMOVE_ALL
2645-
) {
2646-
zend_value_error("%s(): Option \"" LDAP_MODIFY_BATCH_MODTYPE "\" must be one of the LDAP_MODIFY_BATCH_* constants", get_active_function_name());
2647-
RETURN_THROWS();
2648-
}
2577+
const zval *attrib = zend_hash_str_find(modification, LDAP_MODIFY_BATCH_ATTRIB, strlen(LDAP_MODIFY_BATCH_ATTRIB));
2578+
if (UNEXPECTED(attrib == NULL)) {
2579+
zend_argument_value_error(3, "a modification entry must contain the \"" LDAP_MODIFY_BATCH_ATTRIB "\" option");
2580+
RETURN_THROWS();
2581+
}
2582+
if (UNEXPECTED(Z_TYPE_P(attrib) != IS_STRING)) {
2583+
zend_argument_type_error(3, "the value for option \"" LDAP_MODIFY_BATCH_ATTRIB "\" must be of type string, %s given", zend_zval_value_name(attrib));
2584+
RETURN_THROWS();
2585+
}
2586+
if (zend_str_has_nul_byte(Z_STR_P(attrib))) {
2587+
zend_argument_value_error(3, "the value for option \"" LDAP_MODIFY_BATCH_ATTRIB "\" must not contain null bytes");
2588+
RETURN_THROWS();
2589+
}
26492590

2650-
/* if it's REMOVE_ALL, there must not be a values array; otherwise, there must */
2651-
if (modtype == LDAP_MODIFY_BATCH_REMOVE_ALL) {
2652-
if (zend_hash_str_exists(Z_ARRVAL_P(mod), LDAP_MODIFY_BATCH_VALUES, strlen(LDAP_MODIFY_BATCH_VALUES))) {
2653-
zend_value_error("%s(): If option \"" LDAP_MODIFY_BATCH_MODTYPE "\" is LDAP_MODIFY_BATCH_REMOVE_ALL, option \"" LDAP_MODIFY_BATCH_VALUES "\" cannot be provided", get_active_function_name());
2654-
RETURN_THROWS();
2655-
}
2656-
}
2657-
else {
2658-
if (!zend_hash_str_exists(Z_ARRVAL_P(mod), LDAP_MODIFY_BATCH_VALUES, strlen(LDAP_MODIFY_BATCH_VALUES))) {
2659-
zend_value_error("%s(): If option \"" LDAP_MODIFY_BATCH_MODTYPE "\" is not LDAP_MODIFY_BATCH_REMOVE_ALL, option \"" LDAP_MODIFY_BATCH_VALUES "\" must be provided", get_active_function_name());
2660-
RETURN_THROWS();
2661-
}
2662-
}
2663-
}
2664-
else if (_ldap_str_equal_to_const(ZSTR_VAL(modkey), ZSTR_LEN(modkey), LDAP_MODIFY_BATCH_VALUES)) {
2665-
if (Z_TYPE_P(modinfo) != IS_ARRAY) {
2666-
zend_type_error("%s(): Option \"" LDAP_MODIFY_BATCH_VALUES "\" must be of type array, %s given", get_active_function_name(), zend_zval_value_name(modinfo));
2667-
RETURN_THROWS();
2668-
}
2591+
const zval *modtype_zv = zend_hash_str_find(modification, LDAP_MODIFY_BATCH_MODTYPE, strlen(LDAP_MODIFY_BATCH_MODTYPE));
2592+
if (UNEXPECTED(modtype_zv == NULL)) {
2593+
zend_argument_value_error(3, "a modification entry must contain the \"" LDAP_MODIFY_BATCH_MODTYPE "\" option");
2594+
RETURN_THROWS();
2595+
}
2596+
if (UNEXPECTED(Z_TYPE_P(modtype_zv) != IS_LONG)) {
2597+
zend_argument_type_error(3, "the value for option \"" LDAP_MODIFY_BATCH_MODTYPE "\" must be of type int, %s given", zend_zval_value_name(attrib));
2598+
RETURN_THROWS();
2599+
}
2600+
zend_long modtype = Z_LVAL_P(modtype_zv);
2601+
if (
2602+
modtype != LDAP_MODIFY_BATCH_ADD &&
2603+
modtype != LDAP_MODIFY_BATCH_REMOVE &&
2604+
modtype != LDAP_MODIFY_BATCH_REPLACE &&
2605+
modtype != LDAP_MODIFY_BATCH_REMOVE_ALL
2606+
) {
2607+
zend_argument_value_error(3, "the value for option \"" LDAP_MODIFY_BATCH_MODTYPE "\" must be"
2608+
" LDAP_MODIFY_BATCH_ADD, LDAP_MODIFY_BATCH_REMOVE, LDAP_MODIFY_BATCH_REPLACE,"
2609+
" or LDAP_MODIFY_BATCH_REMOVE_ALL");
2610+
RETURN_THROWS();
2611+
}
2612+
/* We assume that the modification array is well-formed and only ever contains an extra "values" key */
2613+
if (modtype == LDAP_MODIFY_BATCH_REMOVE_ALL && modification_size == 3) {
2614+
zend_argument_value_error(3, "a modification entry must not contain the "
2615+
"\"" LDAP_MODIFY_BATCH_VALUES "\" option when option \"" LDAP_MODIFY_BATCH_MODTYPE "\" "
2616+
"is LDAP_MODIFY_BATCH_REMOVE_ALL");
2617+
RETURN_THROWS();
2618+
}
26692619

2670-
SEPARATE_ARRAY(modinfo);
2671-
const HashTable *modification_values = Z_ARRVAL_P(modinfo);
2672-
/* is the array not empty? */
2673-
uint32_t num_modification_values = zend_hash_num_elements(modification_values);
2674-
if (num_modification_values == 0) {
2675-
zend_argument_value_error(3, "the value for option \"" LDAP_MODIFY_BATCH_VALUES "\" must not be empty");
2676-
RETURN_THROWS();
2677-
}
2678-
if (!zend_array_is_list(modification_values)) {
2679-
zend_argument_value_error(3, "the value for option \"" LDAP_MODIFY_BATCH_VALUES "\" must be a list");
2680-
RETURN_THROWS();
2681-
}
2620+
zval *modification_values_zv = zend_hash_str_find(modification, LDAP_MODIFY_BATCH_VALUES, strlen(LDAP_MODIFY_BATCH_VALUES));
2621+
if (modification_values_zv == NULL) {
2622+
if (modtype != LDAP_MODIFY_BATCH_REMOVE_ALL) {
2623+
zend_argument_value_error(3, "a modification entry must contain the "
2624+
"\"" LDAP_MODIFY_BATCH_VALUES "\" option when the \"" LDAP_MODIFY_BATCH_MODTYPE "\" option "
2625+
"is not LDAP_MODIFY_BATCH_REMOVE_ALL");
2626+
RETURN_THROWS();
26822627
}
2683-
2684-
zend_hash_move_forward(Z_ARRVAL_P(mod));
2628+
continue;
2629+
}
2630+
if (Z_TYPE_P(modification_values_zv) != IS_ARRAY) {
2631+
zend_argument_type_error(3, "the value for option \"" LDAP_MODIFY_BATCH_VALUES "\" must be of type array, %s given", zend_zval_value_name(attrib));
2632+
RETURN_THROWS();
26852633
}
26862634

2687-
if (!has_attrib_key) {
2688-
zend_value_error("%s(): Required option \"" LDAP_MODIFY_BATCH_ATTRIB "\" is missing", get_active_function_name());
2635+
SEPARATE_ARRAY(modification_values_zv);
2636+
const HashTable *modification_values = Z_ARRVAL_P(modification_values_zv);
2637+
/* is the array not empty? */
2638+
uint32_t num_modvals = zend_hash_num_elements(modification_values);
2639+
if (num_modvals == 0) {
2640+
zend_argument_value_error(3, "the value for option \"" LDAP_MODIFY_BATCH_VALUES "\" must not be empty");
26892641
RETURN_THROWS();
26902642
}
2691-
if (!has_modtype_key) {
2692-
zend_value_error("%s(): Required option \"" LDAP_MODIFY_BATCH_MODTYPE "\" is missing", get_active_function_name());
2643+
if (!zend_array_is_list(modification_values)) {
2644+
zend_argument_value_error(3, "the value for option \"" LDAP_MODIFY_BATCH_VALUES "\" must be a list");
26932645
RETURN_THROWS();
26942646
}
26952647
}

‎ext/ldap/tests/gh16032-1.phpt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,4 +23,4 @@ try {
2323

2424
?>
2525
--EXPECT--
26-
ValueError: ldap_modify_batch(): Required option "attrib" is missing
26+
ValueError: ldap_modify_batch(): Argument #3 ($modifications_info) a modification entry must contain the "attrib" option

‎ext/ldap/tests/gh16032-2.phpt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,4 +23,4 @@ try {
2323

2424
?>
2525
--EXPECT--
26-
ValueError: ldap_modify_batch(): Required option "modtype" is missing
26+
ValueError: ldap_modify_batch(): Argument #3 ($modifications_info) a modification entry must contain the "modtype" option

‎ext/ldap/tests/ldap_modify_batch_programming_error.phpt

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -259,18 +259,18 @@ ValueError: ldap_modify_batch(): Argument #2 ($dn) must not contain null bytes
259259
TypeError: ldap_modify_batch(): Argument #3 ($modifications_info) must be integer-indexed
260260
TypeError: ldap_modify_batch(): Argument #3 ($modifications_info) must be integer-indexed
261261
ValueError: ldap_modify_batch(): Argument #3 ($modifications_info) must have consecutive integer indices starting from 0
262-
ValueError: ldap_modify_batch(): Argument #3 ($modifications_info) must only contain arrays
263-
TypeError: ldap_modify_batch(): Argument #3 ($modifications_info) must only contain string-indexed arrays
264-
ValueError: ldap_modify_batch(): Argument #3 ($modifications_info) must contain arrays only containing the "attrib", "modtype" and "values" keys
265-
TypeError: ldap_modify_batch(): Option "attrib" must be of type string, int given
262+
TypeError: ldap_modify_batch(): Argument #3 ($modifications_info) must only contain arrays
263+
ValueError: ldap_modify_batch(): Argument #3 ($modifications_info) a modification entry must not contain the "values" option when option "modtype" is LDAP_MODIFY_BATCH_REMOVE_ALL
264+
ValueError: ldap_modify_batch(): Argument #3 ($modifications_info) a modification entry must only contain the keys "attrib", "modtype", and "values"
265+
TypeError: ldap_modify_batch(): Argument #3 ($modifications_info) the value for option "attrib" must be of type string, int given
266266
ValueError: ldap_modify_batch(): Argument #3 ($modifications_info) the value for option "attrib" must not contain null bytes
267-
TypeError: ldap_modify_batch(): Option "modtype" must be of type int, stdClass given
268-
ValueError: ldap_modify_batch(): Option "modtype" must be one of the LDAP_MODIFY_BATCH_* constants
269-
ValueError: ldap_modify_batch(): If option "modtype" is LDAP_MODIFY_BATCH_REMOVE_ALL, option "values" cannot be provided
270-
ValueError: ldap_modify_batch(): If option "modtype" is not LDAP_MODIFY_BATCH_REMOVE_ALL, option "values" must be provided
271-
TypeError: ldap_modify_batch(): Option "values" must be of type array, string given
267+
TypeError: ldap_modify_batch(): Argument #3 ($modifications_info) the value for option "modtype" must be of type int, string given
268+
ValueError: ldap_modify_batch(): Argument #3 ($modifications_info) the value for option "modtype" must be LDAP_MODIFY_BATCH_ADD, LDAP_MODIFY_BATCH_REMOVE, LDAP_MODIFY_BATCH_REPLACE, or LDAP_MODIFY_BATCH_REMOVE_ALL
269+
ValueError: ldap_modify_batch(): Argument #3 ($modifications_info) a modification entry must not contain the "values" option when option "modtype" is LDAP_MODIFY_BATCH_REMOVE_ALL
270+
ValueError: ldap_modify_batch(): Argument #3 ($modifications_info) a modification entry must contain the "values" option when the "modtype" option is not LDAP_MODIFY_BATCH_REMOVE_ALL
271+
TypeError: ldap_modify_batch(): Argument #3 ($modifications_info) the value for option "values" must be of type array, string given
272272
ValueError: ldap_modify_batch(): Argument #3 ($modifications_info) the value for option "values" must not be empty
273273
ValueError: ldap_modify_batch(): Argument #3 ($modifications_info) the value for option "values" must be a list
274274
ValueError: ldap_modify_batch(): Argument #3 ($modifications_info) the value for option "values" must be a list
275-
ValueError: ldap_modify_batch(): Required option "attrib" is missing
276-
ValueError: ldap_modify_batch(): Required option "modtype" is missing
275+
ValueError: ldap_modify_batch(): Argument #3 ($modifications_info) a modification entry must contain the "attrib" option
276+
ValueError: ldap_modify_batch(): Argument #3 ($modifications_info) a modification entry must contain the "modtype" option

0 commit comments

Comments
 (0)