wininet: Fix InternetGetConnectedStateEx[A|W] (try 2)

[Bruno Jesus]

try 2:
Add comments to make it more clear why we can't trust LoadStringW;
Add more tests to get some corner cases about filling the buffer when
string will not fit;
Check WideCharToMultiByte result to ensure the buffer is not filled incorrectly.

Superseeds patch 102239.

Fixes http://bugs.winehq.org/show_bug.cgi?id=17796
-------------- next part --------------
diff --git a/dlls/wininet/internet.c b/dlls/wininet/internet.c
index a37606a..88ec166 100644
--- a/dlls/wininet/internet.c
+++ b/dlls/wininet/internet.c
@@ -1213,13 +1213,22 @@ BOOL WINAPI InternetGetConnectedStateExW(LPDWORD lpdwStatus, LPWSTR lpszConnecti
 
     /* Must be zero */
     if(dwReserved)
-	return FALSE;
+        return FALSE;
 
     if (lpdwStatus) {
         WARN("always returning LAN connection.\n");
         *lpdwStatus = INTERNET_CONNECTION_LAN;
     }
-    return LoadStringW(WININET_hModule, IDS_LANCONNECTION, lpszConnectionName, dwNameLen) > 0;
+
+    /* When the buffer size is zero LoadStringW fills the buffer with a pointer to
+     * the resource, avoid it as we must not change the buffer in this case */
+    if(lpszConnectionName && dwNameLen) {
+        *lpszConnectionName = '\0';
+        LoadStringW(WININET_hModule, IDS_LANCONNECTION, lpszConnectionName, dwNameLen);
+    }
+
+     /* Ensure TRUE is returned and not != 0 as some applications depend on that */
+    return TRUE;
 }
 
 
@@ -1241,8 +1250,9 @@ BOOL WINAPI InternetGetConnectedStateExA(LPDWORD lpdwStatus, LPSTR lpszConnectio
                                       dwReserved);
     if (rc && lpwszConnectionName)
     {
-        WideCharToMultiByte(CP_ACP,0,lpwszConnectionName,-1,lpszConnectionName,
-                            dwNameLen, NULL, NULL);
+        if (WideCharToMultiByte(CP_ACP,0,lpwszConnectionName,-1,lpszConnectionName,
+                                dwNameLen, NULL, NULL) == 0)
+            *lpszConnectionName = '\0';
         heap_free(lpwszConnectionName);
     }
     return rc;
diff --git a/dlls/wininet/tests/internet.c b/dlls/wininet/tests/internet.c
index 837e429..b574629 100644
--- a/dlls/wininet/tests/internet.c
+++ b/dlls/wininet/tests/internet.c
@@ -1473,19 +1473,16 @@ static void test_InternetGetConnectedStateExA(void)
     trace("Internet Connection: Flags 0x%02x - Name '%s'\n", flags, buffer);
 
     res = pInternetGetConnectedStateExA(NULL, NULL, 0, 0);
-todo_wine
     ok(res == TRUE, "Expected TRUE, got %d\n", res);
 
     flags = 0;
     res = pInternetGetConnectedStateExA(&flags, NULL, 0, 0);
-todo_wine
     ok(res == TRUE, "Expected TRUE, got %d\n", res);
     ok(flags, "Expected at least one flag set\n");
 
     buffer[0] = 0;
     flags = 0;
     res = pInternetGetConnectedStateExA(&flags, buffer, 0, 0);
-todo_wine
     ok(res == TRUE, "Expected TRUE, got %d\n", res);
     ok(flags, "Expected at least one flag set\n");
     ok(!buffer[0], "Buffer must not change, got %02X\n", buffer[0]);
@@ -1504,6 +1501,11 @@ todo_wine
     sz = strlen(buffer);
     ok(sz > 0, "Expected a connection name\n");
 
+    flags = 0;
+    res = pInternetGetConnectedStateExA(&flags, NULL, sizeof(buffer), 0);
+    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 */
     buffer[0] = 0;
     flags = 0;
@@ -1515,7 +1517,12 @@ todo_wine
     buffer[0] = 0;
     flags = 0;
     res = pInternetGetConnectedStateExA(&flags, buffer, 1, 0);
-todo_wine
+    ok(res == TRUE, "Expected TRUE, got %d\n", res);
+    ok(flags, "Expected at least one flag set\n");
+    ok(strlen(buffer) == 0, "Expected 0 bytes, got %u\n", lstrlenA(buffer));
+
+    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(strlen(buffer) == 0, "Expected 0 bytes, got %u\n", lstrlenA(buffer));
@@ -1540,12 +1547,10 @@ static void test_InternetGetConnectedStateExW(void)
     trace("Internet Connection: Flags 0x%02x - Name '%s'\n", flags, wine_dbgstr_w(buffer));
 
     res = pInternetGetConnectedStateExW(NULL, NULL, 0, 0);
-todo_wine
     ok(res == TRUE, "Expected TRUE, got %d\n", res);
 
     flags = 0;
     res = pInternetGetConnectedStateExW(&flags, NULL, 0, 0);
-todo_wine
     ok(res == TRUE, "Expected TRUE, got %d\n", res);
     ok(flags, "Expected at least one flag set\n");
 
@@ -1554,7 +1559,6 @@ todo_wine
     res = pInternetGetConnectedStateExW(&flags, buffer, 0, 0);
     ok(res == TRUE, "Expected TRUE, got %d\n", res);
     ok(flags, "Expected at least one flag set\n");
-todo_wine
     ok(!buffer[0], "Buffer must not change, got %02X\n", buffer[0]);
 
     buffer[0] = 0;
@@ -1571,6 +1575,11 @@ todo_wine
     sz = lstrlenW(buffer);
     ok(sz > 0, "Expected a connection name\n");
 
+    flags = 0;
+    res = pInternetGetConnectedStateExW(&flags, NULL, sizeof(buffer) / sizeof(buffer[0]), 0);
+    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 */
     buffer[0] = 0;
     flags = 0;
@@ -1582,7 +1591,12 @@ todo_wine
     buffer[0] = 0;
     flags = 0;
     res = pInternetGetConnectedStateExW(&flags, buffer, 1, 0);
-todo_wine
+    ok(res == TRUE, "Expected TRUE, got %d\n", res);
+    ok(flags, "Expected at least one flag set\n");
+    ok(lstrlenW(buffer) == 0, "Expected 0 bytes, got %u\n", lstrlenW(buffer));
+
+    buffer[0] = 0xDEAD;
+    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(lstrlenW(buffer) == 0, "Expected 0 bytes, got %u\n", lstrlenW(buffer));