[PATCH 0/5] MR2634: winintest:internet InternetGetConnectedStateEx*() fixes

[Francois Gouget (＠fgouget)]

* 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.

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

[PATCH 1/5] wininet/tests: Avoid an unnecessary lstrlenW() call in internet.c.

[Francois Gouget]

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

[PATCH 2/5] winintest/tests: Remove a couple of redundant InternetGetConnectedStateEx*() tests.

[Francois Gouget]

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

[PATCH 3/5] wininet/tests: Dump the state string if it is not as expected.

[Francois Gouget]

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..e87fce049b1 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, strlen(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, strlen(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", strlen(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", strlen(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

[PATCH 4/5] wininet: InternetGetConnectedStateExA() must always null-terminate the state string.

[Francois Gouget]

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 e87fce049b1..a32813b598b 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

[PATCH 5/5] wininet/tests: Fix InternetGetConnectedStateEx() parameter checking.

[Francois Gouget]

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 a32813b598b..50bb4883735 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