[PATCH 2/5] activeds: Implement ADsOpenObject.
Signed-off-by: Dmitry Timoshkov <dmitry(a)baikal.ru> --- dlls/activeds/Makefile.in | 1 + dlls/activeds/activeds_main.c | 68 ++++++++++++++++++++++++++++++++--- include/iads.idl | 14 ++++++++ 3 files changed, 79 insertions(+), 4 deletions(-) diff --git a/dlls/activeds/Makefile.in b/dlls/activeds/Makefile.in index 20578a93bc..076a2b5693 100644 --- a/dlls/activeds/Makefile.in +++ b/dlls/activeds/Makefile.in @@ -1,5 +1,6 @@ MODULE = activeds.dll IMPORTLIB = activeds +IMPORTS = advapi32 ole32 EXTRADLLFLAGS = -mno-cygwin diff --git a/dlls/activeds/activeds_main.c b/dlls/activeds/activeds_main.c index 69c5c447dc..a280f50c2f 100644 --- a/dlls/activeds/activeds_main.c +++ b/dlls/activeds/activeds_main.c @@ -30,8 +30,10 @@ #include "winuser.h" #include "objbase.h" +#include "initguid.h" #include "iads.h" #include "adshlp.h" +#include "adserr.h" #include "wine/debug.h" @@ -112,11 +114,69 @@ HRESULT WINAPI ADsBuildVarArrayInt(LPDWORD lpdwObjectTypes, DWORD dwObjectTypes, /***************************************************** * ADsOpenObject [ACTIVEDS.9] */ -HRESULT WINAPI ADsOpenObject(LPCWSTR lpszPathName, LPCWSTR lpszUserName, LPCWSTR lpszPassword, DWORD dwReserved, REFIID riid, VOID** ppObject) +HRESULT WINAPI ADsOpenObject(LPCWSTR path, LPCWSTR user, LPCWSTR password, DWORD reserved, REFIID riid, void **obj) { - FIXME("(%s,%s,%u,%s,%p)!stub\n", debugstr_w(lpszPathName), - debugstr_w(lpszUserName), dwReserved, debugstr_guid(riid), ppObject); - return E_NOTIMPL; + HRESULT hr; + HKEY hkey, hprov; + WCHAR provider[MAX_PATH], progid[MAX_PATH]; + DWORD idx = 0; + + TRACE("(%s,%s,%u,%s,%p)\n", debugstr_w(path), debugstr_w(user), reserved, debugstr_guid(riid), obj); + + if (!path || !riid || !obj) + return E_INVALIDARG; + + if (RegOpenKeyExW(HKEY_LOCAL_MACHINE, L"Software\\Microsoft\\ADs\\Providers", 0, KEY_READ, &hkey)) + return E_ADS_BAD_PATHNAME; + + hr = E_ADS_BAD_PATHNAME; + + for (;;) + { + if (RegEnumKeyW(hkey, idx++, provider, ARRAY_SIZE(provider))) + break; + + TRACE("provider %s\n", debugstr_w(provider)); + + if (!wcsnicmp(path, provider, wcslen(provider)) && path[wcslen(provider)] == ':') + { + LONG size; + + if (RegOpenKeyExW(hkey, provider, 0, KEY_READ, &hprov)) + break; + + size = ARRAY_SIZE(progid); + if (!RegQueryValueW(hprov, NULL, progid, &size)) + { + CLSID clsid; + + if (CLSIDFromProgID(progid, &clsid) == S_OK) + { + IADsOpenDSObject *adsopen; + IDispatch *disp; + + TRACE("ns %s => clsid %s\n", debugstr_w(progid), wine_dbgstr_guid(&clsid)); + if (CoCreateInstance(&clsid, 0, CLSCTX_INPROC_SERVER, &IID_IADsOpenDSObject, (void **)&adsopen) == S_OK) + { + hr = IADsOpenDSObject_OpenDSObject(adsopen, (BSTR)path, (BSTR)user, (BSTR)password, reserved, &disp); + if (hr == S_OK) + { + hr = IDispatch_QueryInterface(disp, riid, obj); + IDispatch_Release(disp); + } + + IADsOpenDSObject_Release(adsopen); + } + } + } + + RegCloseKey(hprov); + } + } + + RegCloseKey(hkey); + + return hr; } /***************************************************** diff --git a/include/iads.idl b/include/iads.idl index 6931b6f734..add52571c7 100644 --- a/include/iads.idl +++ b/include/iads.idl @@ -794,6 +794,20 @@ interface IDirectorySearch : IUnknown HRESULT CloseSearchHandle([in] ADS_SEARCH_HANDLE hSearchResult); } +/***************************************************************************** + * IID_IADsOpenDSObject interface + */ +[ + odl, + uuid(ddf2891e-0f9c-11d0-8ad4-00c04fd8d503), + dual, + oleautomation +] +interface IADsOpenDSObject : IDispatch +{ + HRESULT OpenDSObject([in] BSTR path, [in] BSTR user, [in] BSTR password, [in] long reserved, [out,retval] IDispatch **obj); +} + /***************************************************************************** * IADsADSystemInfo interface */ -- 2.20.1
Hi Dmitry, On 16/12/19 12:41 pm, Dmitry Timoshkov wrote:
Signed-off-by: Dmitry Timoshkov <dmitry(a)baikal.ru> + if (CoCreateInstance(&clsid, 0, CLSCTX_INPROC_SERVER, &IID_IADsOpenDSObject, (void **)&adsopen) == S_OK) + { + hr = IADsOpenDSObject_OpenDSObject(adsopen, (BSTR)path, (BSTR)user, (BSTR)password, reserved, &disp); + if (hr == S_OK) You cannot cast to a BSTR, it will corrupt the data structure.
+ { + hr = IDispatch_QueryInterface(disp, riid, obj); + IDispatch_Release(disp); Now that you have found the object, shouldn't you exit the loop?
Regards Alistair.
Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> wrote:
+ if (CoCreateInstance(&clsid, 0, CLSCTX_INPROC_SERVER, &IID_IADsOpenDSObject, (void **)&adsopen) == S_OK) + { + hr = IADsOpenDSObject_OpenDSObject(adsopen, (BSTR)path, (BSTR)user, (BSTR)password, reserved, &disp); + if (hr == S_OK) You cannot cast to a BSTR, it will corrupt the data structure.
I depends wheather the callee uses SysStringLen() and friends, in this particular case it works just fine, there are many other places in the code when it's not necessary to create intermediate copies of the strings.
+ { + hr = IDispatch_QueryInterface(disp, riid, obj); + IDispatch_Release(disp); Now that you have found the object, shouldn't you exit the loop?
Yes, thanks for spotting this. I think it could be fixed in a later patch. -- Dmitry.
On Mon, Dec 16, 2019 at 11:16 AM Dmitry Timoshkov <dmitry(a)baikal.ru> wrote:
Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> wrote:
+ if (CoCreateInstance(&clsid, 0, CLSCTX_INPROC_SERVER, &IID_IADsOpenDSObject, (void **)&adsopen) == S_OK) + { + hr = IADsOpenDSObject_OpenDSObject(adsopen, (BSTR)path, (BSTR)user, (BSTR)password, reserved, &disp); + if (hr == S_OK) You cannot cast to a BSTR, it will corrupt the data structure.
I depends wheather the callee uses SysStringLen() and friends, in this particular case it works just fine, there are many other places in the code when it's not necessary to create intermediate copies of the strings.
It does not matter if it works in this case, arguments should use correct types. If we have similar violations somewhere else, those need fixing as well.
+ { + hr = IDispatch_QueryInterface(disp, riid, obj); + IDispatch_Release(disp); Now that you have found the object, shouldn't you exit the loop?
Yes, thanks for spotting this. I think it could be fixed in a later patch.
-- Dmitry.
Nikolay Sivov <bunglehead(a)gmail.com> wrote:
+ hr = IADsOpenDSObject_OpenDSObject(adsopen, (BSTR)path, (BSTR)user, (BSTR)password, reserved, &disp); + if (hr == S_OK) You cannot cast to a BSTR, it will corrupt the data structure.
I depends wheather the callee uses SysStringLen() and friends, in this particular case it works just fine, there are many other places in the code when it's not necessary to create intermediate copies of the strings.
It does not matter if it works in this case, arguments should use correct types. If we have similar violations somewhere else, those need fixing as well.
That's a pretty incorrect assertion, there's nothing wrong in casting BSTR to LPWSTR and vice versa. -- Dmitry.
On Mon, Dec 16, 2019 at 11:52 AM Dmitry Timoshkov <dmitry(a)baikal.ru> wrote:
Nikolay Sivov <bunglehead(a)gmail.com> wrote:
+ hr = IADsOpenDSObject_OpenDSObject(adsopen, (BSTR)path, (BSTR)user, (BSTR)password, reserved, &disp); + if (hr == S_OK) You cannot cast to a BSTR, it will corrupt the data structure.
I depends wheather the callee uses SysStringLen() and friends, in this particular case it works just fine, there are many other places in the code when it's not necessary to create intermediate copies of the strings.
It does not matter if it works in this case, arguments should use correct types. If we have similar violations somewhere else, those need fixing as well.
That's a pretty incorrect assertion, there's nothing wrong in casting BSTR to LPWSTR and vice versa.
What's incorrect about it? You can use BSTR as WCHAR * argument, obviously. Not the other way around, even if it happens to work.
-- Dmitry.
Nikolay Sivov <bunglehead(a)gmail.com> wrote:
+ hr = IADsOpenDSObject_OpenDSObject(adsopen, (BSTR)path, (BSTR)user, (BSTR)password, reserved, &disp); + if (hr == S_OK) You cannot cast to a BSTR, it will corrupt the data structure.
I depends wheather the callee uses SysStringLen() and friends, in this particular case it works just fine, there are many other places in the code when it's not necessary to create intermediate copies of the strings.
It does not matter if it works in this case, arguments should use correct types. If we have similar violations somewhere else, those need fixing as well.
That's a pretty incorrect assertion, there's nothing wrong in casting BSTR to LPWSTR and vice versa.
What's incorrect about it? You can use BSTR as WCHAR * argument, obviously. Not the other way around, even if it happens to work.
You are contradicting yourself: if it works that means that's correct behaviour. If you are going to further argue, please show the code that won't work for this particular implementation. -- Dmitry.
On 12/16/19 3:21 AM, Dmitry Timoshkov wrote:
Nikolay Sivov <bunglehead(a)gmail.com> wrote:
> + hr = IADsOpenDSObject_OpenDSObject(adsopen, (BSTR)path, (BSTR)user, (BSTR)password, reserved, &disp); > + if (hr == S_OK) You cannot cast to a BSTR, it will corrupt the data structure.
I depends wheather the callee uses SysStringLen() and friends, in this particular case it works just fine, there are many other places in the code when it's not necessary to create intermediate copies of the strings.
It does not matter if it works in this case, arguments should use correct types. If we have similar violations somewhere else, those need fixing as well.
That's a pretty incorrect assertion, there's nothing wrong in casting BSTR to LPWSTR and vice versa.
What's incorrect about it? You can use BSTR as WCHAR * argument, obviously. Not the other way around, even if it happens to work.
You are contradicting yourself: if it works that means that's correct behaviour. If you are going to further argue, please show the code that won't work for this particular implementation.
I think it's just a matter of following correct and documented behaviour on principle. You wouldn't intentionally use an object after calling Release() just because it happens to work (relying on the fact that the object is static, or that the heap memory just doesn't get overwritten). Native might already have used something like SysStringLen() internally and have corrupted data structures that simply aren't causing any problems yet with this test. It also might not, but it costs very little to just do the documented thing.
participants (4)
-
Alistair Leslie-Hughes -
Dmitry Timoshkov -
Nikolay Sivov -
Zebediah Figura