diff --git a/UPGRADING b/UPGRADING index c9c9468a3bdc1..9a2d999e92b99 100644 --- a/UPGRADING +++ b/UPGRADING @@ -19,6 +19,10 @@ PHP 8.5 UPGRADE NOTES 1. Backward Incompatible Changes ======================================== +- LDAP: + . ldap_get_option() and ldap_set_option() now throw a ValueError when + passing an invalid option. + - SPL: . ArrayObject no longer accepts enums, as modifying the $name or $value properties can break engine assumptions. diff --git a/ext/ldap/ldap.c b/ext/ldap/ldap.c index d505c8846bd1a..3fea267d14ff9 100644 --- a/ext/ldap/ldap.c +++ b/ext/ldap/ldap.c @@ -2101,7 +2101,6 @@ PHP_FUNCTION(ldap_explode_dn) { zend_long with_attrib; char *dn, **ldap_value; - int i, count; size_t dn_len; if (zend_parse_parameters(ZEND_NUM_ARGS(), "pl", &dn, &dn_len, &with_attrib) != SUCCESS) { @@ -2113,16 +2112,12 @@ PHP_FUNCTION(ldap_explode_dn) RETURN_FALSE; } - i=0; - while (ldap_value[i] != NULL) i++; - count = i; - array_init(return_value); - - add_assoc_long(return_value, "count", count); - for (i = 0; ilink, ldap_result->result, &lerrcode, - myargcount > 3 ? &lmatcheddn : NULL, - myargcount > 4 ? &lerrmsg : NULL, - myargcount > 5 ? &lreferrals : NULL, - myargcount > 6 ? &lserverctrls : NULL, + matcheddn ? &lmatcheddn : NULL, + errmsg ? &lerrmsg : NULL, + referrals ? &lreferrals : NULL, + serverctrls ? &lserverctrls : NULL, 0); if (rc != LDAP_SUCCESS) { php_error_docref(NULL, E_WARNING, "Unable to parse result: %s", ldap_err2string(rc)); @@ -3233,41 +3245,40 @@ PHP_FUNCTION(ldap_parse_result) ZEND_TRY_ASSIGN_REF_LONG(errcode, lerrcode); - /* Reverse -> fall through */ - switch (myargcount) { - case 7: - _php_ldap_controls_to_array(ld->link, lserverctrls, serverctrls, 0); - ZEND_FALLTHROUGH; - case 6: - referrals = zend_try_array_init(referrals); - if (!referrals) { - RETURN_THROWS(); - } - if (lreferrals != NULL) { - refp = lreferrals; - while (*refp) { - add_next_index_string(referrals, *refp); - refp++; - } - ldap_memvfree((void**)lreferrals); - } - ZEND_FALLTHROUGH; - case 5: - if (lerrmsg == NULL) { - ZEND_TRY_ASSIGN_REF_EMPTY_STRING(errmsg); - } else { - ZEND_TRY_ASSIGN_REF_STRING(errmsg, lerrmsg); - ldap_memfree(lerrmsg); - } - ZEND_FALLTHROUGH; - case 4: - if (lmatcheddn == NULL) { - ZEND_TRY_ASSIGN_REF_EMPTY_STRING(matcheddn); - } else { - ZEND_TRY_ASSIGN_REF_STRING(matcheddn, lmatcheddn); - ldap_memfree(lmatcheddn); + if (serverctrls) { + _php_ldap_controls_to_array(ld->link, lserverctrls, serverctrls, 0); + } + if (referrals) { + referrals = zend_try_array_init(referrals); + if (!referrals) { + RETURN_THROWS(); + } + if (lreferrals != NULL) { + refp = lreferrals; + while (*refp) { + add_next_index_string(referrals, *refp); + refp++; } + ldap_memvfree((void**)lreferrals); + } + } + if (errmsg) { + if (lerrmsg == NULL) { + ZEND_TRY_ASSIGN_REF_EMPTY_STRING(errmsg); + } else { + ZEND_TRY_ASSIGN_REF_STRING(errmsg, lerrmsg); + ldap_memfree(lerrmsg); + } } + if (matcheddn) { + if (lmatcheddn == NULL) { + ZEND_TRY_ASSIGN_REF_EMPTY_STRING(matcheddn); + } else { + ZEND_TRY_ASSIGN_REF_STRING(matcheddn, lmatcheddn); + ldap_memfree(lmatcheddn); + } + } + RETURN_TRUE; } /* }}} */ @@ -3278,14 +3289,14 @@ PHP_FUNCTION(ldap_parse_result) /* {{{ Extract information from extended operation result */ PHP_FUNCTION(ldap_parse_exop) { - zval *link, *result, *retdata, *retoid; + zval *link, *result, *retdata = NULL, *retoid = NULL; ldap_linkdata *ld; ldap_resultdata *ldap_result; char *lretoid; struct berval *lretdata; - int rc, myargcount = ZEND_NUM_ARGS(); + int rc; - if (zend_parse_parameters(myargcount, "OO|zz", &link, ldap_link_ce, &result, ldap_result_ce, &retdata, &retoid) != SUCCESS) { + if (zend_parse_parameters(ZEND_NUM_ARGS(), "OO|zz", &link, ldap_link_ce, &result, ldap_result_ce, &retdata, &retoid) != SUCCESS) { RETURN_THROWS(); } @@ -3296,34 +3307,34 @@ PHP_FUNCTION(ldap_parse_exop) VERIFY_LDAP_RESULT_OPEN(ldap_result); rc = ldap_parse_extended_result(ld->link, ldap_result->result, - myargcount > 3 ? &lretoid: NULL, - myargcount > 2 ? &lretdata: NULL, + retoid ? &lretoid: NULL, + retdata ? &lretdata: NULL, 0); if (rc != LDAP_SUCCESS) { php_error_docref(NULL, E_WARNING, "Unable to parse extended operation result: %s", ldap_err2string(rc)); RETURN_FALSE; } - /* Reverse -> fall through */ - switch (myargcount) { - case 4: - if (lretoid == NULL) { - ZEND_TRY_ASSIGN_REF_EMPTY_STRING(retoid); - } else { - ZEND_TRY_ASSIGN_REF_STRING(retoid, lretoid); - ldap_memfree(lretoid); - } - ZEND_FALLTHROUGH; - case 3: - /* use arg #3 as the data returned by the server */ - if (lretdata == NULL) { - ZEND_TRY_ASSIGN_REF_EMPTY_STRING(retdata); - } else { - ZEND_TRY_ASSIGN_REF_STRINGL(retdata, lretdata->bv_val, lretdata->bv_len); - ldap_memfree(lretdata->bv_val); - ldap_memfree(lretdata); - } + if (retoid) { + if (lretoid == NULL) { + ZEND_TRY_ASSIGN_REF_EMPTY_STRING(retoid); + } else { + ZEND_TRY_ASSIGN_REF_STRING(retoid, lretoid); + ldap_memfree(lretoid); + } + } + + if (retdata) { + /* use arg #3 as the data returned by the server */ + if (lretdata == NULL) { + ZEND_TRY_ASSIGN_REF_EMPTY_STRING(retdata); + } else { + ZEND_TRY_ASSIGN_REF_STRINGL(retdata, lretdata->bv_val, lretdata->bv_len); + ldap_memfree(lretdata->bv_val); + ldap_memfree(lretdata); + } } + RETURN_TRUE; } /* }}} */ @@ -3907,7 +3918,7 @@ PHP_FUNCTION(ldap_exop_sync) /* {{{ Passwd modify extended operation */ PHP_FUNCTION(ldap_exop_passwd) { - zval *link, *serverctrls; + zval *link, *serverctrls = NULL; struct berval luser = { 0L, NULL }; struct berval loldpw = { 0L, NULL }; struct berval lnewpw = { 0L, NULL }; @@ -3915,22 +3926,22 @@ PHP_FUNCTION(ldap_exop_passwd) LDAPControl *ctrl, **lserverctrls = NULL, *requestctrls[2] = { NULL, NULL }; LDAPMessage* ldap_res = NULL; ldap_linkdata *ld; - int rc, myargcount = ZEND_NUM_ARGS(), msgid, err; + int rc, msgid, err; char* errmsg = NULL; - if (zend_parse_parameters(myargcount, "O|sssz/", &link, ldap_link_ce, &luser.bv_val, &luser.bv_len, &loldpw.bv_val, &loldpw.bv_len, &lnewpw.bv_val, &lnewpw.bv_len, &serverctrls) == FAILURE) { + if (zend_parse_parameters(ZEND_NUM_ARGS(), "O|sssz/", &link, ldap_link_ce, &luser.bv_val, &luser.bv_len, &loldpw.bv_val, &loldpw.bv_len, &lnewpw.bv_val, &lnewpw.bv_len, &serverctrls) == FAILURE) { RETURN_THROWS(); } ld = Z_LDAP_LINK_P(link); VERIFY_LDAP_LINK_CONNECTED(ld); - switch (myargcount) { - case 5: - /* ldap_create_passwordpolicy_control() allocates ctrl */ - if (ldap_create_passwordpolicy_control(ld->link, &ctrl) == LDAP_SUCCESS) { - requestctrls[0] = ctrl; - } + if (serverctrls) { + /* ldap_create_passwordpolicy_control() allocates ctrl */ + if (ldap_create_passwordpolicy_control(ld->link, &ctrl) == LDAP_SUCCESS) { + requestctrls[0] = ctrl; + } + // TODO Should this warn? } /* asynchronous call to get result and controls */ @@ -3965,14 +3976,14 @@ PHP_FUNCTION(ldap_exop_passwd) goto cleanup; } - rc = ldap_parse_result(ld->link, ldap_res, &err, NULL, &errmsg, NULL, (myargcount > 4 ? &lserverctrls : NULL), 0); + rc = ldap_parse_result(ld->link, ldap_res, &err, NULL, &errmsg, NULL, (serverctrls ? &lserverctrls : NULL), 0); if( rc != LDAP_SUCCESS ) { php_error_docref(NULL, E_WARNING, "Passwd modify extended operation failed: %s (%d)", ldap_err2string(rc), rc); RETVAL_FALSE; goto cleanup; } - if (myargcount > 4) { + if (serverctrls) { _php_ldap_controls_to_array(ld->link, lserverctrls, serverctrls, 0); } diff --git a/ext/ldap/tests/ldap_explode_dn.phpt b/ext/ldap/tests/ldap_explode_dn.phpt index 2012506c3ef07..047078c7beb79 100644 --- a/ext/ldap/tests/ldap_explode_dn.phpt +++ b/ext/ldap/tests/ldap_explode_dn.phpt @@ -34,18 +34,16 @@ echo "Done\n"; ?> --EXPECT-- array(4) { - ["count"]=> - int(3) [0]=> string(6) "cn=bob" [1]=> string(10) "dc=example" [2]=> string(6) "dc=com" + ["count"]=> + int(3) } array(5) { - ["count"]=> - int(4) [0]=> string(6) "cn=bob" [1]=> @@ -54,20 +52,20 @@ array(5) { string(10) "dc=example" [3]=> string(6) "dc=com" + ["count"]=> + int(4) } array(4) { - ["count"]=> - int(3) [0]=> string(3) "bob" [1]=> string(7) "example" [2]=> string(3) "com" + ["count"]=> + int(3) } array(5) { - ["count"]=> - int(4) [0]=> string(3) "bob" [1]=> @@ -76,6 +74,8 @@ array(5) { string(7) "example" [3]=> string(3) "com" + ["count"]=> + int(4) } bool(false) bool(false) diff --git a/ext/ldap/tests/ldap_get_option_package_basic.phpt b/ext/ldap/tests/ldap_get_option_package_basic.phpt index 6424a1c5d104f..9135ea233701d 100644 --- a/ext/ldap/tests/ldap_get_option_package_basic.phpt +++ b/ext/ldap/tests/ldap_get_option_package_basic.phpt @@ -11,9 +11,7 @@ $link = ldap_connect($uri); $result = ldap_get_option($link, LDAP_OPT_X_TLS_PACKAGE, $optionval); var_dump(in_array($optionval, ['GnuTLS', 'OpenSSL', 'MozNSS'])); -// This is a read-only option. -var_dump(ldap_set_option($link, LDAP_OPT_X_TLS_PACKAGE, 'foo')); + ?> --EXPECT-- bool(true) -bool(false) diff --git a/ext/ldap/tests/ldap_get_set_invalid_option_error.phpt b/ext/ldap/tests/ldap_get_set_invalid_option_error.phpt new file mode 100644 index 0000000000000..2079353e48c97 --- /dev/null +++ b/ext/ldap/tests/ldap_get_set_invalid_option_error.phpt @@ -0,0 +1,22 @@ +--TEST-- +ldap_(g|s)et_option() with non existing option +--EXTENSIONS-- +ldap +--FILE-- +getMessage(), PHP_EOL; +} +try { + var_dump(ldap_get_option($ldap, 999999, $value)); +} catch (Throwable $e) { + echo $e::class, ": ", $e->getMessage(), PHP_EOL; +} +?> +--EXPECT-- +ValueError: ldap_set_option(): Argument #2 ($option) must be a valid LDAP option +ValueError: ldap_get_option(): Argument #2 ($option) must be a valid LDAP option diff --git a/ext/ldap/tests/ldap_set_option_error.phpt b/ext/ldap/tests/ldap_set_option_error.phpt index fa66e348c46ba..eda3dc5121733 100644 --- a/ext/ldap/tests/ldap_set_option_error.phpt +++ b/ext/ldap/tests/ldap_set_option_error.phpt @@ -33,11 +33,9 @@ foreach ($controls as $control) { } } -var_dump(ldap_set_option($link, 999999, 999999)); ?> --EXPECT-- bool(false) ValueError: ldap_set_option(): Control must have an "oid" key TypeError: ldap_set_option(): Argument #3 ($value) must contain only arrays, where each array is a control TypeError: ldap_set_option(): Argument #3 ($value) must be of type array for the LDAP_OPT_CLIENT_CONTROLS option, string given -bool(false)