[PATCH v4 0/2] MR2481: wldap32: Handle null DN or null message in ldap_delete* and add tests.

[Alex Henrie (＠alexhenrie)]

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=54702

--
  v4: wldap32: Handle null DN or null message in ldap_delete* and add tests.

https://gitlab.winehq.org/wine/wine/-/merge_requests/2481

[PATCH v4 1/2] wldap32/tests: Use the same context in test_ldap_(add|modify_compare).

[Alex Henrie]

From: Alex Henrie

---
 dlls/wldap32/tests/parse.c | 30 ++++++------------------------
 1 file changed, 6 insertions(+), 24 deletions(-)

diff --git a/dlls/wldap32/tests/parse.c b/dlls/wldap32/tests/parse.c
index 34fcb9a4201..f5af100ca60 100644
--- a/dlls/wldap32/tests/parse.c
+++ b/dlls/wldap32/tests/parse.c
@@ -175,17 +175,13 @@ static void test_ldap_bind_sA( void )
     ldap_unbind( ld );
 }

-static void test_ldap_add(void)
+static void test_ldap_add( LDAP *ld )
 {
     char *one_empty_string[] = { (char *)"", NULL };
     LDAPModA empty_equals_empty = { 0, (char *)"", { one_empty_string } };
     LDAPModA *attrs[] = { &empty_equals_empty, NULL };
-    LDAP *ld;
     ULONG ret, num;

-    ld = ldap_initA( (char *)"db.debian.org", 389 );
-    ok( ld != NULL, "ldap_init failed\n" );
-
     ret = ldap_addA( NULL, NULL, NULL );
     ok( ret == (ULONG)-1, "ldap_addA should fail, got %#lx\n", ret );
     ret = ldap_addA( NULL, (char *)"", attrs );
@@ -231,21 +227,15 @@ static void test_ldap_add(void)
     ok( ret == LDAP_PROTOCOL_ERROR, "ldap_add_ext_sA should fail, got %#lx\n", ret );
     ret = ldap_add_ext_sA( ld, (char *)"", attrs, NULL, NULL );
     ok( ret == LDAP_ALREADY_EXISTS, "ldap_add_ext_sA should fail, got %#lx\n", ret );
-
-    ldap_unbind( ld );
 }

-static void test_ldap_modify(void)
+static void test_ldap_modify( LDAP *ld )
 {
     char *one_empty_string[] = { (char *)"", NULL };
     LDAPModA empty_equals_empty = { 0, (char *)"", { one_empty_string } };
     LDAPModA *attrs[] = { &empty_equals_empty, NULL };
-    LDAP *ld;
     ULONG ret, num;

-    ld = ldap_initA( (char *)"db.debian.org", 389 );
-    ok( ld != NULL, "ldap_init failed\n" );
-
     ret = ldap_modifyA( NULL, NULL, NULL );
     ok( ret == (ULONG)-1, "ldap_modifyA should fail, got %#lx\n", ret );
     ret = ldap_modifyA( NULL, (char *)"", attrs );
@@ -291,19 +281,13 @@ static void test_ldap_modify(void)
     ok( ret == LDAP_UNWILLING_TO_PERFORM, "ldap_modify_ext_sA should fail, got %#lx\n", ret );
     ret = ldap_modify_ext_sA( ld, (char *)"", attrs, NULL, NULL );
     ok( ret == LDAP_UNDEFINED_TYPE, "ldap_modify_ext_sA should fail, got %#lx\n", ret );
-
-    ldap_unbind( ld );
 }

-static void test_ldap_compare(void)
+static void test_ldap_compare( LDAP *ld )
 {
     struct berval empty_value = { 0 };
-    LDAP *ld;
     ULONG ret, num;

-    ld = ldap_initA( (char *)"db.debian.org", 389 );
-    ok( ld != NULL, "ldap_init failed\n" );
-
     ret = ldap_compareA( NULL, NULL, NULL, NULL );
     ok( ret == (ULONG)-1, "ldap_compareA should fail, got %#lx\n", ret );
     ret = ldap_compareA( NULL, (char *)"", (char *)"", (char *)"" );
@@ -363,8 +347,6 @@ static void test_ldap_compare(void)
     ok( ret == LDAP_UNDEFINED_TYPE, "ldap_compare_ext_sA should fail, got %#lx\n", ret );
     ret = ldap_compare_ext_sA( ld, (char *)"", (char *)"", (char *)"", &empty_value, NULL, NULL );
     ok( ret == LDAP_UNDEFINED_TYPE, "ldap_compare_ext_sA should fail, got %#lx\n", ret );
-
-    ldap_unbind( ld );
 }

static void test_ldap_server_control( void )
@@ -489,13 +471,13 @@ START_TEST (parse)
     test_ldap_paged_search();
     test_ldap_server_control();
     test_ldap_bind_sA();
-    test_ldap_add();
-    test_ldap_modify();
-    test_ldap_compare();

ld = ldap_initA( (char *)"db.debian.org", 389 );
     ok( ld != NULL, "ldap_init failed\n" );

+    test_ldap_add( ld );
+    test_ldap_modify( ld );
+    test_ldap_compare( ld );
     test_ldap_parse_sort_control( ld );
     test_ldap_search_extW( ld );
     test_ldap_get_optionW( ld );

[PATCH v4 2/2] wldap32: Handle null DN or null message in ldap_delete* and add tests.

[Alex Henrie]

From: Alex Henrie

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=54702
---
 dlls/wldap32/delete.c      |  8 ++++----
 dlls/wldap32/tests/parse.c | 28 ++++++++++++++++++++++++++++
 2 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/dlls/wldap32/delete.c b/dlls/wldap32/delete.c
index b085142d83d..8539b7266eb 100644
--- a/dlls/wldap32/delete.c
+++ b/dlls/wldap32/delete.c
@@ -72,7 +72,7 @@ ULONG CDECL ldap_delete_extA( LDAP *ld, char *dn, LDAPControlA **serverctrls, LD

TRACE( "(%p, %s, %p, %p, %p)\n", ld, debugstr_a(dn), serverctrls, clientctrls, message );

-    if (!ld) return WLDAP32_LDAP_PARAM_ERROR;
+    if (!ld || !message) return WLDAP32_LDAP_PARAM_ERROR;

if (dn && !(dnW = strAtoW( dn ))) goto exit;
     if (serverctrls && !(serverctrlsW = controlarrayAtoW( serverctrls ))) goto exit;
@@ -99,9 +99,9 @@ ULONG CDECL ldap_delete_extW( LDAP *ld, WCHAR *dn, LDAPControlW **serverctrls, L

TRACE( "(%p, %s, %p, %p, %p)\n", ld, debugstr_w(dn), serverctrls, clientctrls, message );

-    if (!ld) return WLDAP32_LDAP_PARAM_ERROR;
+    if (!ld || !message) return WLDAP32_LDAP_PARAM_ERROR;

-    if (dn && !(dnU = strWtoU( dn ))) goto exit;
+    if (!(dnU = dn ? strWtoU( dn ) : strdup( "" ))) goto exit;
     if (serverctrls && !(serverctrlsU = controlarrayWtoU( serverctrls ))) goto exit;
     if (clientctrls && !(clientctrlsU = controlarrayWtoU( clientctrls ))) goto exit;
     else
@@ -155,7 +155,7 @@ ULONG CDECL ldap_delete_ext_sW( LDAP *ld, WCHAR *dn, LDAPControlW **serverctrls,

if (!ld) return WLDAP32_LDAP_PARAM_ERROR;

-    if (dn && !(dnU = strWtoU( dn ))) goto exit;
+    if (!(dnU = dn ? strWtoU( dn ) : strdup( "" ))) goto exit;
     if (serverctrls && !(serverctrlsU = controlarrayWtoU( serverctrls ))) goto exit;
     if (clientctrls && !(clientctrlsU = controlarrayWtoU( clientctrls ))) goto exit;
     else
diff --git a/dlls/wldap32/tests/parse.c b/dlls/wldap32/tests/parse.c
index f5af100ca60..19e7a48b471 100644
--- a/dlls/wldap32/tests/parse.c
+++ b/dlls/wldap32/tests/parse.c
@@ -349,6 +349,33 @@ static void test_ldap_compare( LDAP *ld )
     ok( ret == LDAP_UNDEFINED_TYPE, "ldap_compare_ext_sA should fail, got %#lx\n", ret );
 }

+static void test_ldap_delete( LDAP *ld )
+{
+    ULONG ret, num;
+
+    ret = ldap_deleteA( NULL, NULL );
+    ok( ret == (ULONG)-1, "ldap_deleteA should fail, got %#lx\n", ret );
+    ret = ldap_deleteA( NULL, (char *)"" );
+    ok( ret == (ULONG)-1, "ldap_deleteA should fail, got %#lx\n", ret );
+
+    ret = ldap_delete_sA( NULL, NULL );
+    ok( ret == LDAP_PARAM_ERROR, "ldap_delete_sA should fail, got %#lx\n", ret );
+    ret = ldap_delete_sA( NULL, (char *)"" );
+    ok( ret == LDAP_PARAM_ERROR, "ldap_delete_sA should fail, got %#lx\n", ret );
+
+    ret = ldap_delete_extA( NULL, NULL, NULL, NULL, NULL );
+    ok( ret == LDAP_PARAM_ERROR, "ldap_delete_extA should fail, got %#lx\n", ret );
+    ret = ldap_delete_extA( NULL, (char *)"", NULL, NULL, &num );
+    ok( ret == LDAP_PARAM_ERROR, "ldap_delete_extA should fail, got %#lx\n", ret );
+    ret = ldap_delete_extA( ld, (char *)"", NULL, NULL, NULL );
+    ok( ret == LDAP_PARAM_ERROR, "ldap_delete_extA should fail, got %#lx\n", ret );
+
+    ret = ldap_delete_ext_sA( NULL, NULL, NULL, NULL );
+    ok( ret == LDAP_PARAM_ERROR, "ldap_delete_ext_sA should fail, got %#lx\n", ret );
+    ret = ldap_delete_ext_sA( NULL, (char *)"", NULL, NULL );
+    ok( ret == LDAP_PARAM_ERROR, "ldap_delete_ext_sA should fail, got %#lx\n", ret );
+}
+
 static void test_ldap_server_control( void )
 {
     /* SEQUENCE  { INTEGER :: 0x07 } */
@@ -478,6 +505,7 @@ START_TEST (parse)
     test_ldap_add( ld );
     test_ldap_modify( ld );
     test_ldap_compare( ld );
+    test_ldap_delete( ld );
     test_ldap_parse_sort_control( ld );
     test_ldap_search_extW( ld );
     test_ldap_get_optionW( ld );

[PATCH v4 0/2] MR2481: wldap32: Handle null DN or null message in ldap_delete* and add tests. - approved

[Hans Leidekker (＠hans)]

This merge request was approved by Hans Leidekker.

[Alex Henrie (＠alexhenrie)]

On Fri Mar 24 16:09:43 2023 +0000, Alex Henrie wrote:
> changed this line in [version 4 of the diff](/wine/wine/-/merge_requests/2481/diffs?diff_id=38897&start_sha=fcabe7618540629fbd8663beda6b58393a13b534#05336bf15ebbf9de3ea8d3310da1d7d5b75f5800_361_360)

Thanks for catching that; I didn't immediately notice that the tests in test_ldap_parse_sort_control were being skipped.