[PATCH 0/2] MR3733: urlmon: test using http protocol through CreateBinding

[Yuxuan Shui (＠yshui)]

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/3733

[PATCH 1/2] urlmon: test http protocol through CreateBinding

[Yuxuan Shui]

From: Yuxuan Shui <yshui(a)codeweavers.com>

---
 dlls/urlmon/tests/protocol.c | 39 +++++++++++++++++++++++++++++-------
 1 file changed, 32 insertions(+), 7 deletions(-)

diff --git a/dlls/urlmon/tests/protocol.c b/dlls/urlmon/tests/protocol.c
index 550f38659c6..cb8c8c954ce 100644
--- a/dlls/urlmon/tests/protocol.c
+++ b/dlls/urlmon/tests/protocol.c
@@ -143,9 +143,11 @@ static const WCHAR winehq_ipW[] = {'2','0','9','.','4','6','.','2','5','.','1','
 static const WCHAR emptyW[] = {0};
 static const WCHAR pjpegW[] = {'i','m','a','g','e','/','p','j','p','e','g',0};
 static const WCHAR gifW[] = {'i','m','a','g','e','/','g','i','f',0};
+static const WCHAR null_guid[] = {'{','0','0','0','0','0','0','0','0','-','0','0','0','0','-',
+    '0','0','0','0','-','0','0','0','0','-','0','0','0','0','0','0','0','0','0','0','0','0','}',0};
 
 static HRESULT expect_hrResult;
-static LPCWSTR file_name, http_url, expect_wsz;
+static LPCWSTR file_name, http_url, expect_wsz, protocol_clsid;
 static IInternetProtocol *async_protocol = NULL;
 static BOOL first_data_notif, http_is_first, test_redirect, redirect_on_continue;
 static int prot_state, read_report_data, post_stream_read;
@@ -359,7 +361,7 @@ static HRESULT WINAPI HttpNegotiate_GetRootSecurityId(IHttpNegotiate2 *iface,
         BYTE *pbSecurityId, DWORD *pcbSecurityId, DWORD_PTR dwReserved)
 {
     static const BYTE sec_id[] = {'h','t','t','p',':','t','e','s','t',1,0,0,0};
-    
+
     CHECK_EXPECT(GetRootSecurityId);
 
     ok(!dwReserved, "dwReserved=%Id, expected 0\n", dwReserved);
@@ -822,8 +824,6 @@ static const char *status_names[] =
 static HRESULT WINAPI ProtocolSink_ReportProgress(IInternetProtocolSink *iface, ULONG ulStatusCode,
         LPCWSTR szStatusText)
 {
-    static const WCHAR null_guid[] = {'{','0','0','0','0','0','0','0','0','-','0','0','0','0','-',
-        '0','0','0','0','-','0','0','0','0','-','0','0','0','0','0','0','0','0','0','0','0','0','}',0};
     static const WCHAR text_plain[] = {'t','e','x','t','/','p','l','a','i','n',0};
 
     if (winetest_debug > 1)
@@ -913,7 +913,7 @@ static HRESULT WINAPI ProtocolSink_ReportProgress(IInternetProtocolSink *iface,
     case BINDSTATUS_PROTOCOLCLASSID:
         CHECK_EXPECT(ReportProgress_PROTOCOLCLASSID);
         ok(szStatusText != NULL, "szStatusText == NULL\n");
-        ok(!lstrcmpW(szStatusText, null_guid), "unexpected classid %s\n", wine_dbgstr_w(szStatusText));
+        ok(!lstrcmpW(szStatusText, protocol_clsid), "unexpected classid %s\n", wine_dbgstr_w(szStatusText));
         break;
     case BINDSTATUS_COOKIE_SENT:
         CHECK_EXPECT2(ReportProgress_COOKIE_SENT);
@@ -2710,6 +2710,7 @@ static IClassFactory mimefilter_cf = { &MimeFilterCFVtbl };
 #define TEST_FROMCACHE   0x4000
 #define TEST_DISABLEAUTOREDIRECT  0x8000
 #define TEST_RESULTFROMLOCK      0x10000
+#define TEST_USEBINDING  0x20000
 
 static void register_filter(BOOL do_register)
 {
@@ -2770,6 +2771,7 @@ static void init_test(int prot, DWORD flags)
     file_with_hash = FALSE;
     security_problem = FALSE;
     reuse_protocol_thread = FALSE;
+    protocol_clsid = null_guid;
 
     bindinfo_options = 0;
     if(flags & TEST_DISABLEAUTOREDIRECT)
@@ -3357,6 +3359,7 @@ static void test_http_protocol_url(LPCWSTR url, int prot, DWORD flags, DWORD tym
 {
     IInternetProtocolInfo *protocol_info;
     IClassFactory *factory;
+    IInternetSession *session;
     IUnknown *unk;
     HRESULT hres;
 
@@ -3383,8 +3386,13 @@ static void test_http_protocol_url(LPCWSTR url, int prot, DWORD flags, DWORD tym
     if(FAILED(hres))
         return;
 
-    hres = IClassFactory_CreateInstance(factory, NULL, &IID_IInternetProtocol,
-                                        (void**)&async_protocol);
+    if (flags & TEST_USEBINDING) {
+        hres = pCoInternetGetSession(0, &session, 0);
+        ok(hres == S_OK, "CoInternetGetSession failed: %08lx\n", hres);
+        hres = IInternetSession_CreateBinding(session, NULL, url, NULL, NULL, &async_protocol, 0);
+    } else
+        hres = IClassFactory_CreateInstance(factory, NULL, &IID_IInternetProtocol,
+                                            (void**)&async_protocol);
     ok(hres == S_OK, "Could not get IInternetProtocol: %08lx\n", hres);
     if(SUCCEEDED(hres)) {
         BYTE buf[3600];
@@ -3393,6 +3401,12 @@ static void test_http_protocol_url(LPCWSTR url, int prot, DWORD flags, DWORD tym
 
         test_priority(async_protocol);
 
+        if (flags & TEST_USEBINDING) {
+            SET_EXPECT(QueryService_InternetProtocol);
+            SET_EXPECT(ReportProgress_PROTOCOLCLASSID);
+            StringFromCLSID(&CLSID_HttpProtocol, (LPOLESTR *)&protocol_clsid);
+        }
+
         SET_EXPECT(ReportProgress_COOKIE_SENT);
         if(http_is_first) {
             SET_EXPECT(ReportProgress_FINDINGRESOURCE);
@@ -3422,6 +3436,13 @@ static void test_http_protocol_url(LPCWSTR url, int prot, DWORD flags, DWORD tym
             return;
         }
 
+        if (flags & TEST_USEBINDING) {
+            todo_wine CHECK_NOT_CALLED(QueryService_InternetProtocol);
+            todo_wine CHECK_NOT_CALLED(ReportProgress_PROTOCOLCLASSID);
+            CoTaskMemFree((void *)protocol_clsid);
+            protocol_clsid = null_guid;
+        }
+
         if(!direct_read && !bind_from_cache)
             SET_EXPECT(ReportResult);
 
@@ -3589,6 +3610,10 @@ static void test_http_protocol(void)
     bindf = BINDF_ASYNCHRONOUS | BINDF_ASYNCSTORAGE | BINDF_PULLDATA | BINDF_FROMURLMON | BINDF_NOWRITECACHE;
     test_http_protocol_url(empty_url, HTTP_TEST, TEST_EMPTY, TYMED_NULL);
 
+    trace("Testing http protocol (redirected, binding)...\n");
+    bindf = BINDF_ASYNCHRONOUS | BINDF_ASYNCSTORAGE | BINDF_PULLDATA | BINDF_FROMURLMON | BINDF_NOWRITECACHE;
+    test_http_protocol_url(redirect_url, HTTP_TEST, TEST_REDIRECT|TEST_USEBINDING, TYMED_NULL);
+
     /* This is a bit ugly. We unconditionally disable this test on Wine. This won't work until we have
      * support for reading from cache via HTTP layer in wininet. Until then, Wine will fail badly, affecting
      * other, unrelated, tests. Working around it is not worth the trouble, we may simply make sure those
-- 
GitLab


https://gitlab.winehq.org/wine/wine/-/merge_requests/3733

[PATCH 2/2] urlmon: fix NULL dereference in BindProtocol_UnlockRequest

[Yuxuan Shui]

From: Yuxuan Shui <yshui(a)codeweavers.com>

---
 dlls/urlmon/bindprot.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/dlls/urlmon/bindprot.c b/dlls/urlmon/bindprot.c
index b09acdd5c3a..ec52ac5e999 100644
--- a/dlls/urlmon/bindprot.c
+++ b/dlls/urlmon/bindprot.c
@@ -815,7 +815,9 @@ static HRESULT WINAPI ProtocolHandler_UnlockRequest(IInternetProtocol *iface)
 
     TRACE("(%p)\n", This);
 
-    return IInternetProtocol_UnlockRequest(This->protocol);
+    if (This->protocol)
+        return IInternetProtocol_UnlockRequest(This->protocol);
+    return S_OK;
 }
 
 static const IInternetProtocolVtbl InternetProtocolHandlerVtbl = {
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/3733

[Jacek Caban (＠jacek)]

Jacek Caban (@jacek) commented about dlls/urlmon/tests/protocol.c:
> test_priority(async_protocol);
>
> +        if (flags & TEST_USEBINDING) {
> +            SET_EXPECT(QueryService_InternetProtocol);
> +            SET_EXPECT(ReportProgress_PROTOCOLCLASSID);
> +            StringFromCLSID(&CLSID_HttpProtocol, (LPOLESTR *)&protocol_clsid);

Please use an array for protocol_clsid and `StringFromGUID2` here to avoid allocation. Other than that, the patch looks good to me.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/3733#note_44392

[Yuxuan Shui (＠yshui)]

On Wed Sep  6 09:35:27 2023 +0000, Jacek Caban wrote:
> Please use an array for protocol_clsid and `StringFromGUID2` here to
> avoid allocation. Other than that, the patch looks good to me.
> can I do

```c
static WCHAR protocol_clsid[ARRAY_SIZE(null_guid)];
```

or do I have to

```c
static WCHAR protocol_clsid[39];
```

?

Or maybe there is a constant defined for the length of a GUID string?

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/3733#note_44397

[Yuxuan Shui (＠yshui)]

On Wed Sep  6 10:25:59 2023 +0000, Yuxuan Shui wrote:
> can I do
> ```c
> static WCHAR protocol_clsid[ARRAY_SIZE(null_guid)];
> ```
> or do I have to
> ```c
> static WCHAR protocol_clsid[39];
> ```
> ?
> Or maybe there is a constant defined for the length of a GUID string?
> Ah there is `CHARS_IN_GUID`

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/3733#note_44401

[Yuxuan Shui (＠yshui)]

On Wed Sep  6 10:27:15 2023 +0000, Yuxuan Shui wrote:
> Ah there is `CHARS_IN_GUID`
> But it's private

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/3733#note_44403