improve handling of proxy environmental variables (#28088)

* improve handling of proxy environmental variables
* feedback from reviews
* update port parsing
* use Span instead of Substring to parse port
* fix more feedback

Author: wfurt

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

@@ -65,16 +65,26 @@ private static NetworkCredential GetCredentialsFromString(string value)
             {
                 return null;
             }
-            int idx = value.IndexOf(":");
-            if (idx < 0)
+
+            value = Uri.UnescapeDataString(value);
+
+            string password = "";
+            string domain = null;
+            int idx = value.IndexOf(':');
+            if (idx != -1)
             {
-                // only user name without password
-                return new NetworkCredential(value, "");
+                password = value.Substring(idx+1);
+                value = value.Substring(0, idx);
             }
-            else
+
+            idx = value.IndexOf('\\');
+            if (idx != -1)
             {
-                return new NetworkCredential(value.Substring(0, idx), value.Substring(idx+1, value.Length - idx -1 ));
+                domain = value.Substring(0, idx);
+                value = value.Substring(idx+1);
             }
+
+            return new NetworkCredential(value, password, domain);
         }
     }
 
@@ -168,18 +178,77 @@ private static Uri GetUriFromString(string value)
             {
                 return null;
             }
+            if (value.StartsWith("http://", StringComparison.OrdinalIgnoreCase))
+            {
+                value = value.Substring(7);
+            }
+
+            string user = null;
+            string password = null;
+            UInt16 port = 80;
+            string host = null;
+
+            // 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('@');
+            if (separatorIndex != -1)
+            {
+                string auth = value.Substring(0, separatorIndex);
+
+                // The User and password may or may not be URL encoded.
+                // Curl seems to accept both. To match that,
+                // we do opportunistic decode and we use original string if it fails.
+                try
+                {
+                    auth = Uri.UnescapeDataString(auth);
+                }
+                catch { };
+
+                value = value.Substring(separatorIndex + 1);
+                separatorIndex = auth.IndexOf(':');
+                if (separatorIndex == -1)
+                {
+                    user = auth;
+                }
+                else
+                {
+                    user = auth.Substring(0, separatorIndex);
+                    password = auth.Substring(separatorIndex + 1);
+                }
+            }
 
-            if (!value.Contains("://"))
+            int ipV6AddressEnd = value.IndexOf(']');
+            separatorIndex = value.LastIndexOf(':');
+            // No ':' or it is part of IPv6 address.
+            if (separatorIndex == -1 || (ipV6AddressEnd != -1 && separatorIndex < ipV6AddressEnd))
             {
-                value = "http://" + value;
+                host = value;
+            }
+            else
+            {
+                host = value.Substring(0, separatorIndex);
+                if (!UInt16.TryParse(value.AsSpan().Slice(separatorIndex + 1), out port))
+                {
+                    return null;
+                }
             }
 
-            if (Uri.TryCreate(value, UriKind.Absolute, out Uri uri) &&
-                    (uri.Scheme == Uri.UriSchemeHttp || uri.Scheme == Uri.UriSchemeHttps))
+            try
             {
-                // We only support http and https for now.
-                return uri;
+                UriBuilder ub = new UriBuilder("http", host, port);
+                if (user != null)
+                {
+                    ub.UserName = Uri.EscapeDataString(user);
+                }
+
+                if (password != null)
+                {
+                    ub.Password = Uri.EscapeDataString(password);
+                }
+
+                return ub.Uri;
             }
+            catch { };
             return null;
         }
 

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

@@ -101,6 +101,54 @@ public void HttpProxy_EnvironmentProxy_Loaded()
             }).Dispose();
         }
 
+        [Theory]
+        [InlineData("1.1.1.5", "1.1.1.5", "80", null, null)]
+        [InlineData("http://1.1.1.5:3005", "1.1.1.5", "3005", null, null)]
+        [InlineData("http://foo@1.1.1.5", "1.1.1.5", "80", "foo", "")]
+        [InlineData("http://[::1]:80", "[::1]", "80", null, null)]
+        [InlineData("foo:bar@[::1]:3128", "[::1]", "3128", "foo", "bar")]
+        [InlineData("foo:Pass$!#\\.$@127.0.0.1:3128", "127.0.0.1", "3128", "foo", "Pass$!#\\.$")]
+        [InlineData("[::1]", "[::1]", "80", null, null)]
+        [InlineData("domain\\foo:bar@1.1.1.1", "1.1.1.1", "80", "foo", "bar")]
+        [InlineData("domain%5Cfoo:bar@1.1.1.1", "1.1.1.1", "80", "foo", "bar")]
+        [InlineData("HTTP://ABC.COM/", "abc.com", "80", null, null)]
+        public void HttpProxy_Uri_Parsing(string _input, string _host, string _port, string _user , string _password)
+        {
+            RemoteInvoke((input, host, port, user, password) =>
+            {
+                // Remote exec does not allow to pass null at this moment.
+                if (user == "null")
+                {
+                    user = null;
+                }
+                if (password == "null")
+                {
+                    password = null;
+                }
+
+                Environment.SetEnvironmentVariable("all_proxy", input);
+                IWebProxy p;
+                Uri u;
+
+                Assert.True(HttpEnvironmentProxy.TryCreate(out p));
+                Assert.NotNull(p);
+
+                u = p.GetProxy(fooHttp);
+                Assert.Equal(host, u.Host);
+                Assert.Equal(Convert.ToInt32(port), u.Port);
+
+                if (user != null)
+                {
+                    NetworkCredential nc = p.Credentials.GetCredential(u, "Basic");
+                    Assert.NotNull(nc);
+                    Assert.Equal(user, nc.UserName);
+                    Assert.Equal(password, nc.Password);
+                }
+
+                return SuccessExitCode;
+            }, _input, _host, _port, _user ?? "null" , _password ?? "null").Dispose();
+        }
+
         [Fact]
         public void HttpProxy_CredentialParsing_Basic()
         {