Skip to content

Commit 9bc2a03

Browse files
GirgiasGirgias
committed
ext/ldap: Use HASH_FOREACH macro to traverse array
1 parent 4c98b2c commit 9bc2a03

File tree

2 files changed

+85
-32
lines changed

2 files changed

+85
-32
lines changed

‎ext/ldap/ldap.c

Lines changed: 40 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1485,10 +1485,6 @@ static void php_ldap_do_search(INTERNAL_FUNCTION_PARAMETERS, int scope)
14851485

14861486
/* parallel search? */
14871487
if (Z_TYPE_P(link) == IS_ARRAY) {
1488-
int i, *rcs;
1489-
ldap_linkdata **lds;
1490-
zval *entry, object;
1491-
14921488
uint32_t num_links = zend_hash_num_elements(Z_ARRVAL_P(link));
14931489
if (num_links == 0) {
14941490
zend_argument_must_not_be_empty_error(1);
@@ -1508,7 +1504,6 @@ static void php_ldap_do_search(INTERNAL_FUNCTION_PARAMETERS, int scope)
15081504
ret = 0;
15091505
goto cleanup;
15101506
}
1511-
zend_hash_internal_pointer_reset(base_dn_ht);
15121507
} else {
15131508
if (zend_str_has_nul_byte(base_dn_str)) {
15141509
zend_argument_value_error(2, "must not contain null bytes");
@@ -1530,7 +1525,6 @@ static void php_ldap_do_search(INTERNAL_FUNCTION_PARAMETERS, int scope)
15301525
ret = 0;
15311526
goto cleanup;
15321527
}
1533-
zend_hash_internal_pointer_reset(filter_ht);
15341528
} else {
15351529
if (zend_str_has_nul_byte(filter_str)) {
15361530
zend_argument_value_error(3, "must not contain null bytes");
@@ -1540,73 +1534,87 @@ static void php_ldap_do_search(INTERNAL_FUNCTION_PARAMETERS, int scope)
15401534
ldap_filter = zend_string_copy(filter_str);
15411535
}
15421536

1537+
int *rcs;
1538+
ldap_linkdata **lds;
15431539
lds = safe_emalloc(num_links, sizeof(ldap_linkdata), 0);
15441540
rcs = safe_emalloc(num_links, sizeof(*rcs), 0);
15451541

1546-
zend_hash_internal_pointer_reset(Z_ARRVAL_P(link));
1547-
for (i=0; i<num_links; i++) {
1548-
entry = zend_hash_get_current_data(Z_ARRVAL_P(link));
1549-
1550-
if (Z_TYPE_P(entry) != IS_OBJECT || !instanceof_function(Z_OBJCE_P(entry), ldap_link_ce)) {
1551-
zend_argument_value_error(1, "must only contain objects of type LDAP");
1542+
zend_ulong ldap_link_index = 0;
1543+
zval *link_zv = NULL;
1544+
ZEND_HASH_FOREACH_NUM_KEY_VAL(Z_ARRVAL_P(link), ldap_link_index, link_zv) {
1545+
if (Z_TYPE_P(link_zv) != IS_OBJECT || !instanceof_function(Z_OBJCE_P(link_zv), ldap_link_ce)) {
1546+
zend_argument_value_error(1, "must be a list of LDAP\\Connection");
15521547
ret = 0;
15531548
goto cleanup_parallel;
15541549
}
15551550

1556-
ld = Z_LDAP_LINK_P(entry);
1557-
if (!ld->link) {
1551+
ldap_linkdata *current_ld = Z_LDAP_LINK_P(link_zv);
1552+
if (!current_ld->link) {
15581553
zend_throw_error(NULL, "LDAP connection has already been closed");
15591554
ret = 0;
15601555
goto cleanup_parallel;
15611556
}
15621557

15631558
if (num_base_dns != 0) { /* base_dn an array? */
1564-
entry = zend_hash_get_current_data(base_dn_ht);
1565-
zend_hash_move_forward(base_dn_ht);
1566-
ldap_base_dn = zval_get_string(entry);
1567-
if (EG(exception)) {
1559+
zval *base_dn_zv = zend_hash_index_find(base_dn_ht, ldap_link_index);
1560+
ZEND_ASSERT(base_dn_zv);
1561+
if (Z_TYPE_P(base_dn_zv) != IS_STRING) {
1562+
zend_argument_type_error(2, "must be a list of strings, %s given", zend_zval_value_name(base_dn_zv));
1563+
ret = 0;
1564+
goto cleanup_parallel;
1565+
}
1566+
ldap_base_dn = zend_string_copy(Z_STR_P(base_dn_zv));
1567+
if (zend_str_has_nul_byte(ldap_base_dn)) {
1568+
zend_argument_value_error(2, "must not contain null bytes");
15681569
ret = 0;
15691570
goto cleanup_parallel;
15701571
}
1571-
// TODO check dn does not have any nul bytes
15721572
}
15731573
if (num_filters != 0) { /* filter an array? */
1574-
entry = zend_hash_get_current_data(filter_ht);
1575-
zend_hash_move_forward(filter_ht);
1576-
ldap_filter = zval_get_string(entry);
1577-
if (EG(exception)) {
1574+
zval *filter_zv = zend_hash_index_find(filter_ht, ldap_link_index);
1575+
ZEND_ASSERT(filter_zv);
1576+
if (Z_TYPE_P(filter_zv) != IS_STRING) {
1577+
zend_argument_type_error(3, "must be a list of strings, %s given", zend_zval_value_name(filter_zv));
1578+
ret = 0;
1579+
goto cleanup_parallel;
1580+
}
1581+
ldap_filter = zend_string_copy(Z_STR_P(filter_zv));
1582+
if (zend_str_has_nul_byte(ldap_filter)) {
1583+
zend_argument_value_error(3, "must not contain null bytes");
15781584
ret = 0;
15791585
goto cleanup_parallel;
15801586
}
1581-
// TODO check filter does not have any nul bytes
15821587
}
15831588

15841589
if (serverctrls) {
15851590
/* We have to parse controls again for each link as they use it */
15861591
_php_ldap_controls_free(&lserverctrls);
1587-
lserverctrls = _php_ldap_controls_from_array(ld->link, serverctrls, 9);
1592+
lserverctrls = _php_ldap_controls_from_array(current_ld->link, serverctrls, 9);
15881593
if (lserverctrls == NULL) {
1589-
rcs[i] = -1;
1594+
rcs[ldap_link_index] = -1;
1595+
// TODO Throw an exception/cleanup?
15901596
continue;
15911597
}
15921598
}
15931599

1594-
php_set_opts(ld->link, ldap_sizelimit, ldap_timelimit, ldap_deref, &old_ldap_sizelimit, &old_ldap_timelimit, &old_ldap_deref);
1600+
php_set_opts(current_ld->link, ldap_sizelimit, ldap_timelimit, ldap_deref, &old_ldap_sizelimit, &old_ldap_timelimit, &old_ldap_deref);
15951601

15961602
/* Run the actual search */
1597-
ldap_search_ext(ld->link, ZSTR_VAL(ldap_base_dn), scope, ZSTR_VAL(ldap_filter), ldap_attrs, ldap_attrsonly, lserverctrls, NULL, NULL, ldap_sizelimit, &rcs[i]);
1598-
lds[i] = ld;
1599-
zend_hash_move_forward(Z_ARRVAL_P(link));
1600-
}
1603+
ldap_search_ext(current_ld->link, ZSTR_VAL(ldap_base_dn), scope, ZSTR_VAL(ldap_filter), ldap_attrs, ldap_attrsonly, lserverctrls, NULL, NULL, ldap_sizelimit, &rcs[ldap_link_index]);
1604+
lds[ldap_link_index] = current_ld;
1605+
1606+
// TODO Reset the options of the link?
1607+
} ZEND_HASH_FOREACH_END();
16011608

16021609
array_init(return_value);
16031610

16041611
/* Collect results from the searches */
1605-
for (i=0; i<num_links; i++) {
1612+
for (uint32_t i = 0; i < num_links; i++) {
16061613
if (rcs[i] != -1) {
16071614
rcs[i] = ldap_result(lds[i]->link, LDAP_RES_ANY, 1 /* LDAP_MSG_ALL */, NULL, &ldap_res);
16081615
}
16091616
if (rcs[i] != -1) {
1617+
zval object;
16101618
object_init_ex(&object, ldap_result_ce);
16111619
result = Z_LDAP_RESULT_P(&object);
16121620
result->result = ldap_res;

‎ext/ldap/tests/ldap_list_read_search_parallel_programming_errors.phpt

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,16 @@ $valid_filter = "";
1313

1414
$ldaps = [$ldap, $ldap];
1515

16+
$not_list_ldaps = [
17+
"string1",
18+
$ldap,
19+
];
20+
try {
21+
var_dump(ldap_list($not_list_ldaps, $valid_dn, $valid_filter));
22+
} catch (Throwable $e) {
23+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
24+
}
25+
1626
$dn = "dn_with\0nul_byte";
1727
try {
1828
var_dump(ldap_list($ldaps, $dn, $valid_filter));
@@ -58,11 +68,46 @@ try {
5868
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
5969
}
6070

71+
$list_not_all_strings = [
72+
42,
73+
"string2",
74+
];
75+
try {
76+
var_dump(ldap_list($ldaps, $list_not_all_strings, $valid_filter));
77+
} catch (Throwable $e) {
78+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
79+
}
80+
try {
81+
var_dump(ldap_list($ldaps, $valid_dn, $list_not_all_strings));
82+
} catch (Throwable $e) {
83+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
84+
}
85+
86+
$list_strings_with_nul_bytes = [
87+
"string\0nul_byte",
88+
"string2",
89+
];
90+
try {
91+
var_dump(ldap_list($ldaps, $list_strings_with_nul_bytes, $valid_filter));
92+
} catch (Throwable $e) {
93+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
94+
}
95+
try {
96+
var_dump(ldap_list($ldaps, $valid_dn, $list_strings_with_nul_bytes));
97+
} catch (Throwable $e) {
98+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
99+
}
100+
61101
?>
62102
--EXPECT--
103+
ValueError: ldap_list(): Argument #1 ($ldap) must be a list of LDAP\Connection
63104
ValueError: ldap_list(): Argument #2 ($base) must not contain null bytes
64105
ValueError: ldap_list(): Argument #3 ($filter) must not contain null bytes
65106
ValueError: ldap_list(): Argument #2 ($base) must be a list
66107
ValueError: ldap_list(): Argument #3 ($filter) must be a list
67108
ValueError: ldap_list(): Argument #2 ($base) must be the same size as argument #1
68109
ValueError: ldap_list(): Argument #3 ($filter) must be the same size as argument #1
110+
TypeError: ldap_list(): Argument #2 ($base) must be a list of strings, int given
111+
TypeError: ldap_list(): Argument #3 ($filter) must be a list of strings, int given
112+
ValueError: ldap_list(): Argument #2 ($base) must not contain null bytes
113+
ValueError: ldap_list(): Argument #3 ($filter) must not contain null bytes

0 commit comments

Comments
 (0)