On 06/03/2010 03:47 PM, Andrew Eikum wrote:
On 06/03/2010 03:12 PM, Nikolay Sivov wrote:
No, this is not what I meant actually. The possible reason it was named
as IUnknown_EnumObjects is the same as for the rest of similar calls.
IUknown is a valid input for all of them and another _QueryInterface is
performed for IID_IShellFolder is case of this function. That makes no
sense to have such export as your patch makes it, but it's possible of
course. I expect that is does check that IShellFolder is supported.

Okay, I see what you're saying now. I hadn't thought of that. After some
quick tests, I think you're on the right path. I'll work on fixing it.
Thanks!


So, with this in mind, here are some much more extensive tests. As is explained in the comment, Windows _does_ get the ClassID of the object in this call. But, Windows crashes when a non-IShellFolder object is given to this function. Presumably it's trying to call IShellFolder_EnumObjects on it. So it must not be using the CLSID for this kind of type-checking.

Trouble is, I have no idea what it is used for. I tried testing most of the CLSIDs in shlguid.h with different combinations of flags, none of which had any effect. It must just be looking out for a small number of specific CLSIDs. One possible guess is that it tweaks the flags parameter when certain CLSIDs are detected, but none of my tests turned up anything interesting.

Any thoughts?  Should I just send as-is?

Andrew
>From eb76e2bef28be5f5079979d5db91271606ef0c7f Mon Sep 17 00:00:00 2001
From: Andrew Eikum <[email protected]>
Date: Thu, 3 Jun 2010 08:50:59 -0500
Subject: shlwapi: Be less strict on which type of IShellFolder can be enumerated
To: wine-patches <[email protected]>
Reply-To: wine-devel <[email protected]>

This fixes a bug with Internet Explorer 6's Favorites menu.
---
 dlls/shlwapi/ordinal.c       |   20 ++-
 dlls/shlwapi/tests/ordinal.c |  293 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 308 insertions(+), 5 deletions(-)

diff --git a/dlls/shlwapi/ordinal.c b/dlls/shlwapi/ordinal.c
index 18295e4..becc7f2 100644
--- a/dlls/shlwapi/ordinal.c
+++ b/dlls/shlwapi/ordinal.c
@@ -3697,21 +3697,31 @@ HRESULT WINAPI SHIShellFolder_EnumObjects(LPSHELLFOLDER 
lpFolder, HWND hwnd, SHC
     IPersist *persist;
     HRESULT hr;
 
+    /* Windows attempts to get an IPersist interface and, if that fails, an
+     * IPersistFolder interface on the folder passed-in here.  If one of those
+     * interfaces is available, it then calls GetClassID on the folder... and 
+     * then calls IShellFolder_EnumObjects no matter what, even crashing if
+     * lpFolder isn't actually an IShellFolder object.  The purpose of getting
+     * the ClassID is unknown.
+     *
+     * See tests for examples.
+     */
     hr = IShellFolder_QueryInterface(lpFolder, &IID_IPersist, 
(LPVOID)&persist);
+    if(FAILED(hr))
+        hr = IShellFolder_QueryInterface(lpFolder, &IID_IPersistFolder, 
(LPVOID)&persist);
+
     if(SUCCEEDED(hr))
     {
         CLSID clsid;
         hr = IPersist_GetClassID(persist, &clsid);
         if(SUCCEEDED(hr))
         {
-            if(IsEqualCLSID(&clsid, &CLSID_ShellFSFolder))
-                hr = IShellFolder_EnumObjects(lpFolder, hwnd, flags, ppenum);
-            else
-                hr = E_FAIL;
+            /* ??? */
         }
         IPersist_Release(persist);
     }
-    return hr;
+
+    return IShellFolder_EnumObjects(lpFolder, hwnd, flags, ppenum);
 }
 
 /* INTERNAL: Map from HLS color space to RGB */
diff --git a/dlls/shlwapi/tests/ordinal.c b/dlls/shlwapi/tests/ordinal.c
index cfc25e1..782fef8 100644
--- a/dlls/shlwapi/tests/ordinal.c
+++ b/dlls/shlwapi/tests/ordinal.c
@@ -56,6 +56,7 @@ static BOOL   (WINAPI *pGUIDFromStringA)(LPSTR, CLSID *);
 static HRESULT (WINAPI *pIUnknown_QueryServiceExec)(IUnknown*, REFIID, const 
GUID*, DWORD, DWORD, VARIANT*, VARIANT*);
 static HRESULT (WINAPI *pIUnknown_ProfferService)(IUnknown*, REFGUID, 
IServiceProvider*, DWORD*);
 static HWND    (WINAPI *pSHCreateWorkerWindowA)(LONG, HWND, DWORD, DWORD, 
HMENU, LONG_PTR);
+static HRESULT (WINAPI *pSHIShellFolder_EnumObjects)(LPSHELLFOLDER, HWND, 
SHCONTF, IEnumIDList**);
 
 static HMODULE hmlang;
 static HRESULT (WINAPI *pLcidToRfc1766A)(LCID, LPSTR, INT);
@@ -2369,6 +2370,296 @@ static void test_SHCreateWorkerWindowA(void)
     DestroyWindow(hwnd);
 }
 
+typedef struct _p_ShellFolder {
+    const IShellFolderVtbl *sfVtbl;
+    const IPersistVtbl *pVtbl;
+
+    INT QueryInterface_calls;
+    INT EnumObjects_calls;
+    INT GetClassID_calls;
+
+    REFIID *exp_IIDs;
+
+    BOOL QueryInterface_fail;
+    BOOL GetClassID_fail;
+} p_ShellFolder;
+#define PERSIST_TO_SF(x) ((p_ShellFolder*)(((BYTE*)x) - sizeof(IPersistVtbl*)))
+
+static HRESULT WINAPI SF_QueryInterface(IShellFolder *iface,
+        REFIID riid, void **ppv)
+{
+    p_ShellFolder *This = (p_ShellFolder*)iface;
+    const GUID *id = riid;
+
+    ok(This->QueryInterface_calls > 0 && IsEqualGUID(*This->exp_IIDs, riid),
+            "Unexpected QI: 
{%08x-%04x-%04x-%02x%02x-%02x%02x%02x%02x%02x%02x}\n",
+            id->Data1, id->Data2, id->Data3,
+            id->Data4[0], id->Data4[1], id->Data4[2], id->Data4[3],
+            id->Data4[4], id->Data4[5], id->Data4[6], id->Data4[7]);
+    if(This->QueryInterface_calls > 0)
+        This->QueryInterface_calls--;
+    This->exp_IIDs++;
+
+    if(This->QueryInterface_fail == TRUE)
+        return E_NOINTERFACE;
+
+    if(IsEqualGUID(&IID_IShellFolder, riid)){
+        *ppv = iface;
+        return S_OK;
+    }
+    if(IsEqualGUID(&IID_IPersist, riid)){
+        *ppv = &(This->pVtbl);
+        return S_OK;
+    }
+
+    return E_NOINTERFACE;
+}
+
+static ULONG WINAPI SF_AddRef(IShellFolder *iface)
+{
+    return 2;
+}
+
+static ULONG WINAPI SF_Release(IShellFolder *iface)
+{
+    return 1;
+}
+
+static HRESULT WINAPI SF_ParseDisplayName(IShellFolder *iface,
+        HWND owner, LPBC reserved, LPOLESTR displayName, ULONG *eaten,
+        LPITEMIDLIST *idl, ULONG *attr)
+{
+    ok(0, "Didn't expect ParseDisplayName\n");
+    return E_NOTIMPL;
+}
+
+static HRESULT WINAPI SF_EnumObjects(IShellFolder *iface,
+        HWND owner, SHCONTF flags, IEnumIDList **enm)
+{
+    p_ShellFolder *This = (p_ShellFolder*)iface;
+
+    ok(This->EnumObjects_calls > 0, "Too many EnumObjects calls\n");
+    if(This->EnumObjects_calls > 0)
+        This->EnumObjects_calls--;
+
+    ok(!owner && !flags, "Didn't expect owner (%p) or flags (%x)\n", owner, 
flags);
+
+    /* use null enm to check return val */
+    if(!enm)
+        return E_ACCESSDENIED;
+
+    *enm = (IEnumIDList*)0xcafebabe;
+    return S_OK;
+}
+
+static HRESULT WINAPI SF_BindToObject(IShellFolder *iface,
+        LPCITEMIDLIST idl, LPBC reserved, REFIID riid, void **obj)
+{
+    ok(0, "Didn't expect BindToObject\n");
+    return E_NOTIMPL;
+}
+
+static HRESULT WINAPI SF_BindToStorage(IShellFolder *iface,
+        LPCITEMIDLIST idl, LPBC reserved, REFIID riid, void **obj)
+{
+    ok(0, "Didn't expect BindToStorage\n");
+    return E_NOTIMPL;
+}
+
+static HRESULT WINAPI SF_CompareIDs(IShellFolder *iface,
+        LPARAM lparam, LPCITEMIDLIST idl1, LPCITEMIDLIST idl2)
+{
+    ok(0, "Didn't expect CompareIDs\n");
+    return E_NOTIMPL;
+}
+
+static HRESULT WINAPI SF_CreateViewObject(IShellFolder *iface,
+        HWND owner, REFIID riid, void **out)
+{
+    ok(0, "Didn't expect CreateViewObject\n");
+    return E_NOTIMPL;
+}
+
+static HRESULT WINAPI SF_GetAttributesOf(IShellFolder *iface,
+        UINT cidl, LPCITEMIDLIST *idl, SFGAOF *inOut)
+{
+    ok(0, "Didn't expect GetAttributesOf\n");
+    return E_NOTIMPL;
+}
+
+static HRESULT WINAPI SF_GetUIObjectOf(IShellFolder *iface,
+        HWND owner, UINT cidl, LPCITEMIDLIST *idls, REFIID riid, UINT *inOut,
+        void **out)
+{
+    ok(0, "Didn't expect GetUIObjectOf\n");
+    return E_NOTIMPL;
+}
+
+static HRESULT WINAPI SF_GetDisplayNameOf(IShellFolder *iface,
+        LPCITEMIDLIST idl, SHGDNF flags, STRRET *name)
+{
+    ok(0, "Didn't expect GetDisplayNameOf\n");
+    return E_NOTIMPL;
+}
+
+static HRESULT WINAPI SF_SetNameOf(IShellFolder *iface,
+        HWND hwnd, LPCITEMIDLIST idl, LPCOLESTR name, SHGDNF flags,
+        LPITEMIDLIST *idlOut)
+{
+    ok(0, "Didn't expect SetNameOf\n");
+    return E_NOTIMPL;
+}
+
+static IShellFolderVtbl ShellFolderVtbl = {
+    SF_QueryInterface,
+    SF_AddRef,
+    SF_Release,
+    SF_ParseDisplayName,
+    SF_EnumObjects,
+    SF_BindToObject,
+    SF_BindToStorage,
+    SF_CompareIDs,
+    SF_CreateViewObject,
+    SF_GetAttributesOf,
+    SF_GetUIObjectOf,
+    SF_GetDisplayNameOf,
+    SF_SetNameOf
+};
+
+static HRESULT WINAPI P_QueryInterface(IPersist *iface,
+        REFIID riid, void **ppv)
+{
+    const GUID *id = riid;
+
+    ok(0, "Unexpected QI: 
{%08x-%04x-%04x-%02x%02x-%02x%02x%02x%02x%02x%02x}\n",
+        id->Data1, id->Data2, id->Data3,
+        id->Data4[0], id->Data4[1], id->Data4[2], id->Data4[3],
+        id->Data4[4], id->Data4[5], id->Data4[6], id->Data4[7]);
+
+    if(IsEqualGUID(&IID_IPersist, riid)){
+        *ppv = iface;
+        return S_OK;
+    }
+
+    return E_NOINTERFACE;
+}
+
+static ULONG WINAPI P_AddRef(IPersist *iface)
+{
+    return 2;
+}
+
+static ULONG WINAPI P_Release(IPersist *iface)
+{
+    return 1;
+}
+
+static HRESULT WINAPI P_GetClassID(IPersist *iface,
+        CLSID *clsid)
+{
+    p_ShellFolder *This = PERSIST_TO_SF(iface);
+    static const GUID junk = {1, 2, 3, {4, 5, 6, 7, 8, 9, 10} };
+
+    ok(This->GetClassID_calls > 0, "Too many GetClassID calls\n");
+    if(This->GetClassID_calls > 0)
+        This->GetClassID_calls--;
+
+    if(This->GetClassID_fail == TRUE)
+        return E_FAIL;
+
+    *clsid = junk;
+
+    return S_OK;
+}
+
+static IPersistVtbl PersistVtbl = {
+    P_QueryInterface,
+    P_AddRef,
+    P_Release,
+    P_GetClassID
+};
+
+static p_ShellFolder ShellFolder = { &ShellFolderVtbl, &PersistVtbl };
+
+#define verify_sf_calls(f) r_verify_sf_calls(__LINE__, f)
+static void r_verify_sf_calls(unsigned l, p_ShellFolder *sf)
+{
+    ok_(__FILE__, l)(sf->QueryInterface_calls == 0,
+            "QueryInterface wasn't called enough times: %d\n",
+            sf->QueryInterface_calls);
+
+    ok_(__FILE__, l)(sf->EnumObjects_calls == 0,
+            "EnumObjects wasn't called enough times: %d\n",
+            sf->EnumObjects_calls);
+
+    ok_(__FILE__, l)(sf->GetClassID_calls == 0,
+            "GetClassID wasn't called enough times: %d\n",
+            sf->GetClassID_calls);
+}
+
+static void test_SHIShellFolder_EnumObjects(void)
+{
+    IEnumIDList *enm;
+    HRESULT hres;
+    REFIID exp_IIDs[] = { &IID_IPersist, &IID_IPersistFolder };
+
+    if(!pSHIShellFolder_EnumObjects){
+        win_skip("SHIShellFolder_EnumObjects not available\n");
+        return;
+    }
+
+    if(0){
+        /* NULL object crashes on Windows */
+        hres = pSHIShellFolder_EnumObjects(NULL, NULL, 0, NULL);
+    }
+
+    /* verify a "normal" call */
+    ShellFolder.QueryInterface_calls = 1;
+    ShellFolder.GetClassID_calls = 1;
+    ShellFolder.EnumObjects_calls = 1;
+    ShellFolder.exp_IIDs = exp_IIDs;
+    enm = (IEnumIDList*)0xdeadbeef;
+    hres = pSHIShellFolder_EnumObjects((IShellFolder*)&ShellFolder, NULL, 0, 
&enm);
+    ok(hres == S_OK, "Didn't expect fail: 0x%08x\n", hres);
+    ok(enm == (IEnumIDList*)0xcafebabe, "Didn't get expected enumerator 
location, instead: %p\n", enm);
+    verify_sf_calls(&ShellFolder);
+
+    /* verify that NULL arguments go through to object */
+    ShellFolder.QueryInterface_calls = 1;
+    ShellFolder.GetClassID_calls = 1;
+    ShellFolder.EnumObjects_calls = 1;
+    ShellFolder.exp_IIDs = exp_IIDs;
+    hres = pSHIShellFolder_EnumObjects((IShellFolder*)&ShellFolder, NULL, 0, 
NULL);
+    ok(hres == E_ACCESSDENIED, "Expected other return code: 0x%08x\n", hres);
+    verify_sf_calls(&ShellFolder);
+
+    /* verify that object's EnumObjects method is still called when QI fails */
+    ShellFolder.QueryInterface_calls = 2;
+    ShellFolder.GetClassID_calls = 0;
+    ShellFolder.EnumObjects_calls = 1;
+    ShellFolder.exp_IIDs = exp_IIDs;
+    ShellFolder.QueryInterface_fail = TRUE;
+    ShellFolder.GetClassID_fail = FALSE;
+    enm = (IEnumIDList*)0xdeadbeef;
+    hres = pSHIShellFolder_EnumObjects((IShellFolder*)&ShellFolder, NULL, 0, 
&enm);
+    ok(hres == S_OK, "Didn't expect fail: 0x%08x\n", hres);
+    ok(enm == (IEnumIDList*)0xcafebabe, "Didn't get expected enumerator 
location, instead: %p\n", enm);
+    verify_sf_calls(&ShellFolder);
+
+    /* verify that object's EnumObjects method is still called when GetClassID 
fails */
+    ShellFolder.QueryInterface_calls = 1;
+    ShellFolder.GetClassID_calls = 1;
+    ShellFolder.EnumObjects_calls = 1;
+    ShellFolder.exp_IIDs = exp_IIDs;
+    ShellFolder.QueryInterface_fail = FALSE;
+    ShellFolder.GetClassID_fail = TRUE;
+    enm = (IEnumIDList*)0xdeadbeef;
+    hres = pSHIShellFolder_EnumObjects((IShellFolder*)&ShellFolder, NULL, 0, 
&enm);
+    ok(hres == S_OK, "Didn't expect fail: 0x%08x\n", hres);
+    ok(enm == (IEnumIDList*)0xcafebabe, "Didn't get expected enumerator 
location, instead: %p\n", enm);
+    verify_sf_calls(&ShellFolder);
+}
+
 static void init_pointers(void)
 {
 #define MAKEFUNC(f, ord) (p##f = (void*)GetProcAddress(hShlwapi, (LPSTR)(ord)))
@@ -2387,6 +2678,7 @@ static void init_pointers(void)
     MAKEFUNC(IConnectionPoint_SimpleInvoke, 284);
     MAKEFUNC(SHFormatDateTimeA, 353);
     MAKEFUNC(SHFormatDateTimeW, 354);
+    MAKEFUNC(SHIShellFolder_EnumObjects, 404);
     MAKEFUNC(SHGetObjectCompatFlags, 476);
     MAKEFUNC(IUnknown_QueryServiceExec, 484);
     MAKEFUNC(SHPropertyBag_ReadLONG, 496);
@@ -2419,4 +2711,5 @@ START_TEST(ordinal)
     test_IUnknown_QueryServiceExec();
     test_IUnknown_ProfferService();
     test_SHCreateWorkerWindowA();
+    test_SHIShellFolder_EnumObjects();
 }
-- 
1.7.1



Reply via email to