Fix proxied ipv6 address request formatting and fix port handling for proxied IP address on SocketsHttpHandler (#28740)

* fix proxied ipv6 address request format and fix port handling for ip address

* address feedback

* change naming for test data

2 files changed, 103 insertions, 1 deletions

diff --git a/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs b/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs
index d2e8c02b04..82aa039ee1 100644
--- a/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs
+++ b/src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs
@@ -375,7 +375,18 @@ namespace System.Net.Http
                         // Proxied requests contain full URL
                         Debug.Assert(request.RequestUri.Scheme == Uri.UriSchemeHttp);
                         await WriteBytesAsync(s_httpSchemeAndDelimiter).ConfigureAwait(false);
-                        await WriteAsciiStringAsync(request.RequestUri.IdnHost).ConfigureAwait(false);
+
+                        // If the hostname is an IPv6 address, uri.IdnHost will return the address without enclosing [].
+                        // In this case, use uri.Host instead, which will correctly enclose with [].
+                        // Note we don't need punycode encoding if it's an IP address, so using uri.Host is fine.
+                        await WriteAsciiStringAsync(request.RequestUri.HostNameType == UriHostNameType.IPv6 ?
+                            request.RequestUri.Host : request.RequestUri.IdnHost).ConfigureAwait(false);
+
+                        if (!request.RequestUri.IsDefaultPort)
+                        {
+                            await WriteByteAsync((byte)':').ConfigureAwait(false);
+                            await WriteDecimalInt32Async(request.RequestUri.Port).ConfigureAwait(false);
+                        }
                     }
                     await WriteStringAsync(request.RequestUri.GetComponents(UriComponents.PathAndQuery | UriComponents.Fragment, UriFormat.UriEscaped)).ConfigureAwait(false);
                 }
diff --git a/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.cs b/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.cs
index 7ae31a8926..79048f937f 100644
--- a/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.cs
+++ b/src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.cs
@@ -31,6 +31,7 @@ namespace System.Net.Http.Functional.Tests
         private const string ExpectedContent = "Test content";
         private const string Username = "testuser";
         private const string Password = "password";
+        private const string HttpDefaultPort = "80";
 
         private readonly NetworkCredential _credential = new NetworkCredential(Username, Password);
 
@@ -498,6 +499,96 @@ namespace System.Net.Http.Functional.Tests
             Assert.True(connectionAccepted);
         }
 
+        [Theory]
+        [InlineData("1.2.3.4")]
+        [InlineData("1.2.3.4:8080")]
+        [InlineData("[::1234]")]
+        [InlineData("[::1234]:8080")]
+        public async Task ProxiedIPAddressRequest_NotDefaultPort_CorrectlyFormatted(string host)
+        {
+            string uri = "http://" + host;
+            bool connectionAccepted = false;
+
+            await LoopbackServer.CreateClientAndServerAsync(async proxyUri =>
+            {
+                using (HttpClientHandler handler = CreateHttpClientHandler())
+                using (var client = new HttpClient(handler))
+                {
+                    handler.Proxy = new WebProxy(proxyUri);
+                    try { await client.GetAsync(uri); } catch { }
+                }
+            }, server => server.AcceptConnectionAsync(async connection =>
+            {
+                connectionAccepted = true;
+                List<string> headers = await connection.ReadRequestHeaderAndSendResponseAsync();
+                Assert.Contains($"GET {uri}/ HTTP/1.1", headers);
+            }));
+
+            Assert.True(connectionAccepted);
+        }
+
+        public static IEnumerable<object[]> DestinationHost_MemberData()
+        {
+            yield return new object[] { Configuration.Http.Host };
+            yield return new object[] { "1.2.3.4" };
+            yield return new object[] { "[::1234]" };
+        }
+
+        [Theory]
+        [OuterLoop] // Test uses azure endpoint.
+        [MemberData(nameof(DestinationHost_MemberData))]
+        public async Task ProxiedRequest_DefaultPort_PortStrippedOffInUri(string host)
+        {
+            string addressUri = $"http://{host}:{HttpDefaultPort}/";
+            string expectedAddressUri = $"http://{host}/";
+            bool connectionAccepted = false;
+
+            await LoopbackServer.CreateClientAndServerAsync(async proxyUri =>
+            {
+                using (HttpClientHandler handler = CreateHttpClientHandler())
+                using (var client = new HttpClient(handler))
+                {
+                    handler.Proxy = new WebProxy(proxyUri);
+                    try { await client.GetAsync(addressUri); } catch { }
+                }
+            }, server => server.AcceptConnectionAsync(async connection =>
+            {
+                connectionAccepted = true;
+                List<string> headers = await connection.ReadRequestHeaderAndSendResponseAsync();
+                Assert.Contains($"GET {expectedAddressUri} HTTP/1.1", headers);
+            }));
+
+            Assert.True(connectionAccepted);
+        }
+
+        [Fact]
+        [OuterLoop] // Test uses azure endpoint.
+        public async Task ProxyTunnelRequest_PortSpecified_NotStrippedOffInUri()
+        {
+            // Https proxy request will use CONNECT tunnel, even the default 443 port is specified, it will not be stripped off.
+            string requestTarget = $"{Configuration.Http.SecureHost}:443";
+            string addressUri = $"https://{requestTarget}/";
+            bool connectionAccepted = false;
+
+            await LoopbackServer.CreateClientAndServerAsync(async proxyUri =>
+            {
+                using (HttpClientHandler handler = CreateHttpClientHandler())
+                using (var client = new HttpClient(handler))
+                {
+                    handler.Proxy = new WebProxy(proxyUri);
+                    handler.ServerCertificateCustomValidationCallback = TestHelper.AllowAllCertificates;
+                    try { await client.GetAsync(addressUri); } catch { }
+                }
+            }, server => server.AcceptConnectionAsync(async connection =>
+            {
+                connectionAccepted = true;
+                List<string> headers = await connection.ReadRequestHeaderAndSendResponseAsync();
+                Assert.Contains($"CONNECT {requestTarget} HTTP/1.1", headers);
+            }));
+
+            Assert.True(connectionAccepted);
+        }
+
         public static IEnumerable<object[]> SecureAndNonSecure_IPBasedUri_MemberData() =>
             from address in new[] { IPAddress.Loopback, IPAddress.IPv6Loopback }
             from useSsl in new[] { true, false }