Skip to content

Commit 3f150f3

Browse files
GirgiasGirgias
committed
ext/ldap: Use HASH_FOREACH macro to traverse array
1 parent fa100e1 commit 3f150f3

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);
@@ -1514,7 +1510,6 @@ static void php_ldap_do_search(INTERNAL_FUNCTION_PARAMETERS, int scope)
15141510
ret = 0;
15151511
goto cleanup;
15161512
}
1517-
zend_hash_internal_pointer_reset(base_dn_ht);
15181513
} else {
15191514
if (zend_str_has_nul_byte(base_dn_str)) {
15201515
zend_argument_value_error(2, "must not contain null bytes");
@@ -1537,7 +1532,6 @@ static void php_ldap_do_search(INTERNAL_FUNCTION_PARAMETERS, int scope)
15371532
ret = 0;
15381533
goto cleanup;
15391534
}
1540-
zend_hash_internal_pointer_reset(filter_ht);
15411535
} else {
15421536
if (zend_str_has_nul_byte(filter_str)) {
15431537
zend_argument_value_error(3, "must not contain null bytes");
@@ -1547,73 +1541,87 @@ static void php_ldap_do_search(INTERNAL_FUNCTION_PARAMETERS, int scope)
15471541
ldap_filter = zend_string_copy(filter_str);
15481542
}
15491543

1544+
int *rcs;
1545+
ldap_linkdata **lds;
15501546
lds = safe_emalloc(num_links, sizeof(ldap_linkdata), 0);
15511547
rcs = safe_emalloc(num_links, sizeof(*rcs), 0);
15521548

1553-
zend_hash_internal_pointer_reset(Z_ARRVAL_P(link));
1554-
for (i=0; i<num_links; i++) {
1555-
entry = zend_hash_get_current_data(Z_ARRVAL_P(link));
1556-
1557-
if (Z_TYPE_P(entry) != IS_OBJECT || !instanceof_function(Z_OBJCE_P(entry), ldap_link_ce)) {
1558-
zend_argument_value_error(1, "must only contain objects of type LDAP");
1549+
zend_ulong ldap_link_index = 0;
1550+
zval *link_zv = NULL;
1551+
ZEND_HASH_FOREACH_NUM_KEY_VAL(Z_ARRVAL_P(link), ldap_link_index, link_zv) {
1552+
if (Z_TYPE_P(link_zv) != IS_OBJECT || !instanceof_function(Z_OBJCE_P(link_zv), ldap_link_ce)) {
1553+
zend_argument_value_error(1, "must be a list of LDAP\\Connection");
15591554
ret = 0;
15601555
goto cleanup_parallel;
15611556
}
15621557

1563-
ld = Z_LDAP_LINK_P(entry);
1564-
if (!ld->link) {
1558+
ldap_linkdata *current_ld = Z_LDAP_LINK_P(link_zv);
1559+
if (!current_ld->link) {
15651560
zend_throw_error(NULL, "LDAP connection has already been closed");
15661561
ret = 0;
15671562
goto cleanup_parallel;
15681563
}
15691564

15701565
if (num_base_dns != 0) { /* base_dn an array? */
1571-
entry = zend_hash_get_current_data(base_dn_ht);
1572-
zend_hash_move_forward(base_dn_ht);
1573-
ldap_base_dn = zval_get_string(entry);
1574-
if (EG(exception)) {
1566+
zval *base_dn_zv = zend_hash_index_find(base_dn_ht, ldap_link_index);
1567+
ZEND_ASSERT(base_dn_zv);
1568+
if (Z_TYPE_P(base_dn_zv) != IS_STRING) {
1569+
zend_argument_type_error(2, "must be a list of strings, %s given", zend_zval_value_name(base_dn_zv));
1570+
ret = 0;
1571+
goto cleanup_parallel;
1572+
}
1573+
ldap_base_dn = zend_string_copy(Z_STR_P(base_dn_zv));
1574+
if (zend_str_has_nul_byte(ldap_base_dn)) {
1575+
zend_argument_value_error(2, "must not contain null bytes");
15751576
ret = 0;
15761577
goto cleanup_parallel;
15771578
}
1578-
// TODO check dn does not have any nul bytes
15791579
}
15801580
if (num_filters != 0) { /* filter an array? */
1581-
entry = zend_hash_get_current_data(filter_ht);
1582-
zend_hash_move_forward(filter_ht);
1583-
ldap_filter = zval_get_string(entry);
1584-
if (EG(exception)) {
1581+
zval *filter_zv = zend_hash_index_find(filter_ht, ldap_link_index);
1582+
ZEND_ASSERT(filter_zv);
1583+
if (Z_TYPE_P(filter_zv) != IS_STRING) {
1584+
zend_argument_type_error(3, "must be a list of strings, %s given", zend_zval_value_name(filter_zv));
1585+
ret = 0;
1586+
goto cleanup_parallel;
1587+
}
1588+
ldap_filter = zend_string_copy(Z_STR_P(filter_zv));
1589+
if (zend_str_has_nul_byte(ldap_filter)) {
1590+
zend_argument_value_error(3, "must not contain null bytes");
15851591
ret = 0;
15861592
goto cleanup_parallel;
15871593
}
1588-
// TODO check filter does not have any nul bytes
15891594
}
15901595

15911596
if (serverctrls) {
15921597
/* We have to parse controls again for each link as they use it */
15931598
_php_ldap_controls_free(&lserverctrls);
1594-
lserverctrls = _php_ldap_controls_from_array(ld->link, serverctrls, 9);
1599+
lserverctrls = _php_ldap_controls_from_array(current_ld->link, serverctrls, 9);
15951600
if (lserverctrls == NULL) {
1596-
rcs[i] = -1;
1601+
rcs[ldap_link_index] = -1;
1602+
// TODO Throw an exception/cleanup?
15971603
continue;
15981604
}
15991605
}
16001606

1601-
php_set_opts(ld->link, ldap_sizelimit, ldap_timelimit, ldap_deref, &old_ldap_sizelimit, &old_ldap_timelimit, &old_ldap_deref);
1607+
php_set_opts(current_ld->link, ldap_sizelimit, ldap_timelimit, ldap_deref, &old_ldap_sizelimit, &old_ldap_timelimit, &old_ldap_deref);
16021608

16031609
/* Run the actual search */
1604-
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]);
1605-
lds[i] = ld;
1606-
zend_hash_move_forward(Z_ARRVAL_P(link));
1607-
}
1610+
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]);
1611+
lds[ldap_link_index] = current_ld;
1612+
1613+
// TODO Reset the options of the link?
1614+
} ZEND_HASH_FOREACH_END();
16081615

16091616
array_init(return_value);
16101617

16111618
/* Collect results from the searches */
1612-
for (i=0; i<num_links; i++) {
1619+
for (uint32_t i = 0; i < num_links; i++) {
16131620
if (rcs[i] != -1) {
16141621
rcs[i] = ldap_result(lds[i]->link, LDAP_RES_ANY, 1 /* LDAP_MSG_ALL */, NULL, &ldap_res);
16151622
}
16161623
if (rcs[i] != -1) {
1624+
zval object;
16171625
object_init_ex(&object, ldap_result_ce);
16181626
result = Z_LDAP_RESULT_P(&object);
16191627
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)