[PATCH 0/4] MR9639: Fix the integration of PowerPoint add-in in MS Office 2016
Encountered an issue with PowerPoint add-in integration. Reviewing the logs revealed the problem was due to the lack of CLSID lookup via the CurVer section and incorrect handling of the PARAMFLAG_NONE flag. A test was conducted in Windows to verify the handling of PARAMFLAG_NONE, PARAMFLAG_FIN, and PARAMFLAG_FOUT. Test class: interface ITest : IDispatch { [id(1)] HRESULT TestMethod(BSTR* value); [id(2)] HRESULT TestMethodOut([out] BSTR* retVal); [id(3)] HRESULT TestMethodIn([in] BSTR* inVal); }; Results in Windows: --------TestMethod-------- Variant type: 0x0008 (VT_BSTR) [CTest::TestMethod] Hello from VT_BSTR Invoke returned: 0x00000000 -------TestMethodIn------- Variant type: 0x0008 (VT_BSTR) [CTest::TestMethodIn] Hello from VT_BSTR Invoke returned: 0x00000000 -------TestMethodOut------- Variant type: 0x0008 (VT_BSTR) [CTest::TestMethodOut] (null) Invoke returned: 0x00000000 Results in Wine: --------TestMethod-------- Variant type: 0x0008 (VT_BSTR) [CTest::TestMethod] (null) Invoke returned: 0x00000000 -------TestMethodIn------- Variant type: 0x0008 (VT_BSTR) [CTest::TestMethodIn] Hello from VT_BSTR Invoke returned: 0x00000000 -------TestMethodOut------- Variant type: 0x0008 (VT_BSTR) [CTest::TestMethodOut] (null) Therefore, parameters with the PARAMFLAG_NONE flag are implied to be input parameters in Windows and are handled accordingly as PARAMFLAG_FIN. Added tests for the implemented changes. In the last two tests (their code is provided below), the CLSID is nullified in the guid_from_string function, which is why this test is left with a todo_wine tag. ``` clsid = CLSID_StdFont; hr = CLSIDFromString(L"MyApp.DocumentTest.3", &clsid); todo_wine ok(IsEqualCLSID(&clsid, &CLSID_StdFont), "Unexpected clsid %s.\n", wine_dbgstr_guid(&clsid)); clsid = CLSID_StdFont; hr = CLSIDFromString(L"MyApp.DocumentTest.5", &clsid); todo_wine ok(IsEqualCLSID(&clsid, &CLSID_StdFont), "Unexpected clsid %s.\n", wine_dbgstr_guid(&clsid)); ``` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9639
From: Ivan Lyugaev <valy(a)etersoft.ru> Some COM applications register version-independent ProgIDs that point to version-specific classes through the CurVer registry key. This implements the correct logic for handling such cases --- dlls/combase/combase.c | 93 +++++++++++++++++++++++++++++++++++++++--- 1 file changed, 87 insertions(+), 6 deletions(-) diff --git a/dlls/combase/combase.c b/dlls/combase/combase.c index 34a9a42dc2a..e6e083dcb1e 100644 --- a/dlls/combase/combase.c +++ b/dlls/combase/combase.c @@ -31,6 +31,8 @@ #include "wine/debug.h" +#define CLSID_FROM_STRING_RECURSION 0x80077777 + WINE_DEFAULT_DEBUG_CHANNEL(ole); HINSTANCE hProxyDll; @@ -1441,7 +1443,7 @@ static BOOL guid_from_string(LPCWSTR s, GUID *id) return FALSE; } -static HRESULT clsid_from_string_reg(LPCOLESTR progid, CLSID *clsid) +static HRESULT clsid_from_string_reg_recursive(LPCOLESTR progid, CLSID *clsid, WCHAR **visited_progids, int call_depth) { WCHAR buf2[CHARS_IN_GUID]; LONG buf2len = sizeof(buf2); @@ -1449,35 +1451,114 @@ static HRESULT clsid_from_string_reg(LPCOLESTR progid, CLSID *clsid) WCHAR *buf; memset(clsid, 0, sizeof(*clsid)); + + for (int i = 0; i < call_depth; i++) + { + if (visited_progids[i] && !lstrcmpW(visited_progids[i], progid)) + { + return CLSID_FROM_STRING_RECURSION; + } + } + + if (call_depth >= 10) + { + return CLSID_FROM_STRING_RECURSION; + } + + visited_progids[call_depth] = _wcsdup(progid); + if (!visited_progids[call_depth]) + { + return E_OUTOFMEMORY; + } + call_depth++; + buf = malloc((lstrlenW(progid) + 8) * sizeof(WCHAR)); - if (!buf) return E_OUTOFMEMORY; + if (!buf) + { + free(visited_progids[call_depth-1]); + visited_progids[call_depth-1] = NULL; + return E_OUTOFMEMORY; + } lstrcpyW(buf, progid); lstrcatW(buf, L"\\CLSID"); if (open_classes_key(HKEY_CLASSES_ROOT, buf, MAXIMUM_ALLOWED, &xhkey)) { + WCHAR *curver; + LONG curverlen = 0; + free(buf); - WARN("couldn't open key for ProgID %s\n", debugstr_w(progid)); + + buf = malloc((lstrlenW(progid) + 9) * sizeof(WCHAR)); + lstrcpyW(buf, progid); + lstrcatW(buf, L"\\CurVer"); + + if (open_classes_key(HKEY_CLASSES_ROOT, buf, MAXIMUM_ALLOWED, &xhkey)) + { + WARN("couldn't open key for ProgID %s\n", debugstr_w(progid)); + free(buf); + free(visited_progids[call_depth-1]); + visited_progids[call_depth-1] = NULL; + return CO_E_CLASSSTRING; + } + + free(buf); + + if (RegQueryValueW(xhkey, NULL, NULL, &curverlen) == ERROR_SUCCESS) + { + curver = malloc(curverlen); + if (curver) + { + if (!RegQueryValueW(xhkey, NULL, curver, &curverlen)) + { + RegCloseKey(xhkey); + free(curver); + return clsid_from_string_reg_recursive(curver, clsid, visited_progids, call_depth); + } + free(curver); + } + } + else + { + WARN("couldn't query CurVer value size for ProgID %s\n", debugstr_w(progid)); + } + + RegCloseKey(xhkey); + free(visited_progids[call_depth-1]); + visited_progids[call_depth-1] = NULL; return CO_E_CLASSSTRING; } + free(buf); if (RegQueryValueW(xhkey, NULL, buf2, &buf2len)) { RegCloseKey(xhkey); WARN("couldn't query clsid value for ProgID %s\n", debugstr_w(progid)); + free(visited_progids[call_depth-1]); + visited_progids[call_depth-1] = NULL; return CO_E_CLASSSTRING; } + RegCloseKey(xhkey); + free(visited_progids[call_depth-1]); + visited_progids[call_depth-1] = NULL; return guid_from_string(buf2, clsid) ? S_OK : CO_E_CLASSSTRING; } +static HRESULT clsid_from_string_reg(LPCOLESTR progid, CLSID *clsid) +{ + WCHAR *visited_progids[11] = {0}; + return clsid_from_string_reg_recursive(progid, clsid, visited_progids, 0); +} + /****************************************************************************** * CLSIDFromProgID (combase.@) */ HRESULT WINAPI DECLSPEC_HOTPATCH CLSIDFromProgID(LPCOLESTR progid, CLSID *clsid) { ACTCTX_SECTION_KEYED_DATA data; + HRESULT hr; if (!progid || !clsid) return E_INVALIDARG; @@ -1492,7 +1573,8 @@ HRESULT WINAPI DECLSPEC_HOTPATCH CLSIDFromProgID(LPCOLESTR progid, CLSID *clsid) return S_OK; } - return clsid_from_string_reg(progid, clsid); + hr = clsid_from_string_reg(progid, clsid); + return hr == CLSID_FROM_STRING_RECURSION ? CO_E_CLASSSTRING : hr; } /****************************************************************************** @@ -1523,8 +1605,7 @@ HRESULT WINAPI CLSIDFromString(LPCOLESTR str, LPCLSID clsid) hr = clsid_from_string_reg(str, &tmp_id); if (SUCCEEDED(hr)) *clsid = tmp_id; - - return hr; + return hr == CLSID_FROM_STRING_RECURSION ? REGDB_E_INVALIDVALUE : hr; } /****************************************************************************** -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9639
From: Ivan Lyugaev <valy(a)etersoft.ru> --- dlls/ole32/tests/compobj.c | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/dlls/ole32/tests/compobj.c b/dlls/ole32/tests/compobj.c index 2c4cc566f3a..a603e22f820 100644 --- a/dlls/ole32/tests/compobj.c +++ b/dlls/ole32/tests/compobj.c @@ -23,7 +23,6 @@ #include <stdarg.h> #include <stdio.h> - #include "windef.h" #include "winbase.h" #define USE_COM_CONTEXT_DEF @@ -526,15 +525,11 @@ static void test_CLSIDFromProgID(void) ok(!ret, "Failed to create a test key.\n"); hr = CLSIDFromProgID(L"MyApp.DocumentTest", &clsid); - todo_wine ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); - todo_wine ok(IsEqualCLSID(&clsid, &CLSID_non_existent), "Unexpected clsid %s.\n", wine_dbgstr_guid(&clsid)); hr = CLSIDFromProgID(L"MyApp.DocumentTest.1", &clsid); - todo_wine ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); - todo_wine ok(IsEqualCLSID(&clsid, &CLSID_non_existent), "Unexpected clsid %s.\n", wine_dbgstr_guid(&clsid)); hr = CLSIDFromProgID(L"MyApp.DocumentTest.2", &clsid); @@ -552,15 +547,11 @@ static void test_CLSIDFromProgID(void) ok(IsEqualCLSID(&clsid, &CLSID_NULL), "Unexpected clsid %s.\n", wine_dbgstr_guid(&clsid)); hr = CLSIDFromString(L"MyApp.DocumentTest", &clsid); - todo_wine ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); - todo_wine ok(IsEqualCLSID(&clsid, &CLSID_non_existent), "Unexpected clsid %s.\n", wine_dbgstr_guid(&clsid)); hr = CLSIDFromString(L"MyApp.DocumentTest.1", &clsid); - todo_wine ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); - todo_wine ok(IsEqualCLSID(&clsid, &CLSID_non_existent), "Unexpected clsid %s.\n", wine_dbgstr_guid(&clsid)); hr = CLSIDFromString(L"MyApp.DocumentTest.2", &clsid); @@ -569,7 +560,6 @@ static void test_CLSIDFromProgID(void) clsid = CLSID_StdFont; hr = CLSIDFromString(L"MyApp.DocumentTest.3", &clsid); - todo_wine ok(hr == REGDB_E_INVALIDVALUE, "Unexpected hr %#lx.\n", hr); todo_wine ok(IsEqualCLSID(&clsid, &CLSID_StdFont), "Unexpected clsid %s.\n", wine_dbgstr_guid(&clsid)); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9639
From: Ivan Lyugaev <valy(a)etersoft.ru> --- dlls/oleaut32/typelib.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/oleaut32/typelib.c b/dlls/oleaut32/typelib.c index c4f00ca5bea..d5129a2c5f9 100644 --- a/dlls/oleaut32/typelib.c +++ b/dlls/oleaut32/typelib.c @@ -7326,7 +7326,7 @@ static HRESULT WINAPI ITypeInfo_fnInvoke( } else if ((rgvt[i] & VT_BYREF) && !V_ISBYREF(src_arg)) { - if (wParamFlags & PARAMFLAG_FIN) + if ((wParamFlags & PARAMFLAG_FIN) || (wParamFlags == PARAMFLAG_NONE)) hres = VariantChangeType(&missing_arg[i], src_arg, 0, rgvt[i] & ~VT_BYREF); else V_VT(&missing_arg[i]) = rgvt[i] & ~VT_BYREF; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9639
From: Ivan Lyugaev <valy(a)etersoft.ru> --- dlls/oleaut32/tests/test_reg.idl | 6 ++++ dlls/oleaut32/tests/typelib.c | 50 ++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/dlls/oleaut32/tests/test_reg.idl b/dlls/oleaut32/tests/test_reg.idl index 2ab7638f2fc..4b6c11d81bf 100644 --- a/dlls/oleaut32/tests/test_reg.idl +++ b/dlls/oleaut32/tests/test_reg.idl @@ -171,6 +171,12 @@ library register_test void testget8([out, retval] int* value); [propput, id(11)] void testput8([in] int value); + [id(12)] + HRESULT testbstrnone(BSTR* value, [out, retval] BSTR* retvalue); + [id(13)] + HRESULT testbstrin([in] BSTR* value, [out, retval] BSTR* retvalue); + [id(14)] + HRESULT testbstrout([out] BSTR* value, [out, retval] BSTR* retvalue); } /* uuid is same as for test_struct2 in test_tlb.idl, fields are different */ diff --git a/dlls/oleaut32/tests/typelib.c b/dlls/oleaut32/tests/typelib.c index 153964e17d5..d478d666e95 100644 --- a/dlls/oleaut32/tests/typelib.c +++ b/dlls/oleaut32/tests/typelib.c @@ -316,6 +316,33 @@ static void WINAPI invoketest_get_testput8(IInvokeTest *iface, int value) { } +static HRESULT WINAPI invoketest_test_bstr_none(IInvokeTest *iface, BSTR* value, BSTR* retvalue) +{ + if (value != NULL && *value != NULL) + { + *retvalue = SysAllocString(*value); + } + return S_OK; +} + +static HRESULT WINAPI invoketest_test_bstr_in(IInvokeTest *iface, BSTR* value, BSTR* retvalue) +{ + if (value != NULL && *value != NULL) + { + *retvalue = SysAllocString(*value); + } + return S_OK; +} + +static HRESULT WINAPI invoketest_test_bstr_out(IInvokeTest *iface, BSTR* value, BSTR* retvalue) +{ + if (value != NULL && *value != NULL) + { + *retvalue = SysAllocString(*value); + } + return S_OK; +} + static const IInvokeTestVtbl invoketestvtbl = { invoketest_QueryInterface, invoketest_AddRef, @@ -337,6 +364,9 @@ static const IInvokeTestVtbl invoketestvtbl = { invoketest_get_testget7, invoketest_get_testget8, invoketest_get_testput8, + invoketest_test_bstr_none, + invoketest_test_bstr_in, + invoketest_test_bstr_out }; static IInvokeTest invoketest = { &invoketestvtbl }; @@ -868,6 +898,26 @@ static void test_invoke_func(ITypeInfo *typeinfo) dp.cArgs = 2; hres = ITypeInfo_Invoke(typeinfo, &invoketest, 3, DISPATCH_METHOD, &dp, &res, NULL, &i); ok(hres == DISP_E_BADPARAMCOUNT, "got 0x%08lx\n", hres); + + V_VT(args) = VT_BSTR; + V_BSTR(args) = SysAllocString(L"TestString"); + dp.cArgs = 1; + dp.cNamedArgs = 0; + + hres = ITypeInfo_Invoke(typeinfo, &invoketest, 12, DISPATCH_METHOD, &dp, &res, NULL, &i); + ok(hres == S_OK, "got 0x%08lx\n", hres); + ok(V_VT(&res) == VT_BSTR, "got %d\n", V_VT(&res)); + ok(!lstrcmpW(V_BSTR(&res), L"TestString"), "got %ls\n", !lstrcmpW(V_BSTR(&res), L"") ? L"(null)" : V_BSTR(&res)); + + hres = ITypeInfo_Invoke(typeinfo, &invoketest, 13, DISPATCH_METHOD, &dp, &res, NULL, &i); + ok(hres == S_OK, "got 0x%08lx\n", hres); + ok(V_VT(&res) == VT_BSTR, "got %d\n", V_VT(&res)); + ok(!lstrcmpW(V_BSTR(&res), L"TestString"), "got %ls\n", V_BSTR(&res)); + + hres = ITypeInfo_Invoke(typeinfo, &invoketest, 14, DISPATCH_METHOD, &dp, &res, NULL, &i); + ok(hres == S_OK, "got 0x%08lx\n", hres); + ok(V_VT(&res) == VT_BSTR, "got %d\n", V_VT(&res)); + ok(!lstrcmpW(V_BSTR(&res), L""), "got %ls\n", V_BSTR(&res)); } static WCHAR *create_test_typelib(int res_no) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9639
Regarding CurVer change, we have an older !7539. Both of them need some work and cleanup in general. Does Office use chaining for CurVer keys? If not, it seems to me that implementing just one level could turn out better. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9639#note_124309
participants (3)
-
Ivan Lyugaev -
Ivan Lyugaev (＠valy)
-
Nikolay Sivov (＠nsivov)