Removed two step syntax for EXOP helpers, one call workflow is enough

php · Jul 3, 2017 · e9001ca · e9001ca
1 parent 5020325
commit e9001ca
Show file tree

Hide file tree

Showing 6 changed files with 33 additions and 188 deletions.
diff --git a/ext/ldap/config.m4 b/ext/ldap/config.m4
@@ -204,7 +204,7 @@ if test "$PHP_LDAP" != "no"; then
 
   dnl Solaris 2.8 claims to be 2004 API, but doesn't have
   dnl ldap_parse_reference() nor ldap_start_tls_s()
-  AC_CHECK_FUNCS([ldap_parse_result ldap_parse_reference ldap_start_tls_s ldap_control_find ldap_parse_extended_result ldap_parse_passwd ldap_parse_whoami ldap_extended_operation ldap_extended_operation_s ldap_passwd_s ldap_whoami_s])
+  AC_CHECK_FUNCS([ldap_parse_result ldap_parse_reference ldap_start_tls_s ldap_control_find ldap_parse_extended_result ldap_extended_operation ldap_extended_operation_s ldap_passwd_s ldap_whoami_s])
 
   dnl
   dnl SASL check

diff --git a/ext/ldap/config.w32 b/ext/ldap/config.w32
@@ -22,8 +22,6 @@ if (PHP_LDAP != "no") {
 		AC_DEFINE('LDAP_DEPRECATED', 1);
 		AC_DEFINE('HAVE_LDAP_CONTROL_FIND', 1);
 		AC_DEFINE('HAVE_LDAP_PARSE_EXTENDED_RESULT', 1);
-		AC_DEFINE('HAVE_LDAP_PARSE_PASSWD', 1);
-		AC_DEFINE('HAVE_LDAP_PARSE_WHOAMI', 1);
 		AC_DEFINE('HAVE_LDAP_EXTENDED_OPERATION_S', 1);
 		AC_DEFINE('HAVE_LDAP_PASSWD_S', 1);
 		AC_DEFINE('HAVE_LDAP_WHOAMI_S', 1);

diff --git a/ext/ldap/ldap.c b/ext/ldap/ldap.c
@@ -2631,90 +2631,6 @@ PHP_FUNCTION(ldap_parse_exop)
 }
 /* }}} */
 #endif
-
-#ifdef HAVE_LDAP_PARSE_PASSWD
-/* {{{ proto bool ldap_parse_exop_passwd(resource link, resource result, string newpasswd)
-   Extract information from RFC 3062 password modify extended operation result */
-PHP_FUNCTION(ldap_parse_exop_passwd)
-{
-	zval *link, *result, *newpasswd;
-	ldap_linkdata *ld;
-	LDAPMessage *ldap_result;
-	struct berval lnewpasswd;
-	int rc, myargcount = ZEND_NUM_ARGS();
-
-	if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "rrz/", &link, &result, &newpasswd) != SUCCESS) {
-		WRONG_PARAM_COUNT;
-	}
-
-	if ((ld = (ldap_linkdata *)zend_fetch_resource(Z_RES_P(link), "ldap link", le_link)) == NULL) {
-		RETURN_FALSE;
-	}
-
-	if ((ldap_result = (LDAPMessage *)zend_fetch_resource(Z_RES_P(result), "ldap result", le_result)) == NULL) {
-		RETURN_FALSE;
-	}
-
-	rc = ldap_parse_passwd(ld->link, ldap_result, &lnewpasswd);
-	if (rc != LDAP_SUCCESS) {
-		php_error_docref(NULL TSRMLS_CC, E_WARNING, "Unable to parse passwd modify extended operation result: %s", ldap_err2string(rc));
-		RETURN_FALSE;
-	}
-
-	zval_dtor(newpasswd);
-	if (lnewpasswd.bv_len == 0) {
-		ZVAL_EMPTY_STRING(newpasswd);
-	} else {
-		ZVAL_STRINGL(newpasswd, lnewpasswd.bv_val, lnewpasswd.bv_len);
-		ldap_memfree(lnewpasswd.bv_val);
-	}
-
-	RETURN_TRUE;
-}
-/* }}} */
-#endif
-
-#ifdef HAVE_LDAP_PARSE_WHOAMI
-/* {{{ proto bool ldap_parse_exop_whoami(resource link, resource result, string authzid)
-   Extract information from <draft-zeilenga-ldap-authzid> whoami extended operation result (a Work in Progress) */
-PHP_FUNCTION(ldap_parse_exop_whoami)
-{
-	zval *link, *result, *authzid;
-	ldap_linkdata *ld;
-	LDAPMessage *ldap_result;
-	struct berval *lauthzid;
-	int rc, myargcount = ZEND_NUM_ARGS();
-
-	if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "rrz/", &link, &result, &authzid) != SUCCESS) {
-		WRONG_PARAM_COUNT;
-	}
-
-	if ((ld = (ldap_linkdata *)zend_fetch_resource(Z_RES_P(link), "ldap link", le_link)) == NULL) {
-		RETURN_FALSE;
-	}
-
-	if ((ldap_result = (LDAPMessage *)zend_fetch_resource(Z_RES_P(result), "ldap result", le_result)) == NULL) {
-		RETURN_FALSE;
-	}
-
-	rc = ldap_parse_whoami(ld->link, ldap_result, &lauthzid );
-	if (rc != LDAP_SUCCESS) {
-		php_error_docref(NULL TSRMLS_CC, E_WARNING, "Unable to parse whoami extended operation result: %s", ldap_err2string(rc));
-		RETURN_FALSE;
-	}
-
-	zval_dtor(authzid);
-	if (lauthzid == NULL) {
-		ZVAL_EMPTY_STRING(authzid);
-	} else {
-		ZVAL_STRINGL(authzid, lauthzid->bv_val, lauthzid->bv_len);
-		ldap_memfree(lauthzid->bv_val);
-		ldap_memfree(lauthzid);
-	}
-	RETURN_TRUE;
-}
-/* }}} */
-#endif
 /* }}} */
 
 /* {{{ proto resource ldap_first_reference(resource link, resource result)
@@ -3399,7 +3315,7 @@ PHP_FUNCTION(ldap_exop)
 #endif
 
 #ifdef HAVE_LDAP_PASSWD_S
-/* {{{ proto ? ldap_exop_passwd(resource link [, string user [, string oldpw [, string newpw [, string newpasswd ]]]])
+/* {{{ proto bool ldap_exop_passwd(resource link [, string user [, string oldpw [, string newpw [, string genpasswd ]]]])
    Passwd modify extended operation */
 PHP_FUNCTION(ldap_exop_passwd)
 {
@@ -3440,55 +3356,34 @@ PHP_FUNCTION(ldap_exop_passwd)
 			luser.bv_len = Z_STRLEN_P(user);
 	}
 
-	if (myargcount > 4 || lnewpw.bv_len > 0) {
-		/* synchronous call */
-		rc = ldap_passwd_s(ld->link, &luser,
-			loldpw.bv_len > 0 ? &loldpw : NULL,
-			lnewpw.bv_len > 0 ? &lnewpw : NULL,
-			&lnewpasswd, NULL, NULL);
-		if (rc != LDAP_SUCCESS ) {
-			php_error_docref(NULL TSRMLS_CC, E_WARNING, "Passwd modify extended operation failed: %s (%d)", ldap_err2string(rc), rc);
-			RETURN_FALSE;
-		}
-
-		if (myargcount > 4) {
-			zval_dtor(newpasswd);
-			if (lnewpasswd.bv_len == 0) {
-				ZVAL_EMPTY_STRING(newpasswd);
-			} else {
-				ZVAL_STRINGL(newpasswd, lnewpasswd.bv_val, lnewpasswd.bv_len);
-			}
-		}
-
-		ldap_memfree(lnewpasswd.bv_val);
-
-		RETURN_TRUE;
-	}
-
-	/* asynchronous call */
-	rc = ldap_passwd(ld->link, &luser,
+	/* synchronous call */
+	rc = ldap_passwd_s(ld->link, &luser,
 		loldpw.bv_len > 0 ? &loldpw : NULL,
 		lnewpw.bv_len > 0 ? &lnewpw : NULL,
-		NULL, NULL, &msgid);
+		&lnewpasswd, NULL, NULL);
 	if (rc != LDAP_SUCCESS ) {
 		php_error_docref(NULL TSRMLS_CC, E_WARNING, "Passwd modify extended operation failed: %s (%d)", ldap_err2string(rc), rc);
 		RETURN_FALSE;
 	}
 
-	rc = ldap_result(ld->link, msgid, 1 /* LDAP_MSG_ALL */, NULL, &ldap_res);
-	if (rc == -1) {
-		php_error_docref(NULL TSRMLS_CC, E_WARNING, "Passwd modify extended operation failed");
-		RETURN_FALSE;
+	if (myargcount > 4) {
+		zval_dtor(newpasswd);
+		if (lnewpasswd.bv_len == 0) {
+			ZVAL_EMPTY_STRING(newpasswd);
+		} else {
+			ZVAL_STRINGL(newpasswd, lnewpasswd.bv_val, lnewpasswd.bv_len);
+		}
 	}
 
-	/* return a PHP control object */
-	RETVAL_RES(zend_register_resource(ldap_res, le_result));
+	ldap_memfree(lnewpasswd.bv_val);
+
+	RETURN_TRUE;
 }
 /* }}} */
 #endif
 
 #ifdef HAVE_LDAP_WHOAMI_S
-/* {{{ proto ? ldap_exop_whoami(resource link [, string authzid])
+/* {{{ proto bool ldap_exop_whoami(resource link , string authzid)
    Whoami extended operation */
 PHP_FUNCTION(ldap_exop_whoami)
 {
@@ -3499,49 +3394,31 @@ PHP_FUNCTION(ldap_exop_whoami)
 	LDAPMessage *ldap_res;
 	int rc, msgid, myargcount = ZEND_NUM_ARGS();
 
-	if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "r|z/", &link, &authzid) == FAILURE) {
+	if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "rz/", &link, &authzid) == FAILURE) {
 		WRONG_PARAM_COUNT;
 	}
 
 	if ((ld = (ldap_linkdata *)zend_fetch_resource(Z_RES_P(link), "ldap link", le_link)) == NULL) {
 		RETURN_FALSE;
 	}
 
-	if (myargcount == 2) {
-		/* synchronous call */
-		rc = ldap_whoami_s(ld->link, &lauthzid, NULL, NULL);
-		if (rc != LDAP_SUCCESS ) {
-			php_error_docref(NULL TSRMLS_CC, E_WARNING, "Whoami extended operation failed: %s (%d)", ldap_err2string(rc), rc);
-			RETURN_FALSE;
-		}
-
-		zval_dtor(authzid);
-		if (lauthzid == NULL) {
-			ZVAL_EMPTY_STRING(authzid);
-		} else {
-			ZVAL_STRINGL(authzid, lauthzid->bv_val, lauthzid->bv_len);
-			ldap_memfree(lauthzid->bv_val);
-			ldap_memfree(lauthzid);
-		}
-
-		RETURN_TRUE;
-	}
-
-	/* asynchronous call */
-	rc = ldap_whoami(ld->link, NULL, NULL, &msgid);
+	/* synchronous call */
+	rc = ldap_whoami_s(ld->link, &lauthzid, NULL, NULL);
 	if (rc != LDAP_SUCCESS ) {
 		php_error_docref(NULL TSRMLS_CC, E_WARNING, "Whoami extended operation failed: %s (%d)", ldap_err2string(rc), rc);
 		RETURN_FALSE;
 	}
 
-	rc = ldap_result(ld->link, msgid, 1 /* LDAP_MSG_ALL */, NULL, &ldap_res);
-	if (rc == -1) {
-		php_error_docref(NULL TSRMLS_CC, E_WARNING, "Whoami extended operation failed");
-		RETURN_FALSE;
+	zval_dtor(authzid);
+	if (lauthzid == NULL) {
+		ZVAL_EMPTY_STRING(authzid);
+	} else {
+		ZVAL_STRINGL(authzid, lauthzid->bv_val, lauthzid->bv_len);
+		ldap_memfree(lauthzid->bv_val);
+		ldap_memfree(lauthzid);
 	}
 
-	/* return a PHP control object */
-	RETVAL_RES(zend_register_resource(ldap_res, le_result));
+	RETURN_TRUE;
 }
 /* }}} */
 #endif
@@ -3860,22 +3737,6 @@ ZEND_BEGIN_ARG_INFO_EX(arginfo_ldap_parse_exop, 0, 0, 4)
 	ZEND_ARG_INFO(1, retdata)
 ZEND_END_ARG_INFO()
 #endif
-
-#ifdef HAVE_LDAP_PARSE_PASSWD
-ZEND_BEGIN_ARG_INFO_EX(arginfo_ldap_parse_exop_passwd, 0, 0, 3)
-	ZEND_ARG_INFO(0, link)
-	ZEND_ARG_INFO(0, result)
-	ZEND_ARG_INFO(1, newpasswd)
-ZEND_END_ARG_INFO()
-#endif
-
-#ifdef HAVE_LDAP_PARSE_WHOAMI
-ZEND_BEGIN_ARG_INFO_EX(arginfo_ldap_parse_exop_whoami, 0, 0, 3)
-	ZEND_ARG_INFO(0, link)
-	ZEND_ARG_INFO(0, result)
-	ZEND_ARG_INFO(1, authzid)
-ZEND_END_ARG_INFO()
-#endif
 /* }}} */
 
 /*
@@ -3952,12 +3813,6 @@ const zend_function_entry ldap_functions[] = {
 #ifdef HAVE_LDAP_PARSE_EXTENDED_RESULT
 	PHP_FE(ldap_parse_exop,								arginfo_ldap_parse_exop)
 #endif
-#ifdef HAVE_LDAP_PARSE_PASSWD
-	PHP_FE(ldap_parse_exop_passwd,						arginfo_ldap_parse_exop_passwd)
-#endif
-#ifdef HAVE_LDAP_PARSE_WHOAMI
-	PHP_FE(ldap_parse_exop_whoami,						arginfo_ldap_parse_exop_whoami)
-#endif
 #endif
 
 #if defined(LDAP_API_FEATURE_X_OPENLDAP) && defined(HAVE_3ARG_SETREBINDPROC)

diff --git a/ext/ldap/tests/ldap_exop_passwd.phpt b/ext/ldap/tests/ldap_exop_passwd.phpt
@@ -20,10 +20,7 @@ var_dump(
   $genpw,
   test_bind($host, $port, "cn=userA,$base", $genpw, $protocol_version),
   ldap_exop_passwd($link, "cn=userA,$base", $genpw, "newPassword"),
-  test_bind($host, $port, "cn=userA,$base", "newPassword", $protocol_version),
-  $r = ldap_exop_passwd($link, "cn=userA,$base", "newPassword", ""),
-  ldap_parse_exop_passwd($link, $r, $result),
-  $result
+  test_bind($host, $port, "cn=userA,$base", "newPassword", $protocol_version)
 );
 ?>
 ===DONE===
@@ -41,7 +38,4 @@ string(%d) "%s"
 bool(true)
 bool(true)
 bool(true)
-resource(%d) of type (ldap result)
-bool(true)
-string(%d) "%s"
 ===DONE===
diff --git a/ext/ldap/tests/ldap_exop_passwd_error.phpt b/ext/ldap/tests/ldap_exop_passwd_error.phpt
@@ -13,6 +13,8 @@ $link = ldap_connect_and_bind($host, $port, $user, $passwd, $protocol_version);
 insert_dummy_data($link, $base);
 
 var_dump(ldap_exop_passwd($link, "cn=userA,$base", "wrongPassword", "newPassword"));
+var_dump(ldap_error($link));
+var_dump(ldap_errno($link));
 var_dump(test_bind($host, $port, "cn=userA,$base", "newPassword", $protocol_version));
 ?>
 ===DONE===
@@ -27,6 +29,8 @@ remove_dummy_data($link, $base);
 --EXPECTF--
 Warning: ldap_exop_passwd(): Passwd modify extended operation failed: Server is unwilling to perform (53) in %s on line %d
 bool(false)
+string(30) "Server is unwilling to perform"
+int(53)
 
 Warning: ldap_bind(): Unable to bind to server: Invalid credentials in %s on line %d
 bool(false)

diff --git a/ext/ldap/tests/ldap_exop_whoami.phpt b/ext/ldap/tests/ldap_exop_whoami.phpt
@@ -15,10 +15,7 @@ insert_dummy_data($link, $base);
 // ldap_exop_whoami(resource link [, string authzid])
 var_dump(
   ldap_exop_whoami($link, $authzid),
-  $authzid,
-  $r = ldap_exop_whoami($link),
-  ldap_parse_exop_whoami($link, $r, $result),
-  $result
+  $authzid
 );
 ?>
 ===DONE===
@@ -33,7 +30,4 @@ remove_dummy_data($link, $base);
 --EXPECTF--
 bool(true)
 string(%d) "dn:%s"
-resource(%d) of type (ldap result)
-bool(true)
-string(%d) "dn:%s"
 ===DONE===