Fixing proxy port bug when using Win32 path (#903)

jasonsandlin · web-flow · commit 2868d32f9452 · 2025-08-25T10:52:14.000-07:00
* Fixing proxy port bug when using Win32 path

* PR feedback

* Improving log

* PR feedback

* PR feedback
diff --git a/Build/libHttpClient.GDK.Shared/libHttpClient.GDK.Shared.vcxitems b/Build/libHttpClient.GDK.Shared/libHttpClient.GDK.Shared.vcxitems
@@ -21,6 +21,7 @@
     <ClCompile Include="$(MSBuildThisFileDirectory)..\..\Source\HTTP\Curl\CurlProvider.cpp" />
     <ClCompile Include="$(MSBuildThisFileDirectory)..\..\Source\HTTP\WinHttp\winhttp_connection.cpp" />
     <ClCompile Include="$(MSBuildThisFileDirectory)..\..\Source\HTTP\WinHttp\winhttp_provider.cpp" />
+    <ClCompile Include="$(MSBuildThisFileDirectory)..\..\Source\HTTP\WinHttp\winhttp_proxy_helper.cpp" />
     <ClCompile Include="$(MSBuildThisFileDirectory)..\..\Source\Platform\GDK\PlatformComponents_GDK.cpp" />
     <ClCompile Include="$(MSBuildThisFileDirectory)..\..\Source\Platform\Windows\PlatformTrace_Windows.cpp" />
   </ItemGroup>
diff --git a/Build/libHttpClient.GDK.Shared/libHttpClient.GDK.Shared.vcxitems.filters b/Build/libHttpClient.GDK.Shared/libHttpClient.GDK.Shared.vcxitems.filters
@@ -60,6 +60,7 @@
     <ClCompile Include="$(MSBuildThisFileDirectory)..\..\Source\HTTP\Curl\CurlDynamicLoader.cpp">
       <Filter>Source\HTTP\Curl</Filter>
     </ClCompile>
+    <ClCompile Include="$(MSBuildThisFileDirectory)..\..\Source\HTTP\WinHttp\winhttp_proxy_helper.cpp" />
   </ItemGroup>
   <ItemGroup>
     <ClInclude Include="$(MSBuildThisFileDirectory)..\..\Source\Common\Win\utils_win.h">
diff --git a/Build/libHttpClient.UnitTest/libHttpClient.UnitTest.vcxitems b/Build/libHttpClient.UnitTest/libHttpClient.UnitTest.vcxitems
@@ -36,5 +36,7 @@
     <ClCompile Include="$(MSBuildThisFileDirectory)..\..\Tests\UnitTests\Tests\MockTests.cpp" />
     <ClCompile Include="$(MSBuildThisFileDirectory)..\..\Tests\UnitTests\Tests\TaskQueueTests.cpp" />
     <ClCompile Include="$(MSBuildThisFileDirectory)..\..\Tests\UnitTests\Tests\WebsocketTests.cpp" />
+    <ClCompile Include="$(MSBuildThisFileDirectory)..\..\Tests\UnitTests\Tests\ProxyTests.cpp" />
+  <ClCompile Include="$(MSBuildThisFileDirectory)..\..\Source\HTTP\WinHttp\winhttp_proxy_helper.cpp" />
   </ItemGroup>
 </Project>
diff --git a/Build/libHttpClient.UnitTest/libHttpClient.UnitTest.vcxitems.filters b/Build/libHttpClient.UnitTest/libHttpClient.UnitTest.vcxitems.filters
@@ -92,5 +92,7 @@
     <ClCompile Include="$(MSBuildThisFileDirectory)..\..\Source\Platform\Windows\PlatformTrace_Windows.cpp">
       <Filter>Source\Platform\Windows</Filter>
     </ClCompile>
+    <ClCompile Include="$(MSBuildThisFileDirectory)..\..\Tests\UnitTests\Tests\ProxyTests.cpp" />
+    <ClCompile Include="$(MSBuildThisFileDirectory)..\..\Source\HTTP\WinHttp\winhttp_proxy_helper.cpp" />
   </ItemGroup>
 </Project>
diff --git a/Build/libHttpClient.Win32.Shared/libHttpClient.Win32.Shared.vcxitems b/Build/libHttpClient.Win32.Shared/libHttpClient.Win32.Shared.vcxitems
@@ -13,6 +13,7 @@
     <ClCompile Include="$(MSBuildThisFileDirectory)..\..\Source\Common\Win\utils_win.cpp" />
     <ClCompile Include="$(MSBuildThisFileDirectory)..\..\Source\HTTP\WinHttp\winhttp_connection.cpp" />
     <ClCompile Include="$(MSBuildThisFileDirectory)..\..\Source\HTTP\WinHttp\winhttp_provider.cpp" />
+    <ClCompile Include="$(MSBuildThisFileDirectory)..\..\Source\HTTP\WinHttp\winhttp_proxy_helper.cpp" />
     <ClCompile Include="$(MSBuildThisFileDirectory)..\..\Source\Platform\Win32\PlatformComponents_Win32.cpp" />
     <ClCompile Include="$(MSBuildThisFileDirectory)..\..\Source\Platform\Windows\PlatformTrace_Windows.cpp" />
     <ClCompile Include="$(MSBuildThisFileDirectory)..\..\Source\Task\ThreadPool_win32.cpp" />
diff --git a/Build/libHttpClient.Win32.Shared/libHttpClient.Win32.Shared.vcxitems.filters b/Build/libHttpClient.Win32.Shared/libHttpClient.Win32.Shared.vcxitems.filters
@@ -51,6 +51,7 @@
     <ClCompile Include="$(MSBuildThisFileDirectory)..\..\Source\Platform\Windows\PlatformTrace_Windows.cpp">
       <Filter>Source\Platform\Windows</Filter>
     </ClCompile>
+    <ClCompile Include="$(MSBuildThisFileDirectory)..\..\Source\HTTP\WinHttp\winhttp_proxy_helper.cpp" />
   </ItemGroup>
   <ItemGroup>
     <ClInclude Include="$(MSBuildThisFileDirectory)..\..\Source\Common\Win\utils_win.h">
diff --git a/Source/HTTP/WinHttp/winhttp_provider.cpp b/Source/HTTP/WinHttp/winhttp_provider.cpp
@@ -473,28 +473,7 @@ HRESULT WinHttpProvider::GetProxyName(
     case proxy_type::named_proxy:
     {
         pAccessType = WINHTTP_ACCESS_TYPE_NAMED_PROXY;
-
-        http_internal_wstring wProxyHost = utf16_from_utf8(proxyUri.Host());
-
-        // WinHttpOpen cannot handle trailing slash in the name, so here is some string gymnastics to keep WinHttpOpen happy
-        if (proxyUri.IsPortDefault())
-        {
-            pwProxyName = wProxyHost;
-        }
-        else
-        {
-            if (proxyUri.Port() > 0)
-            {
-                http_internal_basic_stringstream<wchar_t> ss;
-                ss.imbue(std::locale::classic());
-                ss << wProxyHost << L":" << proxyUri.Port();
-                pwProxyName = ss.str().c_str();
-            }
-            else
-            {
-                pwProxyName = wProxyHost;
-            }
-        }
+        pwProxyName = WinHttpProvider::BuildNamedProxyString(proxyUri);
         break;
     }
 
@@ -518,6 +497,7 @@ HRESULT WinHttpProvider::GetProxyName(
     return S_OK;
 }
 
+
 #if HC_PLATFORM == HC_PLATFORM_GDK
 
 void WinHttpProvider::Suspend()
diff --git a/Source/HTTP/WinHttp/winhttp_provider.h b/Source/HTTP/WinHttp/winhttp_provider.h
@@ -74,6 +74,9 @@ class WinHttpProvider
         _In_ String const& proxyUri
     ) noexcept;
 
+    // Public helper for building a proxy name (host[:port]) used by tests and implementation.
+    static http_internal_wstring BuildNamedProxyString(_In_ const xbox::httpclient::Uri& proxyUri);
+
 #ifndef HC_NOWEBSOCKETS
 public: // IWebSocketProvider
     HRESULT ConnectAsync(
@@ -187,7 +190,4 @@ class WinHttp_WebSocketProvider : public IWebSocketProvider
     SharedPtr<WinHttpProvider> const WinHttpProvider;
 };
 #endif
-
-
-
 NAMESPACE_XBOX_HTTP_CLIENT_END
diff --git a/Source/HTTP/WinHttp/winhttp_proxy_helper.cpp b/Source/HTTP/WinHttp/winhttp_proxy_helper.cpp
@@ -0,0 +1,24 @@
+#include "pch.h"
+#include "winhttp_provider.h"
+
+NAMESPACE_XBOX_HTTP_CLIENT_BEGIN
+
+http_internal_wstring WinHttpProvider::BuildNamedProxyString(_In_ const xbox::httpclient::Uri &proxyUri)
+{
+    http_internal_wstring wProxyHost = utf16_from_utf8(proxyUri.Host());
+    if (proxyUri.IsPortDefault())
+    {
+        return wProxyHost;
+    }
+    if (proxyUri.Port() > 0)
+    {
+        auto portStr = std::to_wstring(proxyUri.Port());
+        http_internal_wstring result = wProxyHost;
+        result.push_back(L':');
+        result.append(portStr.c_str());
+        return result;
+    }
+    return wProxyHost;
+}
+
+NAMESPACE_XBOX_HTTP_CLIENT_END
diff --git a/Source/Platform/GDK/PlatformComponents_GDK.cpp b/Source/Platform/GDK/PlatformComponents_GDK.cpp
@@ -55,9 +55,9 @@ HRESULT PlatformInitialize(PlatformComponents& components, HCInitArgs* initArgs)
     }
     else
     {
-        HC_TRACE_INFORMATION(HTTPCLIENT, "PlatformInitialize: Detected non-console platform (e.g., Steam Deck), using WinHTTP for HTTP");
+        HC_TRACE_INFORMATION(HTTPCLIENT, "PlatformInitialize: Detected non-console platform. Using WinHTTP for HTTP");
         
-        // Use WinHTTP for Steam Deck and other non-console platforms
+        // Use WinHTTP for non-console platforms
         auto initWinHttpResult = WinHttpProvider::Initialize();
         RETURN_IF_FAILED(initWinHttpResult.hr);
 
diff --git a/Tests/UnitTests/Tests/ProxyTests.cpp b/Tests/UnitTests/Tests/ProxyTests.cpp
@@ -0,0 +1,80 @@
+// Copyright (c) Microsoft Corporation
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+#include "pch.h"
+#include "UnitTestIncludes.h"
+#define TEST_CLASS_OWNER L"libHttpClient"
+#include "DefineTestMacros.h"
+#include "Utils.h"
+#include "../Common/Win/utils_win.h"
+#include "../../Source/HTTP/WinHttp/winhttp_provider.h"
+
+using namespace xbox::httpclient;
+
+NAMESPACE_XBOX_HTTP_CLIENT_TEST_BEGIN
+
+DEFINE_TEST_CLASS(ProxyTests)
+{
+public:
+    DEFINE_TEST_CLASS_PROPS(ProxyTests);
+
+    DEFINE_TEST_CASE(NamedProxyPortFormatting)
+    {
+        DEFINE_TEST_CASE_PROPERTIES(NamedProxyPortFormatting);
+        // Verify BuildNamedProxyString includes the numeric port (regression guard for prior bug treating uint16_t as a single wide char).
+        xbox::httpclient::Uri uri{"http://127.0.0.1:8888"};
+        auto proxyName = WinHttpProvider::BuildNamedProxyString(uri);
+        VERIFY_ARE_EQUAL_STR(L"127.0.0.1:8888", proxyName.c_str());
+    }
+
+    DEFINE_TEST_CASE(NamedProxyNoPort)
+    {
+        DEFINE_TEST_CASE_PROPERTIES(NamedProxyNoPort);
+        xbox::httpclient::Uri uri{"http://localhost"};
+        auto proxyName = WinHttpProvider::BuildNamedProxyString(uri);
+        VERIFY_ARE_EQUAL_STR(L"localhost", proxyName.c_str());
+    }
+
+    DEFINE_TEST_CASE(NamedProxyExplicitDefaultPort)
+    {
+        DEFINE_TEST_CASE_PROPERTIES(NamedProxyExplicitDefaultPort);
+        // Explicit default port (http://example.com:80) is preserved by Uri parsing (Port()==80 not treated as default)
+        xbox::httpclient::Uri uri{"http://example.com:80"};
+        auto proxyName = WinHttpProvider::BuildNamedProxyString(uri);
+        VERIFY_ARE_EQUAL_STR(L"example.com:80", proxyName.c_str());
+
+        // Verify no default port works
+        xbox::httpclient::Uri uri2{"http://example.com"};
+        auto proxyName2 = WinHttpProvider::BuildNamedProxyString(uri2);
+        VERIFY_ARE_EQUAL_STR(L"example.com", proxyName2.c_str());
+    }
+
+    DEFINE_TEST_CASE(NamedProxyIPv6LiteralWithPort)
+    {
+        DEFINE_TEST_CASE_PROPERTIES(NamedProxyIPv6LiteralWithPort);
+        // Uri class should normalize; host for IPv6 literal typically without brackets when accessed via Host()
+        xbox::httpclient::Uri uri{"http://[2001:db8::1]:3128"};
+        auto proxyName = WinHttpProvider::BuildNamedProxyString(uri);
+        // Expect host + :port (no brackets re-added by BuildNamedProxyString)
+        // If Uri::Host() preserves brackets, adjust expected accordingly; we detect by checking first char
+        auto hostUtf16 = utf16_from_utf8(uri.Host());
+        http_internal_wstring expected = hostUtf16;
+        if (!uri.IsPortDefault() && uri.Port() > 0)
+        {
+            expected.push_back(L':');
+            expected.append(std::to_wstring(uri.Port()));
+        }
+        VERIFY_ARE_EQUAL_STR(expected.c_str(), proxyName.c_str());
+    }
+
+    DEFINE_TEST_CASE(NamedProxyUnicodeHost)
+    {
+        DEFINE_TEST_CASE_PROPERTIES(NamedProxyUnicodeHost);
+        // Internationalized domain name in punycode should round-trip; here we just ensure it's copied
+        xbox::httpclient::Uri uri{"http://xn--bcher-kva.example:8080"}; // bücher.example punycode label
+        auto proxyName = WinHttpProvider::BuildNamedProxyString(uri);
+        VERIFY_ARE_EQUAL_STR(L"xn--bcher-kva.example:8080", proxyName.c_str());
+    }
+};
+
+NAMESPACE_XBOX_HTTP_CLIENT_TEST_END

Original file line number	Diff line number	Diff line change
`@@ -55,9 +55,9 @@ HRESULT PlatformInitialize(PlatformComponents& components, HCInitArgs* initArgs)`
`55`	`55`	`}`
`56`	`56`	`else`
`57`	`57`	`{`
`58`		`- HC_TRACE_INFORMATION(HTTPCLIENT, "PlatformInitialize: Detected non-console platform (e.g., Steam Deck), using WinHTTP for HTTP");`
	`58`	`+ HC_TRACE_INFORMATION(HTTPCLIENT, "PlatformInitialize: Detected non-console platform. Using WinHTTP for HTTP");`
`59`	`59`
`60`		`- // Use WinHTTP for Steam Deck and other non-console platforms`
	`60`	`+ // Use WinHTTP for non-console platforms`
`61`	`61`	`auto initWinHttpResult = WinHttpProvider::Initialize();`
`62`	`62`	`RETURN_IF_FAILED(initWinHttpResult.hr);`
`63`	`63`