[PATCH v2 0/2] MR3733: urlmon: test using http protocol through CreateBinding
-- v2: urlmon: fix NULL dereference in BindProtocol_UnlockRequest urlmon: test http protocol through CreateBinding https://gitlab.winehq.org/wine/wine/-/merge_requests/3733
From: Yuxuan Shui <yshui(a)codeweavers.com> --- dlls/urlmon/tests/protocol.c | 43 +++++++++++++++++++++++++++++++----- 1 file changed, 37 insertions(+), 6 deletions(-) diff --git a/dlls/urlmon/tests/protocol.c b/dlls/urlmon/tests/protocol.c index 550f38659c6..1589ab1f8f3 100644 --- a/dlls/urlmon/tests/protocol.c +++ b/dlls/urlmon/tests/protocol.c @@ -34,6 +34,8 @@ static HRESULT (WINAPI *pCoInternetGetSession)(DWORD, IInternetSession **, DWORD static HRESULT (WINAPI *pReleaseBindInfo)(BINDINFO*); static HRESULT (WINAPI *pCreateUri)(LPCWSTR, DWORD, DWORD_PTR, IUri**); +#define CHARS_IN_GUID 39 + #define DEFINE_EXPECT(func) \ static BOOL expect_ ## func = FALSE, called_ ## func = FALSE @@ -143,11 +145,15 @@ 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 WCHAR protocol_clsid[CHARS_IN_GUID]; static HRESULT expect_hrResult; static LPCWSTR file_name, http_url, expect_wsz; static IInternetProtocol *async_protocol = NULL; static BOOL first_data_notif, http_is_first, test_redirect, redirect_on_continue; +static BOOL no_stop_on_switch; static int prot_state, read_report_data, post_stream_read; static DWORD bindf, ex_priority , pi, bindinfo_options; static IInternetProtocol *binding_protocol, *filtered_protocol; @@ -359,7 +365,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 +828,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 +917,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 +2714,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 +2775,7 @@ static void init_test(int prot, DWORD flags) file_with_hash = FALSE; security_problem = FALSE; reuse_protocol_thread = FALSE; + memcpy(protocol_clsid, null_guid, sizeof(null_guid)); bindinfo_options = 0; if(flags & TEST_DISABLEAUTOREDIRECT) @@ -3357,6 +3363,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 +3390,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 +3405,14 @@ 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); + StringFromGUID2(&CLSID_HttpProtocol, protocol_clsid, ARRAY_SIZE(protocol_clsid)); + if (flags & TEST_DISABLEAUTOREDIRECT) + SET_EXPECT(QueryService_IBindCallbackRedirect); + } + SET_EXPECT(ReportProgress_COOKIE_SENT); if(http_is_first) { SET_EXPECT(ReportProgress_FINDINGRESOURCE); @@ -3422,6 +3442,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); + if (flags & TEST_DISABLEAUTOREDIRECT) + CHECK_CALLED(QueryService_IBindCallbackRedirect); + } + if(!direct_read && !bind_from_cache) SET_EXPECT(ReportResult); @@ -3589,6 +3616,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
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
On Wed Sep 6 10:33:37 2023 +0000, Yuxuan Shui wrote:
changed this line in [version 2 of the diff](/wine/wine/-/merge_requests/3733/diffs?diff_id=67106&start_sha=aaee7c99c9f17831b95d20d9cec181ff06c8a71f#54cd5296ccaff13067f687f78bff8d8335768c05_3407_3411) It's available in axcore.idl.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3733#note_44406
On Wed Sep 6 10:36:09 2023 +0000, Jacek Caban wrote:
It's available in axcore.idl. Which header do I include?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/3733#note_44411
participants (3)
-
Jacek Caban (＠jacek) -
Yuxuan Shui
-
Yuxuan Shui (＠yshui)