[PATCH v2 0/3] MR6333: odbccp32 valid dsn
-- v2: 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
From: Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> --- 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) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6333
From: Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> --- 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) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6333
From: Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> --- dlls/odbccp32/odbccp32.c | 28 +++++++++++++++++++++++----- dlls/odbccp32/tests/misc.c | 14 ++++++++++++++ 2 files changed, 37 insertions(+), 5 deletions(-) diff --git a/dlls/odbccp32/odbccp32.c b/dlls/odbccp32/odbccp32.c index b77c5698c07..e7ba731bb3e 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,28 @@ 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\\ODBC Data Sources\\") + wcslen(lpszDSN) + 1) * sizeof(WCHAR) ); + /* ONLY removes one DSN, USER or SYSTEM */ + if (RegOpenKeyW(HKEY_CURRENT_USER, L"Software\\ODBC\\ODBC.INI\\ODBC Data Sources", &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 +1771,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 +1799,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; diff --git a/dlls/odbccp32/tests/misc.c b/dlls/odbccp32/tests/misc.c index af149e6c973..c2ad7340fd9 100644 --- a/dlls/odbccp32/tests/misc.c +++ b/dlls/odbccp32/tests/misc.c @@ -898,6 +898,12 @@ static void test_SQLWriteDSNToIni(void) return; } + SQLSetConfigMode(ODBC_USER_DSN); + ret = SQLWriteDSNToIni("wine_dbs", "SQL Server"); + ok(ret, "got %d\n", ret); + + SQLSetConfigMode(ODBC_SYSTEM_DSN); + if(ret) { HKEY hkey; @@ -920,6 +926,12 @@ 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); + res = RegOpenKeyExA(HKEY_LOCAL_MACHINE, "Software\\ODBC\\ODBC.INI\\wine_dbs", 0, KEY_READ, &hkey); ok(res == ERROR_SUCCESS, "RegOpenKeyExW failed\n"); @@ -938,6 +950,8 @@ static void test_SQLWriteDSNToIni(void) RegCloseKey(hkey); } + SQLSetConfigMode(ODBC_SYSTEM_DSN); + ret = SQLRemoveDSNFromIni("wine_dbs"); ok(ret, "got %d\n", ret); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6333
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_SYSTEM_DSN) + hkeyroot = HKEY_LOCAL_MACHINE; + else if (config_mode == ODBC_BOTH_DSN) + { + WCHAR *regpath = malloc( (wcslen(L"Software\\ODBC\\ODBC.INI\\ODBC Data Sources\\") + wcslen(lpszDSN) + 1) * sizeof(WCHAR) ); + /* ONLY removes one DSN, USER or SYSTEM */ + if (RegOpenKeyW(HKEY_CURRENT_USER, L"Software\\ODBC\\ODBC.INI\\ODBC Data Sources", &hkey) == ERROR_SUCCESS) + hkeyroot = HKEY_CURRENT_USER; + else + hkeyroot = HKEY_LOCAL_MACHINE; + + RegCloseKey(hkey); + free(regpath);
I guess you meant to build regpath and pass it to RegOpenKeyW(). Please add registry checks to the tests. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6333#note_79741
participants (3)
-
Alistair Leslie-Hughes -
Alistair Leslie-Hughes (＠alesliehughes)
-
Hans Leidekker (＠hans)