Skip to content

Commit 5e2c179

Browse files
GirgiasGirgias
committed
ext/ldap: Use HASH_FOREACH macro to traverse array
1 parent 491fab0 commit 5e2c179

File tree

3 files changed

+125
-32
lines changed

3 files changed

+125
-32
lines changed

‎ext/ldap/ldap.c‎

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

14871487
/* parallel search? */
14881488
if (Z_TYPE_P(link) == IS_ARRAY) {
1489-
int i, *rcs;
1490-
ldap_linkdata **lds;
1491-
zval *entry, object;
1492-
14931489
uint32_t num_links = zend_hash_num_elements(Z_ARRVAL_P(link));
14941490
if (num_links == 0) {
14951491
zend_argument_must_not_be_empty_error(1);
@@ -1515,7 +1511,6 @@ static void php_ldap_do_search(INTERNAL_FUNCTION_PARAMETERS, int scope)
15151511
ret = 0;
15161512
goto cleanup;
15171513
}
1518-
zend_hash_internal_pointer_reset(base_dn_ht);
15191514
} else {
15201515
if (zend_str_has_nul_byte(base_dn_str)) {
15211516
zend_argument_value_error(2, "must not contain null bytes");
@@ -1538,7 +1533,6 @@ static void php_ldap_do_search(INTERNAL_FUNCTION_PARAMETERS, int scope)
15381533
ret = 0;
15391534
goto cleanup;
15401535
}
1541-
zend_hash_internal_pointer_reset(filter_ht);
15421536
} else {
15431537
if (zend_str_has_nul_byte(filter_str)) {
15441538
zend_argument_value_error(3, "must not contain null bytes");
@@ -1548,73 +1542,90 @@ static void php_ldap_do_search(INTERNAL_FUNCTION_PARAMETERS, int scope)
15481542
ldap_filter = zend_string_copy(filter_str);
15491543
}
15501544

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

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

1564-
ld = Z_LDAP_LINK_P(entry);
1565-
if (!ld->link) {
1560+
ldap_linkdata *current_ld = Z_LDAP_LINK_P(link_zv);
1561+
if (!current_ld->link) {
15661562
zend_throw_error(NULL, "LDAP connection has already been closed");
15671563
ret = 0;
15681564
goto cleanup_parallel;
15691565
}
15701566

15711567
if (num_base_dns != 0) { /* base_dn an array? */
1572-
entry = zend_hash_get_current_data(base_dn_ht);
1573-
zend_hash_move_forward(base_dn_ht);
1574-
ldap_base_dn = zval_get_string(entry);
1575-
if (EG(exception)) {
1568+
zval *base_dn_zv = zend_hash_index_find(base_dn_ht, ldap_link_index);
1569+
ZEND_ASSERT(base_dn_zv);
1570+
ZVAL_DEREF(base_dn_zv);
1571+
if (Z_TYPE_P(base_dn_zv) != IS_STRING) {
1572+
zend_argument_type_error(2, "must be a list of strings, %s given", zend_zval_value_name(base_dn_zv));
1573+
ret = 0;
1574+
goto cleanup_parallel;
1575+
}
1576+
ldap_base_dn = zend_string_copy(Z_STR_P(base_dn_zv));
1577+
if (zend_str_has_nul_byte(ldap_base_dn)) {
1578+
zend_argument_value_error(2, "must not contain null bytes");
15761579
ret = 0;
15771580
goto cleanup_parallel;
15781581
}
1579-
// TODO check dn does not have any nul bytes
15801582
}
15811583
if (num_filters != 0) { /* filter an array? */
1582-
entry = zend_hash_get_current_data(filter_ht);
1583-
zend_hash_move_forward(filter_ht);
1584-
ldap_filter = zval_get_string(entry);
1585-
if (EG(exception)) {
1584+
zval *filter_zv = zend_hash_index_find(filter_ht, ldap_link_index);
1585+
ZEND_ASSERT(filter_zv);
1586+
ZVAL_DEREF(filter_zv);
1587+
if (Z_TYPE_P(filter_zv) != IS_STRING) {
1588+
zend_argument_type_error(3, "must be a list of strings, %s given", zend_zval_value_name(filter_zv));
1589+
ret = 0;
1590+
goto cleanup_parallel;
1591+
}
1592+
ldap_filter = zend_string_copy(Z_STR_P(filter_zv));
1593+
if (zend_str_has_nul_byte(ldap_filter)) {
1594+
zend_argument_value_error(3, "must not contain null bytes");
15861595
ret = 0;
15871596
goto cleanup_parallel;
15881597
}
1589-
// TODO check filter does not have any nul bytes
15901598
}
15911599

15921600
if (serverctrls) {
15931601
/* We have to parse controls again for each link as they use it */
15941602
_php_ldap_controls_free(&lserverctrls);
1595-
lserverctrls = _php_ldap_controls_from_array(ld->link, serverctrls, 9);
1603+
lserverctrls = _php_ldap_controls_from_array(current_ld->link, serverctrls, 9);
15961604
if (lserverctrls == NULL) {
1597-
rcs[i] = -1;
1605+
rcs[ldap_link_index] = -1;
1606+
// TODO Throw an exception/cleanup?
15981607
continue;
15991608
}
16001609
}
16011610

1602-
php_set_opts(ld->link, ldap_sizelimit, ldap_timelimit, ldap_deref, &old_ldap_sizelimit, &old_ldap_timelimit, &old_ldap_deref);
1611+
php_set_opts(current_ld->link, ldap_sizelimit, ldap_timelimit, ldap_deref, &old_ldap_sizelimit, &old_ldap_timelimit, &old_ldap_deref);
16031612

16041613
/* Run the actual search */
1605-
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]);
1606-
lds[i] = ld;
1607-
zend_hash_move_forward(Z_ARRVAL_P(link));
1608-
}
1614+
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]);
1615+
lds[ldap_link_index] = current_ld;
1616+
1617+
// TODO Reset the options of the link?
1618+
} ZEND_HASH_FOREACH_END();
16091619

16101620
array_init(return_value);
16111621

16121622
/* Collect results from the searches */
1613-
for (i=0; i<num_links; i++) {
1623+
for (uint32_t i = 0; i < num_links; i++) {
16141624
if (rcs[i] != -1) {
16151625
rcs[i] = ldap_result(lds[i]->link, LDAP_RES_ANY, 1 /* LDAP_MSG_ALL */, NULL, &ldap_res);
16161626
}
16171627
if (rcs[i] != -1) {
1628+
zval object;
16181629
object_init_ex(&object, ldap_result_ce);
16191630
result = Z_LDAP_RESULT_P(&object);
16201631
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

‎ext/ldap/tests/ldap_list_read_search_parallel_references.phpt‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
--TEST--
2+
Programming errors (Value/Type errors) for parallel usage of ldap_list(), ldap_read(), and ldap_search() with references
3+
--EXTENSIONS--
4+
ldap
5+
--FILE--
6+
<?php
7+
8+
/* ldap_list(), ldap_read(), and ldap_search() share an underlying C function */
9+
/* We are assuming 3333 is not connectable */
10+
$ldap = ldap_connect('ldap://127.0.0.1:3333');
11+
$valid_dn = "cn=userA,something";
12+
$valid_filter = "";
13+
14+
$ldaps = [&$ldap, $ldap];
15+
16+
$str = "string\0with_nul_byte";
17+
18+
$list_with_ref_nul_byte = [
19+
&$str,
20+
"string2",
21+
];
22+
23+
try {
24+
var_dump(ldap_list($ldaps, $list_with_ref_nul_byte, $valid_filter));
25+
} catch (Throwable $e) {
26+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
27+
}
28+
try {
29+
var_dump(ldap_list($ldaps, $valid_dn, $list_with_ref_nul_byte));
30+
} catch (Throwable $e) {
31+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
32+
}
33+
34+
?>
35+
--EXPECT--
36+
ValueError: ldap_list(): Argument #2 ($base) must not contain null bytes
37+
ValueError: ldap_list(): Argument #3 ($filter) must not contain null bytes

0 commit comments

Comments
 (0)