Improve HttpEnvironmentProxy uri parsing

Author: MihaZupan

src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpEnvironmentProxy.cs

@@ -1,11 +1,6 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
-using System;
-using System.Collections.Generic;
-using System.Net;
-using System.Net.Http;
-
 namespace System.Net.Http
 {
     internal sealed class HttpEnvironmentProxyCredentials : ICredentials
@@ -115,10 +110,11 @@ private HttpEnvironmentProxy(Uri? httpProxy, Uri? httpsProxy, string? bypassList
         }
 
         /// <summary>
-        /// This function will evaluate given string and it will try to convert
-        /// it to Uri object. The string could contain URI fragment, IP address and  port
-        /// tuple or just IP address or name. It will return null if parsing fails.
+        /// Attempt to parse a partial Uri string into a Uri object.
+        /// The string may contain the scheme, user info, host, and port. The host is the only required part.
+        /// Example expected inputs: contoso.com, contoso.com:8080, http://contoso.com/, [email protected].
         /// </summary>
+        /// <returns><see langword="null"/> if parsing fails.</returns>
         private static Uri? GetUriFromString(string? value)
         {
             if (string.IsNullOrEmpty(value))
@@ -167,7 +163,7 @@ private HttpEnvironmentProxy(Uri? httpProxy, Uri? httpsProxy, string? bypassList
 
             // Check if there is authentication part with user and possibly password.
             // Curl accepts raw text and that may break URI parser.
-            int separatorIndex = value.LastIndexOf('@');
+            int separatorIndex = value.IndexOf('@');
             if (separatorIndex != -1)
             {
                 // The User and password may or may not be URL encoded.
@@ -187,6 +183,14 @@ private HttpEnvironmentProxy(Uri? httpProxy, Uri? httpsProxy, string? bypassList
                 }
             }
 
+            // We expect inputs to not contain the path/query/fragment, but we do handle some simple cases like a trailing slash.
+            // An arbitrary path can break this parsing logic, but we expect environment variables to be trusted and well formed.
+            int delimiterIndex = value.IndexOfAny('/', '?', '#');
+            if (delimiterIndex >= 0)
+            {
+                value = value.Substring(0, delimiterIndex);
+            }
+
             int ipV6AddressEnd = value.IndexOf(']');
             separatorIndex = value.LastIndexOf(':');
             // No ':' or it is part of IPv6 address.
@@ -197,18 +201,8 @@ private HttpEnvironmentProxy(Uri? httpProxy, Uri? httpsProxy, string? bypassList
             else
             {
                 host = value.Substring(0, separatorIndex);
-                int endIndex = separatorIndex + 1;
-                // Strip any trailing characters after port number.
-                while (endIndex < value.Length)
-                {
-                    if (!char.IsDigit(value[endIndex]))
-                    {
-                        break;
-                    }
-                    endIndex += 1;
-                }
 
-                if (!ushort.TryParse(value.AsSpan(separatorIndex + 1, endIndex - separatorIndex - 1), out port))
+                if (!ushort.TryParse(value.AsSpan(separatorIndex + 1), out port))
                 {
                     return null;
                 }
@@ -261,8 +255,7 @@ private bool IsMatchInBypassList(Uri input)
                     {
                         // This should match either domain it self or any subdomain or host
                         // .foo.com will match foo.com it self or *.foo.com
-                        if ((s.Length - 1) == input.Host.Length &&
-                            string.Compare(s, 1, input.Host, 0, input.Host.Length, StringComparison.OrdinalIgnoreCase) == 0)
+                        if (s.AsSpan(1).Equals(input.Host, StringComparison.OrdinalIgnoreCase))
                         {
                             return true;
                         }

src/libraries/System.Net.Http/tests/UnitTests/HttpEnvironmentProxyTest.cs

@@ -137,8 +137,13 @@ await RemoteExecutor.Invoke(() =>
         [InlineData("[::1]", "[::1]", "80", null, null)]
         [InlineData("domain\\foo:[email protected]", "1.1.1.1", "80", "foo", "PLACEHOLDER")]
         [InlineData("domain%5Cfoo:[email protected]", "1.1.1.1", "80", "foo", "PLACEHOLDER")]
+        [InlineData("[email protected]/foo@", "1.2.3.4", "80", "placeholder", "")]
+        [InlineData("placeholder:@1.2.3.4/foo@:", "1.2.3.4", "80", "placeholder", "")]
         [InlineData("HTTP://ABC.COM/", "abc.com", "80", null, null)]
         [InlineData("http://10.30.62.64:7890/", "10.30.62.64", "7890", null, null)]
+        [InlineData("http://1.2.3.4/foo", "1.2.3.4", "80", null, null)]
+        [InlineData("http://1.2.3.4/foo:", "1.2.3.4", "80", null, null)]
+        [InlineData("http://1.2.3.4?foo", "1.2.3.4", "80", null, null)]
         [InlineData("http://1.2.3.4:8888/foo", "1.2.3.4", "8888", null, null)]
         [InlineData("socks4://1.2.3.4:8888/foo", "1.2.3.4", "8888", null, null)]
         [InlineData("socks4a://1.2.3.4:8888/foo", "1.2.3.4", "8888", null, null)]