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

Reply via email to