[PATCH v2 0/5] MR2634: winintest:internet InternetGetConnectedStateEx*() fixes
* Fix InternetGetConnectedStateEx() parameter checking. * InternetGetConnectedStateExA() must always null-terminate the state string. * Dump the state string if it is not as expected. * Remove a couple of redundant InternetGetConnectedStateEx*() tests. * Avoid an unnecessary lstrlenW() call in internet.c. -- v2: wininet/tests: Fix InternetGetConnectedStateEx() parameter checking. wininet: InternetGetConnectedStateExA() must always null-terminate the state string. wininet/tests: Dump the state string if it is not as expected. https://gitlab.winehq.org/wine/wine/-/merge_requests/2634
From: Francois Gouget <fgouget(a)codeweavers.com> --- dlls/wininet/tests/internet.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/dlls/wininet/tests/internet.c b/dlls/wininet/tests/internet.c index e977bb4af6b..7c7fbef8057 100644 --- a/dlls/wininet/tests/internet.c +++ b/dlls/wininet/tests/internet.c @@ -1773,16 +1773,15 @@ static void test_InternetGetConnectedStateExW(void) buffer[0] = 0; res = pInternetGetConnectedStateExW(NULL, buffer, ARRAY_SIZE(buffer), 0); ok(res == TRUE, "Expected TRUE, got %d\n", res); - sz = lstrlenW(buffer); - ok(sz > 0, "Expected a connection name\n"); + ok(buffer[0], "Expected a connection name\n"); buffer[0] = 0; flags = 0; res = pInternetGetConnectedStateExW(&flags, buffer, ARRAY_SIZE(buffer), 0); ok(res == TRUE, "Expected TRUE, got %d\n", res); ok(flags, "Expected at least one flag set\n"); + ok(buffer[0], "Expected a connection name\n"); sz = lstrlenW(buffer); - ok(sz > 0, "Expected a connection name\n"); flags = 0; res = pInternetGetConnectedStateExW(&flags, NULL, ARRAY_SIZE(buffer), 0); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2634
From: Francois Gouget <fgouget(a)codeweavers.com> The first test would succeed whether the API touched the buffer or not, while the second one only succeeds if the API sets the first character to 0. So keeping the second test is sufficient. --- dlls/wininet/tests/internet.c | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/dlls/wininet/tests/internet.c b/dlls/wininet/tests/internet.c index 7c7fbef8057..1449431c8d6 100644 --- a/dlls/wininet/tests/internet.c +++ b/dlls/wininet/tests/internet.c @@ -1698,13 +1698,6 @@ static void test_InternetGetConnectedStateExA(void) ok(flags, "Expected at least one flag set\n"); ok(sz / 2 - 1 == strlen(buffer), "Expected %lu bytes, got %u\n", sz / 2 - 1, lstrlenA(buffer)); - buffer[0] = 0; - flags = 0; - res = pInternetGetConnectedStateExA(&flags, buffer, 1, 0); - ok(res == TRUE, "Expected TRUE, got %d\n", res); - ok(flags, "Expected at least one flag set\n"); - ok(!buffer[0], "Expected 0 bytes, got %u\n", lstrlenA(buffer)); - buffer[0] = 0; flags = 0; res = pInternetGetConnectedStateExA(&flags, buffer, 2, 0); @@ -1809,13 +1802,6 @@ static void test_InternetGetConnectedStateExW(void) else ok(sz / 2 - 1 == lstrlenW(buffer), "Expected %lu bytes, got %u\n", sz / 2 - 1, lstrlenW(buffer)); - buffer[0] = 0; - flags = 0; - res = pInternetGetConnectedStateExW(&flags, buffer, 1, 0); - ok(res == TRUE, "Expected TRUE, got %d\n", res); - ok(flags, "Expected at least one flag set\n"); - ok(!buffer[0], "Expected 0 bytes, got %u\n", lstrlenW(buffer)); - buffer[0] = 0; flags = 0; res = pInternetGetConnectedStateExW(&flags, buffer, 2, 0); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2634
From: Francois Gouget <fgouget(a)codeweavers.com> Also set up the buffer so dumping it is safe enough if the string is not null-terminated. And fix the failure messages since the tested value is the string length in characters, not the buffer size in bytes. --- dlls/wininet/tests/internet.c | 47 +++++++++++++++++++++-------------- 1 file changed, 28 insertions(+), 19 deletions(-) diff --git a/dlls/wininet/tests/internet.c b/dlls/wininet/tests/internet.c index 1449431c8d6..1d717572aad 100644 --- a/dlls/wininet/tests/internet.c +++ b/dlls/wininet/tests/internet.c @@ -29,6 +29,7 @@ #include "winerror.h" #include "winreg.h" #include "winnls.h" +#include "wchar.h" #include "wine/test.h" @@ -1684,33 +1685,37 @@ static void test_InternetGetConnectedStateExA(void) ok(flags, "Expected at least one flag set\n"); /* no space for complete string this time */ - buffer[0] = 0; + memset(buffer, 'z', sizeof(buffer) - 1); + buffer[sizeof(buffer) - 1] = 0; flags = 0; res = pInternetGetConnectedStateExA(&flags, buffer, sz, 0); ok(res == TRUE, "Expected TRUE, got %d\n", res); ok(flags, "Expected at least one flag set\n"); - ok(sz - 1 == strlen(buffer), "Expected %lu bytes, got %u\n", sz - 1, lstrlenA(buffer)); + ok(sz - 1 == strlen(buffer), "Expected len %lu, got %u: %s\n", sz - 1, lstrlenA(buffer), wine_dbgstr_a(buffer)); - buffer[0] = 0; + memset(buffer, 'z', sizeof(buffer) - 1); + buffer[sizeof(buffer) - 1] = 0; flags = 0; res = pInternetGetConnectedStateExA(&flags, buffer, sz / 2, 0); ok(res == TRUE, "Expected TRUE, got %d\n", res); ok(flags, "Expected at least one flag set\n"); - ok(sz / 2 - 1 == strlen(buffer), "Expected %lu bytes, got %u\n", sz / 2 - 1, lstrlenA(buffer)); + ok(sz / 2 - 1 == strlen(buffer), "Expected len %lu, got %u: %s\n", sz / 2 - 1, lstrlenA(buffer), wine_dbgstr_a(buffer)); - buffer[0] = 0; + memset(buffer, 'z', sizeof(buffer) - 1); + buffer[sizeof(buffer) - 1] = 0; flags = 0; res = pInternetGetConnectedStateExA(&flags, buffer, 2, 0); ok(res == TRUE, "Expected TRUE, got %d\n", res); ok(flags, "Expected at least one flag set\n"); - ok(strlen(buffer) == 1, "Expected 1 byte, got %u\n", lstrlenA(buffer)); + ok(strlen(buffer) == 1, "Expected len 1, got %u: %s\n", lstrlenA(buffer), wine_dbgstr_a(buffer)); + memset(buffer, 'z', sizeof(buffer) - 1); + buffer[sizeof(buffer) - 1] = 0; flags = 0; - buffer[0] = 0xDE; res = pInternetGetConnectedStateExA(&flags, buffer, 1, 0); ok(res == TRUE, "Expected TRUE, got %d\n", res); ok(flags, "Expected at least one flag set\n"); - ok(!buffer[0], "Expected 0 bytes, got %u\n", lstrlenA(buffer)); + ok(!buffer[0], "Expected len 0, got %u: %s\n", lstrlenA(buffer), wine_dbgstr_a(buffer)); } static void test_InternetGetConnectedStateExW(void) @@ -1782,42 +1787,46 @@ static void test_InternetGetConnectedStateExW(void) ok(flags, "Expected at least one flag set\n"); /* no space for complete string this time */ - buffer[0] = 0; + wmemset(buffer, 'z', ARRAY_SIZE(buffer) - 1); + buffer[ARRAY_SIZE(buffer) - 1] = 0; flags = 0; res = pInternetGetConnectedStateExW(&flags, buffer, sz, 0); ok(res == TRUE, "Expected TRUE, got %d\n", res); ok(flags, "Expected at least one flag set\n"); if (flags & INTERNET_CONNECTION_MODEM) - ok(!buffer[0], "Expected 0 bytes, got %u\n", lstrlenW(buffer)); + ok(!buffer[0], "Expected len 0, got %u: %s\n", lstrlenW(buffer), wine_dbgstr_w(buffer)); else - ok(sz - 1 == lstrlenW(buffer), "Expected %lu bytes, got %u\n", sz - 1, lstrlenW(buffer)); + ok(sz - 1 == lstrlenW(buffer), "Expected len %lu, got %u: %s\n", sz - 1, lstrlenW(buffer), wine_dbgstr_w(buffer)); - buffer[0] = 0; + wmemset(buffer, 'z', ARRAY_SIZE(buffer) - 1); + buffer[ARRAY_SIZE(buffer) - 1] = 0; flags = 0; res = pInternetGetConnectedStateExW(&flags, buffer, sz / 2, 0); ok(res == TRUE, "Expected TRUE, got %d\n", res); ok(flags, "Expected at least one flag set\n"); if (flags & INTERNET_CONNECTION_MODEM) - ok(!buffer[0], "Expected 0 bytes, got %u\n", lstrlenW(buffer)); + ok(!buffer[0], "Expected len 0, got %u: %s\n", lstrlenW(buffer), wine_dbgstr_w(buffer)); else - ok(sz / 2 - 1 == lstrlenW(buffer), "Expected %lu bytes, got %u\n", sz / 2 - 1, lstrlenW(buffer)); + ok(sz / 2 - 1 == lstrlenW(buffer), "Expected len %lu, got %u: %s\n", sz / 2 - 1, lstrlenW(buffer), wine_dbgstr_w(buffer)); - buffer[0] = 0; + wmemset(buffer, 'z', ARRAY_SIZE(buffer) - 1); + buffer[ARRAY_SIZE(buffer) - 1] = 0; flags = 0; res = pInternetGetConnectedStateExW(&flags, buffer, 2, 0); ok(res == TRUE, "Expected TRUE, got %d\n", res); ok(flags, "Expected at least one flag set\n"); if (flags & INTERNET_CONNECTION_MODEM) - ok(!buffer[0], "Expected 0 bytes, got %u\n", lstrlenW(buffer)); + ok(!buffer[0], "Expected len 0, got %u: %s\n", lstrlenW(buffer), wine_dbgstr_w(buffer)); else - ok(lstrlenW(buffer) == 1, "Expected 1 byte, got %u\n", lstrlenW(buffer)); + ok(lstrlenW(buffer) == 1, "Expected len 1, got %u: %s\n", lstrlenW(buffer), wine_dbgstr_w(buffer)); - buffer[0] = 0xDEAD; + wmemset(buffer, 'z', ARRAY_SIZE(buffer) - 1); + buffer[ARRAY_SIZE(buffer) - 1] = 0; flags = 0; res = pInternetGetConnectedStateExW(&flags, buffer, 1, 0); ok(res == TRUE, "Expected TRUE, got %d\n", res); ok(flags, "Expected at least one flag set\n"); - ok(!buffer[0], "Expected 0 bytes, got %u\n", lstrlenW(buffer)); + ok(!buffer[0], "Expected len 0, got %u: %s\n", lstrlenW(buffer), wine_dbgstr_w(buffer)); } static void test_format_message(HMODULE hdll) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2634
From: Francois Gouget <fgouget(a)codeweavers.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=54799 --- dlls/wininet/internet.c | 4 ++++ dlls/wininet/tests/internet.c | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/dlls/wininet/internet.c b/dlls/wininet/internet.c index 6155d60938a..665e339f920 100644 --- a/dlls/wininet/internet.c +++ b/dlls/wininet/internet.c @@ -1296,8 +1296,12 @@ BOOL WINAPI InternetGetConnectedStateExA(LPDWORD lpdwStatus, LPSTR lpszConnectio rc = InternetGetConnectedStateExW(lpdwStatus,lpwszConnectionName, dwNameLen, dwReserved); if (rc && lpwszConnectionName) + { WideCharToMultiByte(CP_ACP,0,lpwszConnectionName,-1,lpszConnectionName, dwNameLen, NULL, NULL); + /* Yes, blindly truncate double-byte characters */ + lpszConnectionName[dwNameLen - 1] = '\0'; + } free(lpwszConnectionName); return rc; diff --git a/dlls/wininet/tests/internet.c b/dlls/wininet/tests/internet.c index 1d717572aad..385c7c5f826 100644 --- a/dlls/wininet/tests/internet.c +++ b/dlls/wininet/tests/internet.c @@ -1684,7 +1684,7 @@ static void test_InternetGetConnectedStateExA(void) ok(res == TRUE, "Expected TRUE, got %d\n", res); ok(flags, "Expected at least one flag set\n"); - /* no space for complete string this time */ + /* no space for complete string this time, double-byte characters get truncated */ memset(buffer, 'z', sizeof(buffer) - 1); buffer[sizeof(buffer) - 1] = 0; flags = 0; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2634
From: Francois Gouget <fgouget(a)codeweavers.com> It should set the error code if given a non-zero reserved value. --- dlls/wininet/internet.c | 3 +++ dlls/wininet/tests/internet.c | 5 +++++ 2 files changed, 8 insertions(+) diff --git a/dlls/wininet/internet.c b/dlls/wininet/internet.c index 665e339f920..a2d9f00ce6f 100644 --- a/dlls/wininet/internet.c +++ b/dlls/wininet/internet.c @@ -1216,7 +1216,10 @@ BOOL WINAPI InternetGetConnectedStateExW(LPDWORD lpdwStatus, LPWSTR lpszConnecti /* Must be zero */ if(dwReserved) + { + SetLastError(ERROR_INVALID_PARAMETER); return FALSE; + } for (;;) { diff --git a/dlls/wininet/tests/internet.c b/dlls/wininet/tests/internet.c index 385c7c5f826..c2dc83e369b 100644 --- a/dlls/wininet/tests/internet.c +++ b/dlls/wininet/tests/internet.c @@ -1747,6 +1747,11 @@ static void test_InternetGetConnectedStateExW(void) res = pInternetGetConnectedStateExW(NULL, NULL, 0, 0); ok(res == TRUE, "Expected TRUE, got %d\n", res); + SetLastError(0xdeadbeef); + res = pInternetGetConnectedStateExW(NULL, NULL, 0, 1); + ok(res == FALSE, "Expected TRUE, got %d\n", res); + ok(GetLastError() == ERROR_INVALID_PARAMETER, "Unexpected gle %lu\n", GetLastError()); + flags = 0; res = pInternetGetConnectedStateExW(&flags, NULL, 0, 0); ok(res == TRUE, "Expected TRUE, got %d\n", res); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2634
v2: Fix the integer format 64-bit compilation errors. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2634#note_29713
This merge request was approved by Jacek Caban. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2634
participants (3)
-
Francois Gouget -
Francois Gouget (＠fgouget)
-
Jacek Caban (＠jacek)