[PATCH v2 0/1] MR3625: urlmon: Add support for IsValidURL
-- v2: urlmon: Add support for IsValidURL https://gitlab.winehq.org/wine/wine/-/merge_requests/3625
From: Leonid Pospelov <leonidpospelov.dev(a)gmail.com> --- dlls/urlmon/tests/misc.c | 148 ++++++++++++++++++++++++++++++++++++++ dlls/urlmon/urlmon_main.c | 44 ++++++++++-- 2 files changed, 187 insertions(+), 5 deletions(-) diff --git a/dlls/urlmon/tests/misc.c b/dlls/urlmon/tests/misc.c index 4cd64749b1d..dfe3cdc8358 100644 --- a/dlls/urlmon/tests/misc.c +++ b/dlls/urlmon/tests/misc.c @@ -1774,6 +1774,148 @@ static void test_MkParseDisplayNameEx(void) IBindCtx_Release(bctx); } +static const WCHAR* generate_long_url_valid() +{ + static WCHAR w[2078]; + wmemset(w, L'd', 2077); + w[2077] = L'\0'; + + WCHAR prefix[4] = L"C:/"; + wcsncpy(w, prefix, sizeof(prefix) / sizeof(WCHAR) - 1); + + return w; +} + +static const WCHAR* generate_long_url_invalid() +{ + static WCHAR w[2079]; + wmemset(w, L'd', 2078); + w[2078] = L'\0'; + + WCHAR prefix[4] = L"C:/"; + wcsncpy(w, prefix, sizeof(prefix) / sizeof(WCHAR) - 1); + + return w; +} + +static const struct { + const WCHAR* url; + HRESULT expected_result; +} url_tests[] = { + {L"",1}, + {L"://",1}, + {L"a",1}, + {L"7",1}, + {L"%%%",1}, + {L"-",1}, + {L")(*&^%$#@!QAZWSXEDCRFVTGBYHNUJMIK<OL>P:?{",1}, + {L"$$$",1}, + {L";",1}, + {L"\"\"\"",1}, + {L"<>",1}, + {L"???",1}, + {L"ddd",1}, + {L"@|@",1}, + {L"#@|@",1}, + {L"",1}, + {L"vk.com",1}, + {L"https://www.vk.com",0}, + {L"httpc://www.vk.com",1}, + {L"http://www.vk.com",0}, + {L"ftp://www.vk.com",0}, + {L"https://www.vk",0}, + {L"https://www.vk.",0}, + {L"https://www",0}, + {L"https://",0}, + {L"file://",0}, + {L"f://",0}, + {L"o://",0}, + {L"ooo://",1}, + {L"d://",0}, + {L"dd://",1}, + {L"C:/",0}, + {L"C:/ss",0}, + {L"ftp://",0}, + {L"ftf://",1}, + {L"\\\\Media\\Pictures\\Worth\\1000 words",0}, + {L"\\\\\\\\Media\\Pictures\\Worth\\1000 words",0}, + {L"\\\\\\\\Media\\Pictures\\Worth\\1000 words<",0}, + {L"\\\\\\\\Media\\Pictures\\Worth\\1000 words:",0}, + {L"\\\\\\\\Media\\Pictures\\Worth\\1000 words:s",0}, + {L"\\\\\\\\Media\\Pictures\\Worth\\1000 words:.",0}, + {L"\\\\?\\D:\\Plans\\Marshall",1}, + {L"\\\\.\\D:\\Projects\\Human Genome",0}, + {L"C:",0}, + {L":",1}, + {L".\\Manhattan",1}, + {L"..\\Plans",1}, + {L"\\Pacts",1}, + {L"0:",1}, + {L"\\\\Work\\Hard",0}, + {L"\\\\192.168.1.15\\Hard",0}, + {L"https://www.vk.com\1",0}, + {L"sip://www.vk.co",1}, + {L"sms://www.vk.co",1}, + {generate_long_url_valid(),0}, + {generate_long_url_invalid(),1}, + {L"1:",1}, + {L"/",1}, + {L"\\",1}, + {L"irc://www.x.com",1}, + {L"data://www.x.com",1}, + {L"\\\\&\\D:\\Plans\\Marshall",0}, + {L"\\\\!\\D:\\Plans\\Marshall",0}, + {L"\\\\j?j\\D:\\Plans\\Marshall",0}, + {L"\\\\j?\\D:\\Plans\\Marshall",0}, + {L"\\\\?j\\D:\\Plans\\Marshall",1}, + {L"http://www.例子.com",0}, + {L"http://例子.com",0}, + {L"http://www.example.国际",0}, + {L"http://192.0.2.1",0}, + {L"https://[2001:0db8:85a3:0000:0000:8a2e:0370:7334]",0}, + {L"http://localhost",0}, + {L"https://localhost:8080",0}, + {L"https://localhost:80808080",0}, + {L"https://[2001:0db8:85a3:00000000:8a2e:0370:7334]",0}, + {L"http://192.0.2222.1",0}, + {L"http://www.example.com/a_b",0}, + {L"http://www.example.com/a%20b",0}, + {L"http://www.example.com/?a=b&c=d",0}, + {L"http://www.example.com/#!abc",0}, + {L"http://www.example.com/#abc",0}, + {L"mailto:email(a)example.com",0}, + {L"HTTP://www.EXAMPLE.com",0}, + {L"hTTp://www.exaMple.com",0}, + {L"mk://x",0}, + {L"gopher://host /7a_gopher_selector%09foobar",1}, + {L"http:/",0}, + {L"http:",0}, + {L"http",0}, + {L"htt",1}, + {L"https:/",0}, + {L"https:",0}, + {L"https",0}, + {L"http",0}, + {L"ftp:/",0}, + {L"ftp:",0}, + {L"ftp",0}, + {L"ft",1}, + {L"file:/",0}, + {L"file:",0}, + {L"file",0}, + {L"fil",1}, + {L"mailto:/",0}, + {L"mailto:",0}, + {L"mailto",0}, + {L"mailt",1}, + {L"mk:/",0}, + {L"mk:",0}, + {L"mk",0}, + {L"m",1}, + {L"mkkk",1}, + {L"mkkk:/",1} +}; + static void test_IsValidURL(void) { HRESULT hr; @@ -1790,6 +1932,12 @@ static void test_IsValidURL(void) hr = IsValidURL(bctx, wszHttpWineHQ, 0); ok(hr == S_OK, "Expected S_OK, got %08lx\n", hr); + DWORD i; + for(i = 0; i < ARRAY_SIZE(default_feature_tests); ++i) { + hr = IsValidURL(bctx, url_tests[i].url, 0); + ok(hr == url_tests[i].expected_result, "[%d] Expected %08lx, got %08lx\n", i, url_tests[i].expected_result, hr); + } + IBindCtx_Release(bctx); } diff --git a/dlls/urlmon/urlmon_main.c b/dlls/urlmon/urlmon_main.c index 491313b7cf8..9d303d33926 100644 --- a/dlls/urlmon/urlmon_main.c +++ b/dlls/urlmon/urlmon_main.c @@ -493,6 +493,28 @@ HRESULT WINAPI DllRegisterServerEx(void) return E_FAIL; } +static BOOL StartsWithProtocol(LPCWSTR url, size_t len, LPCWSTR protocol) +{ + if (!url || !protocol) + return FALSE; + + size_t protocolLen = wcslen(protocol); + + if (len < protocolLen) + return FALSE; + + if (_wcsnicmp(url, protocol, protocolLen) != 0) + return FALSE; + + return len == protocolLen || url[protocolLen] == L':'; +} + +static BOOL IsDriveLetterURL(LPCWSTR url, size_t len) +{ + return (len >= 2 && url[1] == L':' && + ((url[0] >= L'a' && url[0] <= L'z') || (url[0] >= L'A' && url[0] <= L'Z'))); +} + /************************************************************************** * IsValidURL (URLMON.@) * @@ -507,18 +529,30 @@ HRESULT WINAPI DllRegisterServerEx(void) * Success: S_OK. * Failure: S_FALSE. * returns E_INVALIDARG if one or more of the args is invalid. - * - * TODO: - * test functionality against windows to see what a valid URL is. */ HRESULT WINAPI IsValidURL(LPBC pBC, LPCWSTR szURL, DWORD dwReserved) { - FIXME("(%p, %s, %ld): stub\n", pBC, debugstr_w(szURL), dwReserved); + const size_t MAX_URL_SIZE = 2078; if (dwReserved || !szURL) return E_INVALIDARG; - return S_OK; + const size_t len = wcslen(szURL); + + if (len >= MAX_URL_SIZE) + return S_FALSE; + + if (StartsWithProtocol(szURL, len, L"https") || + StartsWithProtocol(szURL, len, L"http") || + StartsWithProtocol(szURL, len, L"ftp") || + StartsWithProtocol(szURL, len, L"file") || + StartsWithProtocol(szURL, len, L"mailto") || + StartsWithProtocol(szURL, len, L"mk") || + IsDriveLetterURL(szURL, len) || + PathIsUNCW(szURL)) + return S_OK; + + return S_FALSE; } /************************************************************************** -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3625
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=136530 Your paranoid android. === build (build log) === ../wine/include/winbase.h:2951:24: error: unknown type name ���error���; did you mean ���errno_t���? ../wine/include/winbase.h:2951:24: error: unknown type name ���error���; did you mean ���errno_t���? ../wine/dlls/urlmon/tests/misc.c:1859:6: error: initializer element is not constant ../wine/dlls/urlmon/tests/misc.c:1860:6: error: initializer element is not constant ../wine/dlls/urlmon/tests/misc.c:1936:31: error: ���default_feature_tests��� undeclared (first use in this function) ../wine/include/winnt.h:888:35: error: invalid operands to binary / (have ���const struct <anonymous> *��� and ���unsigned int���) Task: The exe32 Wine build failed === debian11 (build log) === ../wine/include/winbase.h:2951:24: error: unknown type name ���error���; did you mean ���errno_t���? ../wine/include/winbase.h:2951:24: error: unknown type name ���error���; did you mean ���errno_t���? ../wine/dlls/urlmon/tests/misc.c:1859:6: error: initializer element is not constant ../wine/dlls/urlmon/tests/misc.c:1860:6: error: initializer element is not constant ../wine/dlls/urlmon/tests/misc.c:1936:31: error: ���default_feature_tests��� undeclared (first use in this function) ../wine/include/winnt.h:888:35: error: invalid operands to binary / (have ���const struct <anonymous> *��� and ���unsigned int���) Task: The win32 Wine build failed === debian11b (build log) === ../wine/include/winbase.h:2951:24: error: unknown type name ���error���; did you mean ���errno_t���? ../wine/include/winbase.h:2951:24: error: unknown type name ���error���; did you mean ���errno_t���? ../wine/dlls/urlmon/tests/misc.c:1859:6: error: initializer element is not constant ../wine/dlls/urlmon/tests/misc.c:1860:6: error: initializer element is not constant ../wine/dlls/urlmon/tests/misc.c:1936:31: error: ���default_feature_tests��� undeclared (first use in this function) ../wine/include/winnt.h:888:35: error: invalid operands to binary / (have ���const struct <anonymous> *��� and ���long long unsigned int���) Task: The wow64 Wine build failed
Jacek Caban (@jacek) commented about dlls/urlmon/urlmon_main.c:
if (dwReserved || !szURL) return E_INVALIDARG;
- return S_OK; + const size_t len = wcslen(szURL); + + if (len >= MAX_URL_SIZE) + return S_FALSE; + + if (StartsWithProtocol(szURL, len, L"https") || + StartsWithProtocol(szURL, len, L"http") || + StartsWithProtocol(szURL, len, L"ftp") || + StartsWithProtocol(szURL, len, L"file") || + StartsWithProtocol(szURL, len, L"mailto") || + StartsWithProtocol(szURL, len, L"mk") ||
I think that we should use pluggable protocols to try to parse the URL. See the attached [test hack](/uploads/ef350910b23dc3455576e5324d63f35f/test.diff). It fails on Windows showing how it's supposed to work: it queries IInternetProtocolInfo interface of protocol handler and calls ParseUrl(PARSE_CANONICALIZE) on it. Maybe we could just use `CoInternetParseUrl` for that and see if it fails, I'm not sure without more tests. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3625#note_43017
participants (4)
-
Jacek Caban (＠jacek) -
Leonid Pospelov
-
Leonid Pospelov (＠Pospelove)
-
Marvin