[PATCH 0/5] MR4480: oledb32: Remove the trailing semicolon.
Also, this fixes an out-of-bounds write because the space for the terminating '\0' wasn't included previously. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4480
From: Zhiyi Zhang <zzhang(a)codeweavers.com> --- dlls/oledb32/tests/database.c | 42 +++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/dlls/oledb32/tests/database.c b/dlls/oledb32/tests/database.c index ebd3cb2fadf..bd940634847 100644 --- a/dlls/oledb32/tests/database.c +++ b/dlls/oledb32/tests/database.c @@ -530,6 +530,25 @@ static void test_errorinfo(void) IUnknown_Release(unk); } +#define expect_initstring(a, b, c) _expect_initstring(__LINE__, a, b, c) +static void _expect_initstring(int line, IDataInitialize *datainit, const WCHAR *initstring, const WCHAR *expected) +{ + IDBInitialize *dbinit = NULL; + WCHAR *result = NULL; + HRESULT hr; + + hr = IDataInitialize_GetDataSource(datainit, NULL, CLSCTX_INPROC_SERVER, initstring, + &IID_IDBInitialize, (IUnknown**)&dbinit); + ok_(__FILE__, line)(hr == S_OK, "got %08lx\n", hr); + hr = IDataInitialize_GetInitializationString(datainit, (IUnknown*)dbinit, 0, &result); + ok_(__FILE__, line)(hr == S_OK, "got %08lx\n", hr); + todo_wine + ok_(__FILE__, line)(!lstrcmpW(expected, result), "expected %s, got %s\n", + wine_dbgstr_w(expected), wine_dbgstr_w(result)); + CoTaskMemFree(result); + IDBInitialize_Release(dbinit); +} + static void test_initializationstring(void) { static const WCHAR *initstring_default = L"Data Source=dummy;"; @@ -539,6 +558,20 @@ static void test_initializationstring(void) static const WCHAR *initstring_mode = L"Provider=MSDASQL.1;Data Source=dummy;Mode=invalid"; static const WCHAR *initstring_mode2 = L"Provider=MSDASQL.1;Data Source=dummy;Mode=WriteRead"; static const WCHAR *initstring_mode3 = L"Provider=MSDASQL.1;Data Source=dummy;Mode=ReadWRITE"; + static const WCHAR *initstring_quote_semicolon = L"Provider=MSDASQL.1;" + "Data Source=dummy;" + "Extended Properties=\"ConnectTo=11.0;Cell Error Mode=TextValue;Optimize Response=3;\""; + static const WCHAR *initstring_duplicate = L"Provider=MSDASQL.1;" + "Data Source=dummy;" + "Data Source=dummy;"; + static const WCHAR *initstring_skip_properties = L"Provider=MSDASQL.1;" + "Data Source=dummy;" + "Window Handle=0;" + "Prompt=4;" + "Connect Timeout=1000;" + "General Timeout=1000;" + "OLE DB Services=0;" + "Locale Identifier=1033"; IDataInitialize *datainit = NULL; IDBInitialize *dbinit; HRESULT hr; @@ -597,6 +630,15 @@ static void test_initializationstring(void) else ok(dbinit == NULL, "got %p\n", dbinit); + /* Test quoting property values */ + expect_initstring(datainit, initstring_quote_semicolon, initstring_quote_semicolon); + + /* Test duplicate properties */ + expect_initstring(datainit, initstring_duplicate, initstring_msdasql); + + /* Test properties that should be skipped */ + expect_initstring(datainit, initstring_skip_properties, initstring_msdasql); + IDataInitialize_Release(datainit); } } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4480
From: Zhiyi Zhang <zzhang(a)codeweavers.com> Also, this fixes an out-of-bounds write because the space for the terminating '\0' wasn't included previously. --- dlls/oledb32/datainit.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/dlls/oledb32/datainit.c b/dlls/oledb32/datainit.c index c2066712f8b..0d321932991 100644 --- a/dlls/oledb32/datainit.c +++ b/dlls/oledb32/datainit.c @@ -903,7 +903,7 @@ static HRESULT WINAPI datainit_GetInitializationString(IDataInitialize *iface, I IDBProperties_Release(props); /* check if we need to skip password */ - len = lstrlenW(progid) + lstrlenW(providerW) + 1; /* including ';' */ + len = lstrlenW(progid) + lstrlenW(providerW) + 1; /* including '\0' */ for (i = 0; i < count; i++) { WCHAR *descr = get_propinfo_descr(&propset->rgProperties[i], propinfoset); @@ -926,7 +926,6 @@ static HRESULT WINAPI datainit_GetInitializationString(IDataInitialize *iface, I /* provider name */ lstrcatW(*init_string, providerW); lstrcatW(*init_string, progid); - lstrcatW(*init_string, L";"); CoTaskMemFree(progid); for (i = 0; i < count; i++) @@ -938,10 +937,10 @@ static HRESULT WINAPI datainit_GetInitializationString(IDataInitialize *iface, I descr = get_propinfo_descr(&propset->rgProperties[i], propinfoset); if (descr) { + lstrcatW(*init_string, L";"); lstrcatW(*init_string, descr); lstrcatW(*init_string, L"="); write_propvalue_str(*init_string, &propset->rgProperties[i]); - lstrcatW(*init_string, L";"); } } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4480
From: Zhiyi Zhang <zzhang(a)codeweavers.com> count was assigned to the property set count, not the count of properties in each property set. Fix a Power Pivot for Excel 2010 connection error when opening a Power Pivot window. --- dlls/oledb32/datainit.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/dlls/oledb32/datainit.c b/dlls/oledb32/datainit.c index 0d321932991..95ff7013e92 100644 --- a/dlls/oledb32/datainit.c +++ b/dlls/oledb32/datainit.c @@ -853,7 +853,7 @@ static HRESULT WINAPI datainit_GetInitializationString(IDataInitialize *iface, I DBPROPINFOSET *propinfoset; IDBProperties *props; DBPROPIDSET propidset; - ULONG count, infocount; + ULONG propset_count, infocount; WCHAR *progid, *desc; DBPROPSET *propset; IPersist *persist; @@ -889,8 +889,8 @@ static HRESULT WINAPI datainit_GetInitializationString(IDataInitialize *iface, I propidset.cPropertyIDs = 0; propidset.guidPropertySet = DBPROPSET_DBINIT; propset = NULL; - count = 0; - hr = IDBProperties_GetProperties(props, 1, &propidset, &count, &propset); + propset_count = 0; + hr = IDBProperties_GetProperties(props, 1, &propidset, &propset_count, &propset); if (FAILED(hr)) { WARN("failed to get data source properties, 0x%08lx\n", hr); @@ -904,7 +904,7 @@ static HRESULT WINAPI datainit_GetInitializationString(IDataInitialize *iface, I /* check if we need to skip password */ len = lstrlenW(progid) + lstrlenW(providerW) + 1; /* including '\0' */ - for (i = 0; i < count; i++) + for (i = 0; i < propset->cProperties; i++) { WCHAR *descr = get_propinfo_descr(&propset->rgProperties[i], propinfoset); if (descr) @@ -928,7 +928,7 @@ static HRESULT WINAPI datainit_GetInitializationString(IDataInitialize *iface, I lstrcatW(*init_string, progid); CoTaskMemFree(progid); - for (i = 0; i < count; i++) + for (i = 0; i < propset->cProperties; i++) { WCHAR *descr; @@ -944,7 +944,7 @@ static HRESULT WINAPI datainit_GetInitializationString(IDataInitialize *iface, I } } - free_dbpropset(count, propset); + free_dbpropset(propset_count, propset); free_dbpropinfoset(infocount, propinfoset); CoTaskMemFree(desc); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4480
From: Zhiyi Zhang <zzhang(a)codeweavers.com> --- dlls/oledb32/datainit.c | 42 +++++++++++++++++++++++++++++++++++------ 1 file changed, 36 insertions(+), 6 deletions(-) diff --git a/dlls/oledb32/datainit.c b/dlls/oledb32/datainit.c index 95ff7013e92..801317f82a5 100644 --- a/dlls/oledb32/datainit.c +++ b/dlls/oledb32/datainit.c @@ -780,18 +780,30 @@ static HRESULT WINAPI datainit_GetDataSource(IDataInitialize *iface, IUnknown *o /* returns character length of string representation */ static int get_propvalue_length(DBPROP *prop) { + VARIANT *v = &prop->vValue; VARIANT str; HRESULT hr; + int length; - if (V_VT(&prop->vValue) == VT_BSTR) return SysStringLen(V_BSTR(&prop->vValue)); + if (V_VT(v) == VT_BSTR) + { + length = SysStringLen(V_BSTR(v)); + /* Quotes values with '\"' if the value contains semicolons */ + if (wcsstr(V_BSTR(v), L";")) + length += 2; + return length; + } VariantInit(&str); - hr = VariantChangeType(&str, &prop->vValue, 0, VT_BSTR); + hr = VariantChangeType(&str, v, 0, VT_BSTR); if (hr == S_OK) { - int len = SysStringLen(V_BSTR(&str)); + length = SysStringLen(V_BSTR(&str)); + /* Quotes values with '\"' if the value contains semicolons */ + if (wcsstr(V_BSTR(&str), L";")) + length += 2; VariantClear(&str); - return len; + return length; } return 0; @@ -805,7 +817,16 @@ static void write_propvalue_str(WCHAR *str, DBPROP *prop) if (V_VT(v) == VT_BSTR) { - lstrcatW(str, V_BSTR(v)); + if (wcsstr(V_BSTR(v), L";")) + { + lstrcatW(str, L"\""); + lstrcatW(str, V_BSTR(v)); + lstrcatW(str, L"\""); + } + else + { + lstrcatW(str, V_BSTR(v)); + } return; } @@ -813,7 +834,16 @@ static void write_propvalue_str(WCHAR *str, DBPROP *prop) hr = VariantChangeType(&vstr, v, VARIANT_ALPHABOOL, VT_BSTR); if (hr == S_OK) { - lstrcatW(str, V_BSTR(&vstr)); + if (wcsstr(V_BSTR(&vstr), L";")) + { + lstrcatW(str, L"\""); + lstrcatW(str, V_BSTR(&vstr)); + lstrcatW(str, L"\""); + } + else + { + lstrcatW(str, V_BSTR(&vstr)); + } VariantClear(&vstr); } } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4480
From: Zhiyi Zhang <zzhang(a)codeweavers.com> The following properties are removed: 1. Duplicate properties. 2. Properties without value. 3. Properties that should be ignored according to MSDN. 4. DBPROP_AUTH_PASSWORD when include_pass is FALSE. --- dlls/oledb32/datainit.c | 51 ++++++++++++++++++++++++++++++----- dlls/oledb32/tests/database.c | 5 ++-- 2 files changed, 47 insertions(+), 9 deletions(-) diff --git a/dlls/oledb32/datainit.c b/dlls/oledb32/datainit.c index 801317f82a5..b185ff4a128 100644 --- a/dlls/oledb32/datainit.c +++ b/dlls/oledb32/datainit.c @@ -875,6 +875,36 @@ static void free_dbpropinfoset(ULONG count, DBPROPINFOSET *propinfoset) CoTaskMemFree(propinfoset); } +/* Whether a property should be skipped in datainit_GetInitializationString() */ +static BOOL skip_property(const DBPROPSET *propset, unsigned int prop_index, boolean include_pass) +{ + DWORD prop_id = propset->rgProperties[prop_index].dwPropertyID; + unsigned int i; + + /* Skip password if include_pass is FALSE */ + if (!include_pass && prop_id == DBPROP_AUTH_PASSWORD) + return TRUE; + + /* Skip these properties according to the function spec */ + if (prop_id == DBPROP_INIT_ASYNCH + || prop_id == DBPROP_INIT_HWND + || prop_id == DBPROP_INIT_PROMPT + || prop_id == DBPROP_INIT_TIMEOUT + || prop_id == DBPROP_INIT_GENERALTIMEOUT + || prop_id == DBPROP_INIT_OLEDBSERVICES + || prop_id == DBPROP_INIT_LCID) /* DBPROP_INIT_LCID should also be ignored according to tests */ + return TRUE; + + /* Skip duplicate properties */ + for (i = prop_index + 1; i < propset->cProperties; i++) + { + if (propset->rgProperties[i].dwPropertyID == prop_id) + return TRUE; + } + + return FALSE; +} + static HRESULT WINAPI datainit_GetInitializationString(IDataInitialize *iface, IUnknown *datasource, boolean include_pass, LPWSTR *init_string) { @@ -884,12 +914,12 @@ static HRESULT WINAPI datainit_GetInitializationString(IDataInitialize *iface, I IDBProperties *props; DBPROPIDSET propidset; ULONG propset_count, infocount; + ULONG i, len, propvalue_length; WCHAR *progid, *desc; DBPROPSET *propset; IPersist *persist; HRESULT hr; CLSID clsid; - ULONG i, len; TRACE("(%p)->(%p %d %p)\n", This, datasource, include_pass, init_string); @@ -936,12 +966,17 @@ static HRESULT WINAPI datainit_GetInitializationString(IDataInitialize *iface, I len = lstrlenW(progid) + lstrlenW(providerW) + 1; /* including '\0' */ for (i = 0; i < propset->cProperties; i++) { - WCHAR *descr = get_propinfo_descr(&propset->rgProperties[i], propinfoset); + WCHAR *descr; + + if (skip_property(propset, i, TRUE)) + continue; + + descr = get_propinfo_descr(&propset->rgProperties[i], propinfoset); if (descr) { - /* include '=' and ';' */ - len += lstrlenW(descr) + 2; - len += get_propvalue_length(&propset->rgProperties[i]); + propvalue_length = get_propvalue_length(&propset->rgProperties[i]); + if (propvalue_length) + len += lstrlenW(descr) + propvalue_length + 2; /* include ';' and '=' */ } if ((propset->rgProperties[i].dwPropertyID == DBPROP_AUTH_PERSIST_SENSITIVE_AUTHINFO) && @@ -962,11 +997,15 @@ static HRESULT WINAPI datainit_GetInitializationString(IDataInitialize *iface, I { WCHAR *descr; - if (!include_pass && propset->rgProperties[i].dwPropertyID == DBPROP_AUTH_PASSWORD) continue; + if (skip_property(propset, i, include_pass)) + continue; descr = get_propinfo_descr(&propset->rgProperties[i], propinfoset); if (descr) { + if (!get_propvalue_length(&propset->rgProperties[i])) + continue; + lstrcatW(*init_string, L";"); lstrcatW(*init_string, descr); lstrcatW(*init_string, L"="); diff --git a/dlls/oledb32/tests/database.c b/dlls/oledb32/tests/database.c index bd940634847..6296ecfe1ef 100644 --- a/dlls/oledb32/tests/database.c +++ b/dlls/oledb32/tests/database.c @@ -542,7 +542,6 @@ static void _expect_initstring(int line, IDataInitialize *datainit, const WCHAR ok_(__FILE__, line)(hr == S_OK, "got %08lx\n", hr); hr = IDataInitialize_GetInitializationString(datainit, (IUnknown*)dbinit, 0, &result); ok_(__FILE__, line)(hr == S_OK, "got %08lx\n", hr); - todo_wine ok_(__FILE__, line)(!lstrcmpW(expected, result), "expected %s, got %s\n", wine_dbgstr_w(expected), wine_dbgstr_w(result)); CoTaskMemFree(result); @@ -596,8 +595,8 @@ static void test_initializationstring(void) if(hr == S_OK) { trace("Init String: %s\n", wine_dbgstr_w(initstring)); - todo_wine ok(!lstrcmpW(initstring_msdasql, initstring) || - !lstrcmpW(initstring_sqloledb, initstring), "got %s\n", wine_dbgstr_w(initstring)); + ok(!lstrcmpW(initstring_msdasql, initstring) || + !lstrcmpW(initstring_sqloledb, initstring), "got %s\n", wine_dbgstr_w(initstring)); CoTaskMemFree(initstring); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4480
participants (2)
-
Zhiyi Zhang -
Zhiyi Zhang (＠zhiyi)