[PATCH v3 0/3] MR6333: odbccp32 valid dsn

[Alistair Leslie-Hughes (＠alesliehughes)]

--
  v3: odbccp32: SQLWrite/RemoveDSNFromIniW correctly handle config_mode
      odbccp32: SQLRemoveDSNFromIniW check for valid DSN before delete
      odbccp32: SQLValidDSNW return false on empty string.

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

[PATCH v3 1/3] odbccp32: SQLValidDSNW return false on empty string.

[Alistair Leslie-Hughes]

From: Alistair Leslie-Hughes

---
 dlls/odbccp32/odbccp32.c   | 4 ++--
 dlls/odbccp32/tests/misc.c | 6 ++++++
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/dlls/odbccp32/odbccp32.c b/dlls/odbccp32/odbccp32.c
index dba24e4b673..ab745ef1bb7 100644
--- a/dlls/odbccp32/odbccp32.c
+++ b/dlls/odbccp32/odbccp32.c
@@ -1725,7 +1725,7 @@ BOOL WINAPI SQLValidDSNW(LPCWSTR lpszDSN)
     clear_errors();
     TRACE("%s\n", debugstr_w(lpszDSN));

-    if (!lpszDSN || lstrlenW(lpszDSN) > SQL_MAX_DSN_LENGTH || wcspbrk(lpszDSN, L"[]{}(),;?*=!@\"))
+    if (!lpszDSN || !*lpszDSN || lstrlenW(lpszDSN) > SQL_MAX_DSN_LENGTH || wcspbrk(lpszDSN, L"[]{}(),;?*=!@\"))
     {
         return FALSE;
     }
@@ -1739,7 +1739,7 @@ BOOL WINAPI SQLValidDSN(LPCSTR lpszDSN)
     clear_errors();
     TRACE("%s\n", debugstr_a(lpszDSN));

-    if (!lpszDSN || strlen(lpszDSN) > SQL_MAX_DSN_LENGTH || strpbrk(lpszDSN, invalid))
+    if (!lpszDSN || !*lpszDSN || strlen(lpszDSN) > SQL_MAX_DSN_LENGTH || strpbrk(lpszDSN, invalid))
     {
         return FALSE;
     }
diff --git a/dlls/odbccp32/tests/misc.c b/dlls/odbccp32/tests/misc.c
index e9f879edda6..b94ed811337 100644
--- a/dlls/odbccp32/tests/misc.c
+++ b/dlls/odbccp32/tests/misc.c
@@ -816,6 +816,9 @@ static void test_SQLValidDSN(void)
     /* Max DSN name value */
     ret = SQLValidDSN("12345678901234567890123456789012");
     ok(ret, "got %d\n", ret);
+
+    ret = SQLValidDSN("");
+    ok(!ret, "got %d\n", ret);
 }

static void test_SQLValidDSNW(void)
@@ -843,6 +846,9 @@ static void test_SQLValidDSNW(void)

ret = SQLValidDSNW(L"12345678901234567890123456789012");
     ok(ret, "got %d\n", ret);
+
+    ret = SQLValidDSNW(L"");
+    ok(!ret, "got %d\n", ret);
 }

static void test_SQLConfigDataSource(void)

[PATCH v3 2/3] odbccp32: SQLRemoveDSNFromIniW check for valid DSN before delete

[Alistair Leslie-Hughes]

From: Alistair Leslie-Hughes

---
 dlls/odbccp32/odbccp32.c   | 6 ++++++
 dlls/odbccp32/tests/misc.c | 6 ++++++
 2 files changed, 12 insertions(+)

diff --git a/dlls/odbccp32/odbccp32.c b/dlls/odbccp32/odbccp32.c
index ab745ef1bb7..b77c5698c07 100644
--- a/dlls/odbccp32/odbccp32.c
+++ b/dlls/odbccp32/odbccp32.c
@@ -1586,6 +1586,12 @@ BOOL WINAPI SQLRemoveDSNFromIniW(LPCWSTR lpszDSN)

TRACE("%s\n", debugstr_w(lpszDSN));

+    if (!SQLValidDSNW(lpszDSN))
+    {
+        push_error(ODBC_ERROR_INVALID_DSN, L"Invalid DSN");
+        return FALSE;
+    }
+
     clear_errors();

if (RegOpenKeyW(HKEY_LOCAL_MACHINE, L"Software\ODBC\ODBC.INI\ODBC Data Sources", &hkey) == ERROR_SUCCESS)
diff --git a/dlls/odbccp32/tests/misc.c b/dlls/odbccp32/tests/misc.c
index b94ed811337..af149e6c973 100644
--- a/dlls/odbccp32/tests/misc.c
+++ b/dlls/odbccp32/tests/misc.c
@@ -885,6 +885,12 @@ static void test_SQLWriteDSNToIni(void)

SQLSetConfigMode(ODBC_SYSTEM_DSN);

+    ret = SQLRemoveDSNFromIni("");
+    ok(!ret, "got %d\n", ret);
+
+    ret = SQLRemoveDSNFromIniW(L"");
+    ok(!ret, "got %d\n", ret);
+
     ret = SQLWriteDSNToIni("wine_dbs", "SQL Server");
     if (!ret)
     {

[PATCH v3 3/3] odbccp32: SQLWrite/RemoveDSNFromIniW correctly handle config_mode

[Alistair Leslie-Hughes]

From: Alistair Leslie-Hughes

---
 dlls/odbccp32/odbccp32.c   | 36 +++++++++++++++++++++++++-----
 dlls/odbccp32/tests/misc.c | 45 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 76 insertions(+), 5 deletions(-)

diff --git a/dlls/odbccp32/odbccp32.c b/dlls/odbccp32/odbccp32.c
index b77c5698c07..1676cfa5746 100644
--- a/dlls/odbccp32/odbccp32.c
+++ b/dlls/odbccp32/odbccp32.c
@@ -1582,7 +1582,7 @@ BOOL WINAPI SQLRemoveDriverManager(LPDWORD pdwUsageCount)

BOOL WINAPI SQLRemoveDSNFromIniW(LPCWSTR lpszDSN)
 {
-    HKEY hkey;
+    HKEY hkey, hkeyroot = HKEY_CURRENT_USER;

TRACE("%s\n", debugstr_w(lpszDSN));

@@ -1594,13 +1594,36 @@ BOOL WINAPI SQLRemoveDSNFromIniW(LPCWSTR lpszDSN)

clear_errors();

-    if (RegOpenKeyW(HKEY_LOCAL_MACHINE, L"Software\ODBC\ODBC.INI\ODBC Data Sources", &hkey) == ERROR_SUCCESS)
+    if (config_mode == ODBC_SYSTEM_DSN)
+        hkeyroot = HKEY_LOCAL_MACHINE;
+    else if (config_mode == ODBC_BOTH_DSN)
+    {
+        WCHAR *regpath = malloc( (wcslen(L"Software\ODBC\ODBC.INI\") + wcslen(lpszDSN) + 1) * sizeof(WCHAR) );
+        if (!regpath)
+        {
+            push_error(ODBC_ERROR_OUT_OF_MEM, L"Out of memory");
+            return FALSE;
+        }
+        wcscpy(regpath, L"Software\ODBC\ODBC.INI\");
+        wcscat(regpath, lpszDSN);
+
+        /* ONLY removes one DSN, USER or SYSTEM */
+        if (RegOpenKeyW(HKEY_CURRENT_USER, regpath, &hkey) == ERROR_SUCCESS)
+            hkeyroot = HKEY_CURRENT_USER;
+        else
+            hkeyroot = HKEY_LOCAL_MACHINE;
+
+        RegCloseKey(hkey);
+        free(regpath);
+    }
+
+    if (RegOpenKeyW(hkeyroot, L"Software\ODBC\ODBC.INI\ODBC Data Sources", &hkey) == ERROR_SUCCESS)
     {
         RegDeleteValueW(hkey, lpszDSN);
         RegCloseKey(hkey);
     }

-    if (RegOpenKeyW(HKEY_LOCAL_MACHINE, L"Software\ODBC\ODBC.INI", &hkey) == ERROR_SUCCESS)
+    if (RegOpenKeyW(hkeyroot, L"Software\ODBC\ODBC.INI", &hkey) == ERROR_SUCCESS)
     {
         RegDeleteTreeW(hkey, lpszDSN);
         RegCloseKey(hkey);
@@ -1756,7 +1779,7 @@ BOOL WINAPI SQLValidDSN(LPCSTR lpszDSN)
 BOOL WINAPI SQLWriteDSNToIniW(LPCWSTR lpszDSN, LPCWSTR lpszDriver)
 {
     DWORD ret;
-    HKEY hkey, hkeydriver;
+    HKEY hkey, hkeydriver, hkeyroot = HKEY_CURRENT_USER;
     WCHAR filename[MAX_PATH];

TRACE("%s %s\n", debugstr_w(lpszDSN), debugstr_w(lpszDriver));
@@ -1784,7 +1807,10 @@ BOOL WINAPI SQLWriteDSNToIniW(LPCWSTR lpszDSN, LPCWSTR lpszDriver)
         RegCloseKey(hkey);
     }

-    if ((ret = RegCreateKeyW(HKEY_LOCAL_MACHINE, L"SOFTWARE\ODBC\ODBC.INI", &hkey)) == ERROR_SUCCESS)
+    if (config_mode == ODBC_SYSTEM_DSN)
+        hkeyroot = HKEY_LOCAL_MACHINE;
+
+    if ((ret = RegCreateKeyW(hkeyroot, L"SOFTWARE\ODBC\ODBC.INI", &hkey)) == ERROR_SUCCESS)
     {
         HKEY sources;

diff --git a/dlls/odbccp32/tests/misc.c b/dlls/odbccp32/tests/misc.c
index af149e6c973..b294e465cc7 100644
--- a/dlls/odbccp32/tests/misc.c
+++ b/dlls/odbccp32/tests/misc.c
@@ -876,6 +876,22 @@ static void test_SQLConfigDataSource(void)
     check_error(ODBC_ERROR_COMPONENT_NOT_FOUND);
 }

+static BOOL check_dsn_exists(HKEY key, const WCHAR *dsn)
+{
+    HKEY hkey;
+    WCHAR buffer[256];
+    LONG res;
+
+    wcscpy(buffer, L"Software\ODBC\ODBC.INI\");
+    wcscat(buffer, dsn);
+
+    res = RegOpenKeyExW(key, buffer, 0, KEY_READ, &hkey);
+    if (res)
+        RegCloseKey(hkey);
+
+    return res == ERROR_SUCCESS;
+}
+
 static void test_SQLWriteDSNToIni(void)
 {
     BOOL ret;
@@ -891,6 +907,9 @@ static void test_SQLWriteDSNToIni(void)
     ret = SQLRemoveDSNFromIniW(L"");
     ok(!ret, "got %d\n", ret);

+    ret = check_dsn_exists(HKEY_LOCAL_MACHINE, L"wine_dbs");
+    ok(!ret, "Found registry key\n");
+
     ret = SQLWriteDSNToIni("wine_dbs", "SQL Server");
     if (!ret)
     {
@@ -898,6 +917,18 @@ static void test_SQLWriteDSNToIni(void)
         return;
     }

+    ret = check_dsn_exists(HKEY_LOCAL_MACHINE, L"wine_dbs");
+    ok(ret, "Failed to find registry key\n");
+
+    SQLSetConfigMode(ODBC_USER_DSN);
+    ret = SQLWriteDSNToIni("wine_dbs", "SQL Server");
+    ok(ret, "got %d\n", ret);
+
+    ret = check_dsn_exists(HKEY_CURRENT_USER, L"wine_dbs");
+    ok(ret, "Failed to find registry key\n");
+
+    SQLSetConfigMode(ODBC_SYSTEM_DSN);
+
     if(ret)
     {
         HKEY hkey;
@@ -920,6 +951,18 @@ static void test_SQLWriteDSNToIni(void)
             RegCloseKey(hkey);
         }

+        SQLSetConfigMode(ODBC_BOTH_DSN);
+
+        /* ODBC_BOTH_DSN set and has both System/User DSN but only removes USER. */
+        ret = SQLRemoveDSNFromIni("wine_dbs");
+        ok(ret, "got %d\n", ret);
+
+        ret = check_dsn_exists(HKEY_CURRENT_USER, L"wine_dbs");
+        ok(!ret, "Found registry key\n");
+
+        ret = check_dsn_exists(HKEY_LOCAL_MACHINE, L"wine_dbs");
+        ok(ret, "Failed to find registry key\n");
+
         res = RegOpenKeyExA(HKEY_LOCAL_MACHINE, "Software\ODBC\ODBC.INI\wine_dbs", 0,
                             KEY_READ, &hkey);
         ok(res == ERROR_SUCCESS, "RegOpenKeyExW failed\n");
@@ -938,6 +981,8 @@ static void test_SQLWriteDSNToIni(void)
             RegCloseKey(hkey);
         }

+        SQLSetConfigMode(ODBC_SYSTEM_DSN);
+
         ret = SQLRemoveDSNFromIni("wine_dbs");
         ok(ret, "got %d\n", ret);
     }

[PATCH v3 3/3] odbccp32: SQLWrite/RemoveDSNFromIniW correctly handle config_mode

[Marvin]

Hi,

It looks like your patch introduced the new failures shown below.
Please investigate and fix them before resubmitting your patch.
If they are not new, fixing them anyway would help a lot. Otherwise
please ask for the known failures list to be updated.

The tests also ran into some preexisting test failures. If you know how
to fix them that would be helpful. See the TestBot job for the details:

The full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=147978

Your paranoid android.

=== debian11b (64 bit WoW report) ===

urlmon:
protocol.c:1078: Test failed: unexpected ReportData2
protocol.c:2132: Test failed: unexpected call Read
protocol.c:2132: Test failed: unexpected call Read

[Hans Leidekker (＠hans)]

Hans Leidekker (@hans) commented about dlls/odbccp32/tests/misc.c:
>  check_error(ODBC_ERROR_COMPONENT_NOT_FOUND);
>
> }
>
> +static BOOL check_dsn_exists(HKEY key, const WCHAR *dsn)
> +{
> - HKEY hkey;
> - WCHAR buffer[256];
> - LONG res;
> -
> - wcscpy(buffer, L"Software\ODBC\ODBC.INI\");
> - wcscat(buffer, dsn);
> -
> - res = RegOpenKeyExW(key, buffer, 0, KEY_READ, &hkey);
> - if (res)
> -
>    RegCloseKey(hkey);

`if (!res) RegCloseKey(hkey);`

[Hans Leidekker (＠hans)]

Hans Leidekker (@hans) commented about dlls/odbccp32/odbccp32.c:
>      RegCloseKey(hkey);
>  }
>
> - if ((ret = RegCreateKeyW(HKEY_LOCAL_MACHINE, L"SOFTWARE\ODBC\ODBC.INI", &hkey)) == ERROR_SUCCESS)
> - if (config_mode == ODBC_SYSTEM_DSN)
> -
>    hkeyroot = HKEY_LOCAL_MACHINE;
> -
> - if ((ret = RegCreateKeyW(hkeyroot, L"SOFTWARE\ODBC\ODBC.INI", &hkey)) == ERROR_SUCCESS)

This still doesn't handle ODBC_BOTH_DSN.