[PATCH 0/3] MR6333: odbccp32 valid dsn

[PATCH 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 2/3] odbccp32: SQLRemoveDSNFromIniW check for valid DSN before delete

[Alistair Leslie-Hughes]

From: Alistair Leslie-Hughes

---
 dlls/odbccp32/odbccp32.c | 6 ++++++
 1 file changed, 6 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)

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

[Alistair Leslie-Hughes]

From: Alistair Leslie-Hughes

---
 dlls/odbccp32/odbccp32.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/dlls/odbccp32/odbccp32.c b/dlls/odbccp32/odbccp32.c
index b77c5698c07..6c368ac3d44 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_LOCAL_MACHINE;;

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

@@ -1594,13 +1594,16 @@ 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_USER_DSN)
+        hkeyroot = HKEY_CURRENT_USER;
+
+    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 +1759,7 @@ BOOL WINAPI SQLValidDSN(LPCSTR lpszDSN)
 BOOL WINAPI SQLWriteDSNToIniW(LPCWSTR lpszDSN, LPCWSTR lpszDriver)
 {
     DWORD ret;
-    HKEY hkey, hkeydriver;
+    HKEY hkey, hkeydriver, hkeyroot = HKEY_LOCAL_MACHINE;
     WCHAR filename[MAX_PATH];

TRACE("%s %s\n", debugstr_w(lpszDSN), debugstr_w(lpszDriver));
@@ -1784,7 +1787,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_USER_DSN)
+        hkeyroot = HKEY_CURRENT_USER;
+
+    if ((ret = RegCreateKeyW(hkeyroot, L"SOFTWARE\ODBC\ODBC.INI", &hkey)) == ERROR_SUCCESS)
     {
         HKEY sources;

[PATCH 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=147891

Your paranoid android.

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

ddraw:
ddraw7.c:3822: Test failed: Expected (0,0)-(640,480), got (0,0)-(1024,768).
ddraw7.c:3847: Test failed: Expected (0,0)-(640,480), got (0,0)-(1024,768).

[Hans Leidekker (＠hans)]

Hans Leidekker (@hans) commented about dlls/odbccp32/odbccp32.c:
>  TRACE("%s\n", debugstr_w(lpszDSN));
>
> - if (!SQLValidDSNW(lpszDSN))
> - {
> -
>    push_error(ODBC_ERROR_INVALID_DSN, L"Invalid DSN");
> -
>    return FALSE;
> - }

Could you add a test for this?

[Hans Leidekker (＠hans)]

Hans Leidekker (@hans) commented about dlls/odbccp32/odbccp32.c:
>  clear_errors();
>
> - if (RegOpenKeyW(HKEY_LOCAL_MACHINE, L"Software\ODBC\ODBC.INI\ODBC Data Sources", &hkey) == ERROR_SUCCESS)
> - if (config_mode == ODBC_USER_DSN)
> -
>    hkeyroot = HKEY_CURRENT_USER;
> -

What about ODBC_BOTH_DSN? This could also use some tests.