Hi list, Having done some PHPT tests on ext/ldap [1] with Davide Mendolia at the Testfest, we realized that the extension wasn't ready in PHP 6. In attachment I put a patch that enables all LDAP tests [1] to "PASS" using PHP6.
As it is my first patch here, could you please revise it and give me your comments? The following changes have been made: - Comparing to IS_UNICODE in addition to IS_STRING. - Using zend_utf8_hash_find() instead of zend_hash_find. - Being sick of duplicating "efree(ldap_base_dn)" everywhere there is an exit, the handling of error is refactored using a "goto cleanup" in combination with storing the return value in "ret". - Parallel/Non-parallel search lightly refactored for clarity. - Dropping WRONG_PARAM_COUNT while zend_parse_parameters() does a good job (not present anymore in 5.3). Thanks, Patrick [1] http://testfest.php.net/repos/testfest/BelgiumUG/ext/ldap/tests/
Index: ext/ldap/ldap.c =================================================================== RCS file: /repository/php-src/ext/ldap/ldap.c,v retrieving revision 1.198 diff -u -r1.198 ldap.c --- ext/ldap/ldap.c 21 Apr 2009 18:07:42 -0000 1.198 +++ ext/ldap/ldap.c 28 May 2009 15:43:05 -0000 @@ -603,6 +603,8 @@ int num_attribs = 0; int i, errno; int myargcount = ZEND_NUM_ARGS(); + int free_base_dn = 0; + int ret = 1; if (zend_parse_parameters(myargcount TSRMLS_CC, "ZZZ|ZZZZZ", &link, &base_dn, &filter, &attrs, &attrsonly, &sizelimit, &timelimit, &deref) == FAILURE) { @@ -630,7 +632,8 @@ case 4 : if (Z_TYPE_PP(attrs) != IS_ARRAY) { php_error_docref(NULL TSRMLS_CC, E_WARNING, "Expected Array as last element"); - RETURN_FALSE; + ret = 0; + goto cleanup; } num_attribs = zend_hash_num_elements(Z_ARRVAL_PP(attrs)); @@ -639,8 +642,8 @@ for (i = 0; i<num_attribs; i++) { if (zend_hash_index_find(Z_ARRVAL_PP(attrs), i, (void **) &attr) != SUCCESS) { php_error_docref(NULL TSRMLS_CC, E_WARNING, "Array initialization wrong"); - efree(ldap_attrs); - RETURN_FALSE; + ret = 0; + goto cleanup; } SEPARATE_ZVAL(attr); @@ -648,18 +651,8 @@ ldap_attrs[i] = Z_STRVAL_PP(attr); } ldap_attrs[num_attribs] = NULL; - case 3 : - /* parallel search? */ - if (Z_TYPE_PP(link) != IS_ARRAY) { - convert_to_string_ex(filter); - ldap_filter = Z_STRVAL_PP(filter); - - /* If anything else than string is passed, ldap_base_dn = NULL */ - if (Z_TYPE_PP(base_dn) == IS_STRING) { - ldap_base_dn = Z_STRVAL_PP(base_dn); - } - } + break; default: @@ -676,20 +669,16 @@ nlinks = zend_hash_num_elements(Z_ARRVAL_PP(link)); if (nlinks == 0) { php_error_docref(NULL TSRMLS_CC, E_WARNING, "No links in link array"); - if (ldap_attrs != NULL) { - efree(ldap_attrs); - } - RETURN_FALSE; + ret = 0; + goto cleanup; } if (Z_TYPE_PP(base_dn) == IS_ARRAY) { nbases = zend_hash_num_elements(Z_ARRVAL_PP(base_dn)); if (nbases != nlinks) { php_error_docref(NULL TSRMLS_CC, E_WARNING, "Base must either be a string, or an array with the same number of elements as the links array"); - if (ldap_attrs != NULL) { - efree(ldap_attrs); - } - RETURN_FALSE; + ret = 0; + goto cleanup; } zend_hash_internal_pointer_reset(Z_ARRVAL_PP(base_dn)); } else { @@ -697,6 +686,10 @@ /* If anything else than string is passed, ldap_base_dn = NULL */ if (Z_TYPE_PP(base_dn) == IS_STRING) { ldap_base_dn = Z_STRVAL_PP(base_dn); + } else if(Z_TYPE_PP(base_dn) == IS_UNICODE){ + char *tmp = zend_unicode_to_ascii(Z_USTRVAL_PP(base_dn), Z_USTRLEN_PP(base_dn) TSRMLS_CC); + ldap_base_dn = estrdup(tmp); + free_base_dn = 1; } else { ldap_base_dn = NULL; } @@ -706,10 +699,8 @@ nfilters = zend_hash_num_elements(Z_ARRVAL_PP(filter)); if (nfilters != nlinks) { php_error_docref(NULL TSRMLS_CC, E_WARNING, "Filter must either be a string, or an array with the same number of elements as the links array"); - if (ldap_attrs != NULL) { - efree(ldap_attrs); - } - RETURN_FALSE; + ret = 0; + goto cleanup; } zend_hash_internal_pointer_reset(Z_ARRVAL_PP(filter)); } else { @@ -727,12 +718,8 @@ ld = (ldap_linkdata *) zend_fetch_resource(entry TSRMLS_CC, -1, "ldap link", NULL, 1, le_link); if (ld == NULL) { - efree(lds); - efree(rcs); - if (ldap_attrs != NULL) { - efree(ldap_attrs); - } - RETURN_FALSE; + ret = 0; + goto cleanup_parallel; } if (nbases != 0) { /* base_dn an array? */ zend_hash_get_current_data(Z_ARRVAL_PP(base_dn), (void **)&entry); @@ -741,6 +728,10 @@ /* If anything else than string is passed, ldap_base_dn = NULL */ if (Z_TYPE_PP(entry) == IS_STRING) { ldap_base_dn = Z_STRVAL_PP(entry); + } else if(Z_TYPE_PP(entry) == IS_UNICODE){ + char *tmp = zend_unicode_to_ascii(Z_USTRVAL_PP(entry), Z_USTRLEN_PP(entry) TSRMLS_CC); + ldap_base_dn = estrdup(tmp); + free_base_dn = 1; } else { ldap_base_dn = NULL; } @@ -760,10 +751,6 @@ zend_hash_move_forward(Z_ARRVAL_PP(link)); } - if (ldap_attrs != NULL) { - efree(ldap_attrs); - } - array_init(return_value); /* Collect results from the searches */ @@ -779,50 +766,68 @@ add_next_index_bool(return_value, 0); } } + +cleanup_parallel: efree(lds); efree(rcs); - return; - } + } else { + convert_to_string_ex(filter); + ldap_filter = Z_STRVAL_PP(filter); - ld = (ldap_linkdata *) zend_fetch_resource(link TSRMLS_CC, -1, "ldap link", NULL, 1, le_link); - if (ld == NULL) { - if (ldap_attrs != NULL) { - efree(ldap_attrs); + /* If anything else than string is passed, ldap_base_dn = NULL */ + if (Z_TYPE_PP(base_dn) == IS_STRING) { + ldap_base_dn = Z_STRVAL_PP(base_dn); + } else if (Z_TYPE_PP(base_dn) == IS_UNICODE) { + char *tmp = zend_unicode_to_ascii(Z_USTRVAL_PP(base_dn), Z_USTRLEN_PP(base_dn) TSRMLS_CC); + ldap_base_dn = estrdup(tmp); + free_base_dn = 1; } - RETURN_FALSE; - } - - php_set_opts(ld->link, ldap_sizelimit, ldap_timelimit, ldap_deref); - - /* Run the actual search */ - errno = ldap_search_s(ld->link, ldap_base_dn, scope, ldap_filter, ldap_attrs, ldap_attrsonly, &ldap_res); - - if (ldap_attrs != NULL) { - efree(ldap_attrs); - } - if (errno != LDAP_SUCCESS - && errno != LDAP_SIZELIMIT_EXCEEDED + ld = (ldap_linkdata *) zend_fetch_resource(link TSRMLS_CC, -1, "ldap link", NULL, 1, le_link); + if (ld == NULL) { + ret = 0; + goto cleanup; + } + + php_set_opts(ld->link, ldap_sizelimit, ldap_timelimit, ldap_deref); + + /* Run the actual search */ + errno = ldap_search_s(ld->link, ldap_base_dn, scope, ldap_filter, ldap_attrs, ldap_attrsonly, &ldap_res); + + if (errno != LDAP_SUCCESS + && errno != LDAP_SIZELIMIT_EXCEEDED #ifdef LDAP_ADMINLIMIT_EXCEEDED - && errno != LDAP_ADMINLIMIT_EXCEEDED + && errno != LDAP_ADMINLIMIT_EXCEEDED #endif #ifdef LDAP_REFERRAL - && errno != LDAP_REFERRAL + && errno != LDAP_REFERRAL #endif - ) { - php_error_docref(NULL TSRMLS_CC, E_WARNING, "Search: %s", ldap_err2string(errno)); - RETVAL_FALSE; - } else { - if (errno == LDAP_SIZELIMIT_EXCEEDED) { - php_error_docref(NULL TSRMLS_CC, E_WARNING, "Partial search results returned: Sizelimit exceeded"); - } + ) { + php_error_docref(NULL TSRMLS_CC, E_WARNING, "Search: %s", ldap_err2string(errno)); + ret = 0; + } else { + if (errno == LDAP_SIZELIMIT_EXCEEDED) { + php_error_docref(NULL TSRMLS_CC, E_WARNING, "Partial search results returned: Sizelimit exceeded"); + } #ifdef LDAP_ADMINLIMIT_EXCEEDED - else if (errno == LDAP_ADMINLIMIT_EXCEEDED) { - php_error_docref(NULL TSRMLS_CC, E_WARNING, "Partial search results returned: Adminlimit exceeded"); - } + else if (errno == LDAP_ADMINLIMIT_EXCEEDED) { + php_error_docref(NULL TSRMLS_CC, E_WARNING, "Partial search results returned: Adminlimit exceeded"); + } #endif - - ZEND_REGISTER_RESOURCE(return_value, ldap_res, le_result); + + ZEND_REGISTER_RESOURCE(return_value, ldap_res, le_result); + } + } + +cleanup: + if (ldap_attrs != NULL) { + efree(ldap_attrs); + } + if (free_base_dn) { + efree(ldap_base_dn); + } + if (!ret) { + RETVAL_BOOL(ret); } } /* }}} */ @@ -1050,8 +1055,8 @@ ldap_resultentry *resultentry; char *attribute; - if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "rr", &link, &result_entry) == FAILURE) { - WRONG_PARAM_COUNT; + if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "rr", &link, &result_entry) != SUCCESS) { + return; } ZEND_FETCH_RESOURCE(ld, ldap_linkdata *, &link, -1, "ldap link", le_link); @@ -1077,8 +1082,8 @@ ldap_resultentry *resultentry; char *attribute; - if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "rr", &link, &result_entry) == FAILURE) { - WRONG_PARAM_COUNT; + if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "rr", &link, &result_entry) != SUCCESS) { + return; } ZEND_FETCH_RESOURCE(ld, ldap_linkdata *, &link, -1, "ldap link", le_link); @@ -1837,7 +1842,7 @@ error = 1; break; } - if (zend_hash_find(Z_ARRVAL_PP(ctrlval), "oid", sizeof("oid"), (void **) &val) != SUCCESS) { + if (zend_utf8_hash_find(Z_ARRVAL_PP(ctrlval), "oid", sizeof("oid"), (void **) &val) != SUCCESS) { php_error_docref(NULL TSRMLS_CC, E_WARNING, "Control must have an oid key"); error = 1; break; @@ -1845,7 +1850,7 @@ ctrl = *ctrlp = emalloc(sizeof(**ctrlp)); convert_to_string_ex(val); ctrl->ldctl_oid = Z_STRVAL_PP(val); - if (zend_hash_find(Z_ARRVAL_PP(ctrlval), "value", sizeof("value"), (void **) &val) == SUCCESS) { + if (zend_utf8_hash_find(Z_ARRVAL_PP(ctrlval), "value", sizeof("value"), (void **) &val) == SUCCESS) { convert_to_string_ex(val); ctrl->ldctl_value.bv_val = Z_STRVAL_PP(val); ctrl->ldctl_value.bv_len = Z_STRLEN_PP(val); @@ -1853,7 +1858,7 @@ ctrl->ldctl_value.bv_val = NULL; ctrl->ldctl_value.bv_len = 0; } - if (zend_hash_find(Z_ARRVAL_PP(ctrlval), "iscritical", sizeof("iscritical"), (void **) &val) == SUCCESS) { + if (zend_utf8_hash_find(Z_ARRVAL_PP(ctrlval), "iscritical", sizeof("iscritical"), (void **) &val) == SUCCESS) { convert_to_boolean_ex(val); ctrl->ldctl_iscritical = Z_BVAL_PP(val); } else { @@ -2165,7 +2170,8 @@ ZEND_FETCH_RESOURCE(ld, ldap_linkdata *, &link, -1, "ldap link", le_link); - if (Z_TYPE_P(callback) == IS_STRING && Z_STRLEN_P(callback) == 0) { + if ((Z_TYPE_P(callback) == IS_STRING && Z_STRLEN_P(callback) == 0) || + (Z_TYPE_P(callback) == IS_UNICODE && Z_USTRLEN_P(callback) == 0)) { /* unregister rebind procedure */ if (ld->rebindproc != NULL) { zval_dtor(ld->rebindproc);
-- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php