Improve parsing of WinINet default proxy strings in SocketsHttpHandler (#28671)

This is a partial fix for #28603.

Author: rmkerr

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

@@ -13,14 +13,16 @@ namespace System.Net.Http
 {
     internal sealed class HttpSystemProxy : IWebProxy, IDisposable
     {
-        private readonly Uri _proxyUri;                 // URI of the system proxy if set
+        private readonly Uri _insecureProxyUri;         // URI of the http system proxy if set
+        private readonly Uri _secureProxyUri;         // URI of the https system proxy if set
         private readonly List<Regex> _bypass;           // list of domains not to proxy
         private readonly bool _bypassLocal = false;     // we should bypass domain considered local
         private readonly List<IPAddress> _localIp;
         private ICredentials _credentials;
         private readonly WinInetProxyHelper _proxyHelper;
         private SafeWinHttpHandle _sessionHandle;
         private bool _disposed;
+        private static readonly char[] s_proxyDelimiters = {';', ' ', '\n', '\r', '\t'};
 
         public static bool TryCreate(out IWebProxy proxy)
         {
@@ -62,7 +64,7 @@ private HttpSystemProxy(WinInetProxyHelper proxyHelper, SafeWinHttpHandle sessio
 
             if (proxyHelper.ManualSettingsOnly)
             {
-                _proxyUri = GetUriFromString(proxyHelper.Proxy);
+                ParseProxyConfig(proxyHelper.Proxy, out _insecureProxyUri, out _secureProxyUri);
 
                 if (!string.IsNullOrWhiteSpace(proxyHelper.ProxyBypass))
                 {
@@ -178,7 +180,7 @@ public void Dispose()
 
         /// <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
+        /// it to a 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.
         /// </summary>
         private static Uri GetUriFromString(string value)
@@ -202,6 +204,64 @@ private static Uri GetUriFromString(string value)
             return null;
         }
 
+        /// <summary>
+        /// This function is used to parse WinINet Proxy strings. The strings are a semicolon
+        /// or whitespace separated list, with each entry in the following format:
+        /// ([<scheme>=][<scheme>"://"]<server>[":"<port>])
+        /// </summary>
+        private static void ParseProxyConfig(string value, out Uri insecureProxy, out Uri secureProxy )
+        {
+            secureProxy = null;
+            insecureProxy = null;
+            if (string.IsNullOrEmpty(value))
+            {
+                return;
+            }
+
+            int idx = value.IndexOf("http://");
+            if (idx >= 0)
+            {
+                int proxyLength = GetProxySubstringLength(value, idx);
+                Uri.TryCreate(value.Substring(idx, proxyLength) , UriKind.Absolute, out insecureProxy);
+            }
+
+            if (insecureProxy == null)
+            {
+                idx = value.IndexOf("http=");
+                if (idx >= 0)
+                {
+                    idx += 5; // Skip "http=" so we can replace it with "http://"
+                    int proxyLength = GetProxySubstringLength(value, idx);
+                    Uri.TryCreate("http://" + value.Substring(idx, proxyLength) , UriKind.Absolute, out insecureProxy);
+                }
+            }
+
+            idx = value.IndexOf("https://");
+            if (idx >= 0)
+            {
+                idx += 8; // Skip "https://" so we can replace it with "http://"
+                int proxyLength = GetProxySubstringLength(value, idx);
+                Uri.TryCreate("http://" + value.Substring(idx, proxyLength) , UriKind.Absolute, out secureProxy);
+            }
+
+            if (secureProxy == null)
+            {
+                idx = value.IndexOf("https=");
+                if (idx >= 0)
+                {
+                    idx += 6; // Skip "https=" so we can replace it with "http://"
+                    int proxyLength = GetProxySubstringLength(value, idx);
+                    Uri.TryCreate("http://" + value.Substring(idx, proxyLength) , UriKind.Absolute, out secureProxy);
+                }
+            }
+        }
+
+        private static int GetProxySubstringLength(String proxyString, int idx)
+        {
+            int endOfProxy = proxyString.IndexOfAny(s_proxyDelimiters, idx);
+            return (endOfProxy == -1) ? proxyString.Length - idx : endOfProxy - idx;
+        }
+
         /// <summary>
         /// Gets the proxy URI. (IWebProxy interface)
         /// </summary>
@@ -261,7 +321,7 @@ public Uri GetProxy(Uri uri)
                 }
 
                 // We did not find match on bypass list.
-                return _proxyUri;
+                return (uri.Scheme == UriScheme.Https || uri.Scheme == UriScheme.Wss) ? _secureProxyUri : _insecureProxyUri;
             }
 
             // For anything else ask WinHTTP.

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

@@ -15,29 +15,53 @@ public class HttpSystemProxyTest : RemoteExecutorTestBase
     {
         private readonly ITestOutputHelper _output;
         private const string FakeProxyString = "http://proxy.contoso.com";
-        private readonly Uri fakeProxyUri = new Uri("http://proxy.contoso.com");
-        private readonly Uri fooHttp = new Uri("http://foo.com");
-        private readonly Uri fooHttps = new Uri("https://foo.com");
+        private const string insecureProxyUri = "http://proxy.insecure.com";
+        private const string secureProxyUri = "http://proxy.secure.com";
+        private const string fooHttp = "http://foo.com";
+        private const string fooHttps = "https://foo.com";
+        private const string fooWs = "ws://foo.com";
+        private const string fooWss = "wss://foo.com";
 
         public HttpSystemProxyTest(ITestOutputHelper output)
         {
             _output = output;
         }
 
-        [Fact]
-        public void HttpProxy_SystemProxy_Loaded()
+        [Theory]
+        [InlineData("http://proxy.insecure.com", true, false)]
+        [InlineData("http=proxy.insecure.com", true, false)]
+        [InlineData("http://proxy.insecure.com http://proxy.wrong.com", true, false)]
+        [InlineData("https=proxy.secure.com http=proxy.insecure.com", true, true)]
+        [InlineData("https://proxy.secure.com\nhttp://proxy.insecure.com", true, true)]
+        [InlineData("https=proxy.secure.com\nhttp=proxy.insecure.com", true, true)]
+        [InlineData("https://proxy.secure.com;http://proxy.insecure.com", true, true)]
+        [InlineData("https=proxy.secure.com;http=proxy.insecure.com", true, true)]
+        [InlineData(";http=proxy.insecure.com;;", true, false)]
+        [InlineData("    http=proxy.insecure.com    ", true, false)]
+        [InlineData("http=proxy.insecure.com;http=proxy.wrong.com", true, false)]
+        [InlineData("http=http://proxy.insecure.com", true, false)]
+        [InlineData("https=https://proxy.secure.com", false, true)]
+        public void HttpProxy_SystemProxy_Loaded(string rawProxyString, bool hasInsecureProxy, bool hasSecureProxy)
         {
-            IWebProxy p;
+            RemoteInvoke((proxyString, insecureProxy, secureProxy) =>
+            {
+                IWebProxy p;
 
-            FakeRegistry.Reset();
-            Assert.False(HttpSystemProxy.TryCreate(out p));
+                FakeRegistry.Reset();
+                Assert.False(HttpSystemProxy.TryCreate(out p));
 
-            FakeRegistry.WinInetProxySettings.Proxy = FakeProxyString;
+                FakeRegistry.WinInetProxySettings.Proxy = proxyString;
+                WinInetProxyHelper proxyHelper = new WinInetProxyHelper();
 
-            Assert.True(HttpSystemProxy.TryCreate(out p));
-            Assert.NotNull(p);
-            Assert.Equal(fakeProxyUri, p.GetProxy(fooHttp));
-            Assert.Equal(fakeProxyUri, p.GetProxy(fooHttps));
+                Assert.True(HttpSystemProxy.TryCreate(out p));
+                Assert.NotNull(p);
+
+                Assert.Equal(Boolean.Parse(insecureProxy) ? new Uri(insecureProxyUri) : null, p.GetProxy(new Uri(fooHttp)));
+                Assert.Equal(Boolean.Parse(secureProxy) ? new Uri(secureProxyUri) : null, p.GetProxy(new Uri(fooHttps)));
+                Assert.Equal(Boolean.Parse(insecureProxy) ? new Uri(insecureProxyUri) : null, p.GetProxy(new Uri(fooWs)));
+                Assert.Equal(Boolean.Parse(secureProxy) ? new Uri(secureProxyUri) : null, p.GetProxy(new Uri(fooWss)));
+                return SuccessExitCode;
+            }, rawProxyString, hasInsecureProxy.ToString(), hasSecureProxy.ToString()).Dispose();
         }
 
         [Theory]
@@ -66,7 +90,7 @@ public void HttpProxy_Local_Bypassed(string name, bool shouldBypass)
                 IWebProxy p;
 
                 FakeRegistry.Reset();
-                FakeRegistry.WinInetProxySettings.Proxy = FakeProxyString;
+                FakeRegistry.WinInetProxySettings.Proxy = insecureProxyUri;
                 FakeRegistry.WinInetProxySettings.ProxyBypass = "23.23.86.44;*.foo.com;<local>;BAR.COM; ; 162*;[2002::11];[*:f8b0:4005:80a::200e]; http://www.xn--mnchhausen-9db.at;http://*.xn--bb-bjab.eu;http://xn--bb-bjab.eu;";
 
                 Assert.True(HttpSystemProxy.TryCreate(out p));
@@ -93,7 +117,7 @@ public void HttpProxy_Local_Parsing(string bypass, int count)
                 IWebProxy p;
 
                 FakeRegistry.Reset();
-                FakeRegistry.WinInetProxySettings.Proxy = FakeProxyString;
+                FakeRegistry.WinInetProxySettings.Proxy = insecureProxyUri;
                 FakeRegistry.WinInetProxySettings.ProxyBypass = bypassValue;
 
                 Assert.True(HttpSystemProxy.TryCreate(out p));
@@ -112,7 +136,36 @@ public void HttpProxy_Local_Parsing(string bypass, int count)
                 }
                 return SuccessExitCode;
            }, bypass, count.ToString()).Dispose();
+        }
 
+        [Theory]
+        [InlineData("http://")]
+        [InlineData("http=")]
+        [InlineData("http://;")]
+        [InlineData("http=;")]
+        [InlineData("  ;  ")]
+        [InlineData("proxy.contoso.com")]
+        public void HttpProxy_InvalidSystemProxy_Null(string rawProxyString)
+        {
+            RemoteInvoke((proxyString) =>
+            {
+                IWebProxy p;
+
+                FakeRegistry.Reset();
+                Assert.False(HttpSystemProxy.TryCreate(out p));
+
+                FakeRegistry.WinInetProxySettings.Proxy = proxyString;
+                WinInetProxyHelper proxyHelper = new WinInetProxyHelper();
+
+                Assert.True(HttpSystemProxy.TryCreate(out p));
+                Assert.NotNull(p);
+
+                Assert.Equal(null, p.GetProxy(new Uri(fooHttp)));
+                Assert.Equal(null, p.GetProxy(new Uri(fooHttps)));
+                Assert.Equal(null, p.GetProxy(new Uri(fooWs)));
+                Assert.Equal(null, p.GetProxy(new Uri(fooWss)));
+                return SuccessExitCode;
+            }, rawProxyString).Dispose();
         }
     }
 }