Skip to content

Commit 21955a9

Browse files
GirgiasGirgias
committed
ext/ldap: Refactor looping of modifications array
1 parent 8188e64 commit 21955a9

File tree

2 files changed

+39
-49
lines changed

2 files changed

+39
-49
lines changed

‎ext/ldap/ldap.c

Lines changed: 36 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -2495,12 +2495,10 @@ PHP_FUNCTION(ldap_delete_ext)
24952495
PHP_FUNCTION(ldap_modify_batch)
24962496
{
24972497
zval *serverctrls = NULL;
2498-
zval *link, *mods, *mod;
2499-
zval *fetched;
2498+
zval *link;
25002499
char *dn;
25012500
size_t dn_len;
2502-
int i, j;
2503-
int num_mods;
2501+
HashTable *modifications;
25042502
LDAPControl **lserverctrls = NULL;
25052503

25062504
/*
@@ -2527,7 +2525,7 @@ PHP_FUNCTION(ldap_modify_batch)
25272525
];
25282526
*/
25292527

2530-
if (zend_parse_parameters(ZEND_NUM_ARGS(), "Osa/|a!", &link, ldap_link_ce, &dn, &dn_len, &mods, &serverctrls) != SUCCESS) {
2528+
if (zend_parse_parameters(ZEND_NUM_ARGS(), "Osh/|a!", &link, ldap_link_ce, &dn, &dn_len, &modifications, &serverctrls) != SUCCESS) {
25312529
RETURN_THROWS();
25322530
}
25332531

@@ -2543,22 +2541,17 @@ PHP_FUNCTION(ldap_modify_batch)
25432541
}
25442542

25452543
/* make sure the top level is a normal array */
2546-
zend_hash_internal_pointer_reset(Z_ARRVAL_P(mods));
2547-
if (zend_hash_get_current_key_type(Z_ARRVAL_P(mods)) != HASH_KEY_IS_LONG) {
2548-
zend_argument_type_error(3, "must be integer-indexed");
2544+
if (zend_hash_num_elements(modifications) == 0) {
2545+
zend_argument_must_not_be_empty_error(3);
2546+
RETURN_THROWS();
2547+
}
2548+
if (!zend_array_is_list(modifications)) {
2549+
zend_argument_value_error(3, "must be a list");
25492550
RETURN_THROWS();
25502551
}
25512552

2552-
num_mods = zend_hash_num_elements(Z_ARRVAL_P(mods));
2553-
2554-
for (i = 0; i < num_mods; i++) {
2555-
/* is the numbering consecutive? */
2556-
if ((fetched = zend_hash_index_find(Z_ARRVAL_P(mods), i)) == NULL) {
2557-
zend_argument_value_error(3, "must have consecutive integer indices starting from 0");
2558-
RETURN_THROWS();
2559-
}
2560-
zval *modification_zv = fetched;
2561-
2553+
zval *modification_zv = NULL;
2554+
ZEND_HASH_FOREACH_VAL(modifications, modification_zv) {
25622555
if (Z_TYPE_P(modification_zv) != IS_ARRAY) {
25632556
zend_argument_type_error(3, "must only contain arrays");
25642557
RETURN_THROWS();
@@ -2644,27 +2637,25 @@ PHP_FUNCTION(ldap_modify_batch)
26442637
zend_argument_value_error(3, "the value for option \"" LDAP_MODIFY_BATCH_VALUES "\" must be a list");
26452638
RETURN_THROWS();
26462639
}
2647-
}
2640+
} ZEND_HASH_FOREACH_END();
26482641
}
26492642
/* validation was successful */
26502643

26512644
/* allocate array of modifications */
2645+
uint32_t num_mods = zend_hash_num_elements(modifications);
26522646
LDAPMod **ldap_mods = safe_emalloc((num_mods+1), sizeof(LDAPMod *), 0);
26532647

26542648
/* for each modification */
2655-
for (i = 0; i < num_mods; i++) {
2656-
/* allocate the modification struct */
2657-
ldap_mods[i] = safe_emalloc(1, sizeof(LDAPMod), 0);
2658-
2659-
/* fetch the relevant data */
2660-
fetched = zend_hash_index_find(Z_ARRVAL_P(mods), i);
2661-
mod = fetched;
2649+
zend_ulong modification_index = 0;
2650+
zval *modification_zv = NULL;
2651+
ZEND_HASH_FOREACH_NUM_KEY_VAL(modifications, modification_index, modification_zv) {
2652+
ldap_mods[modification_index] = safe_emalloc(1, sizeof(LDAPMod), 0);
26622653

2663-
zval *attrib_zv = zend_hash_str_find(Z_ARRVAL_P(mod), LDAP_MODIFY_BATCH_ATTRIB, strlen(LDAP_MODIFY_BATCH_ATTRIB));
2654+
zval *attrib_zv = zend_hash_str_find(Z_ARRVAL_P(modification_zv), LDAP_MODIFY_BATCH_ATTRIB, strlen(LDAP_MODIFY_BATCH_ATTRIB));
26642655
ZEND_ASSERT(Z_TYPE_P(attrib_zv) == IS_STRING);
2665-
zval *modtype_zv = zend_hash_str_find(Z_ARRVAL_P(mod), LDAP_MODIFY_BATCH_MODTYPE, strlen(LDAP_MODIFY_BATCH_MODTYPE));
2656+
zval *modtype_zv = zend_hash_str_find(Z_ARRVAL_P(modification_zv), LDAP_MODIFY_BATCH_MODTYPE, strlen(LDAP_MODIFY_BATCH_MODTYPE));
26662657
ZEND_ASSERT(Z_TYPE_P(modtype_zv) == IS_LONG);
2667-
zval *modification_values = zend_hash_str_find(Z_ARRVAL_P(mod), LDAP_MODIFY_BATCH_VALUES, strlen(LDAP_MODIFY_BATCH_VALUES));
2658+
zval *modification_values = zend_hash_str_find(Z_ARRVAL_P(modification_zv), LDAP_MODIFY_BATCH_VALUES, strlen(LDAP_MODIFY_BATCH_VALUES));
26682659
ZEND_ASSERT(modification_values == NULL || Z_TYPE_P(modification_values) == IS_ARRAY);
26692660

26702661
/* map the modification type */
@@ -2684,43 +2675,42 @@ PHP_FUNCTION(ldap_modify_batch)
26842675
}
26852676

26862677
/* fill in the basic info */
2687-
ldap_mods[i]->mod_op = ldap_operation | LDAP_MOD_BVALUES;
2688-
ldap_mods[i]->mod_type = estrndup(Z_STRVAL_P(attrib_zv), Z_STRLEN_P(attrib_zv));
2678+
ldap_mods[modification_index]->mod_op = ldap_operation | LDAP_MOD_BVALUES;
2679+
ldap_mods[modification_index]->mod_type = estrndup(Z_STRVAL_P(attrib_zv), Z_STRLEN_P(attrib_zv));
26892680

26902681
if (Z_LVAL_P(modtype_zv) == LDAP_MODIFY_BATCH_REMOVE_ALL) {
26912682
/* no values */
2692-
ldap_mods[i]->mod_bvalues = NULL;
2693-
}
2694-
else {
2683+
ldap_mods[modification_index]->mod_bvalues = NULL;
2684+
} else {
26952685
/* allocate space for the values as part of this modification */
26962686
uint32_t num_modification_values = zend_hash_num_elements(Z_ARRVAL_P(modification_values));
2697-
ldap_mods[i]->mod_bvalues = safe_emalloc((num_modification_values+1), sizeof(struct berval *), 0);
2687+
ldap_mods[modification_index]->mod_bvalues = safe_emalloc((num_modification_values+1), sizeof(struct berval *), 0);
26982688

26992689
/* for each value */
2700-
for (j = 0; j < num_modification_values; j++) {
2690+
for (uint32_t j = 0; j < num_modification_values; j++) {
27012691
/* fetch it */
2702-
fetched = zend_hash_index_find(Z_ARRVAL_P(modification_values), j);
2692+
zval *fetched = zend_hash_index_find(Z_ARRVAL_P(modification_values), j);
27032693
zend_string *modval = zval_get_string(fetched);
27042694
if (EG(exception)) {
27052695
RETVAL_FALSE;
2706-
ldap_mods[i]->mod_bvalues[j] = NULL;
2707-
num_mods = i + 1;
2696+
ldap_mods[modification_index]->mod_bvalues[j] = NULL;
2697+
num_mods = modification_index + 1;
27082698
goto cleanup;
27092699
}
27102700

27112701
/* allocate the data struct */
2712-
ldap_mods[i]->mod_bvalues[j] = safe_emalloc(1, sizeof(struct berval), 0);
2702+
ldap_mods[modification_index]->mod_bvalues[j] = safe_emalloc(1, sizeof(struct berval), 0);
27132703

27142704
/* fill it */
2715-
ldap_mods[i]->mod_bvalues[j]->bv_len = ZSTR_LEN(modval);
2716-
ldap_mods[i]->mod_bvalues[j]->bv_val = estrndup(ZSTR_VAL(modval), ZSTR_LEN(modval));
2705+
ldap_mods[modification_index]->mod_bvalues[j]->bv_len = ZSTR_LEN(modval);
2706+
ldap_mods[modification_index]->mod_bvalues[j]->bv_val = estrndup(ZSTR_VAL(modval), ZSTR_LEN(modval));
27172707
zend_string_release(modval);
27182708
}
27192709

27202710
/* NULL-terminate values */
2721-
ldap_mods[i]->mod_bvalues[num_modification_values] = NULL;
2711+
ldap_mods[modification_index]->mod_bvalues[num_modification_values] = NULL;
27222712
}
2723-
}
2713+
} ZEND_HASH_FOREACH_END();
27242714

27252715
/* NULL-terminate modifications */
27262716
ldap_mods[num_mods] = NULL;
@@ -2744,13 +2734,13 @@ PHP_FUNCTION(ldap_modify_batch)
27442734

27452735
/* clean up */
27462736
cleanup: {
2747-
for (i = 0; i < num_mods; i++) {
2737+
for (uint32_t i = 0; i < num_mods; i++) {
27482738
/* attribute */
27492739
efree(ldap_mods[i]->mod_type);
27502740

27512741
if (ldap_mods[i]->mod_bvalues != NULL) {
27522742
/* each BER value */
2753-
for (j = 0; ldap_mods[i]->mod_bvalues[j] != NULL; j++) {
2743+
for (int j = 0; ldap_mods[i]->mod_bvalues[j] != NULL; j++) {
27542744
/* free the data bytes */
27552745
efree(ldap_mods[i]->mod_bvalues[j]->bv_val);
27562746

‎ext/ldap/tests/ldap_modify_batch_programming_error.phpt

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -256,9 +256,9 @@ try {
256256
?>
257257
--EXPECT--
258258
ValueError: ldap_modify_batch(): Argument #2 ($dn) must not contain null bytes
259-
TypeError: ldap_modify_batch(): Argument #3 ($modifications_info) must be integer-indexed
260-
TypeError: ldap_modify_batch(): Argument #3 ($modifications_info) must be integer-indexed
261-
ValueError: ldap_modify_batch(): Argument #3 ($modifications_info) must have consecutive integer indices starting from 0
259+
ValueError: ldap_modify_batch(): Argument #3 ($modifications_info) must not be empty
260+
ValueError: ldap_modify_batch(): Argument #3 ($modifications_info) must be a list
261+
ValueError: ldap_modify_batch(): Argument #3 ($modifications_info) must be a list
262262
TypeError: ldap_modify_batch(): Argument #3 ($modifications_info) must only contain arrays
263263
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
264264
ValueError: ldap_modify_batch(): Argument #3 ($modifications_info) a modification entry must only contain the keys "attrib", "modtype", and "values"

0 commit comments

Comments
 (0)