Read WinHTTP's proxy format correctly (dotnet/corefx#39371)
authorCory Nelson <phrosty@gmail.com>
Fri, 12 Jul 2019 16:58:34 +0000 (09:58 -0700)
committerGitHub <noreply@github.com>
Fri, 12 Jul 2019 16:58:34 +0000 (09:58 -0700)
Fix SocketsHttpHandler to properly read WinHTTP's proxy string format, preventing an exception when a PAC script returns multiple proxies. Resolves dotnet/corefx#39104.

Commit migrated from https://github.com/dotnet/corefx/commit/70b45baaf36eab34fcfb44056b030654dba9a4dc

src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpWindowsProxy.cs
src/libraries/System.Net.Http/tests/UnitTests/HttpWindowsProxyTest.cs

index 68d1034..41a0d41 100644 (file)
@@ -68,11 +68,6 @@ namespace System.Net.Http
             {
                 if (NetEventSource.IsEnabled) NetEventSource.Info(proxyHelper, $"ManualSettingsUsed, {proxyHelper.Proxy}");
                 ParseProxyConfig(proxyHelper.Proxy, out _insecureProxyUri, out _secureProxyUri);
-                if (_insecureProxyUri == null && _secureProxyUri == null)
-                {
-                    // If advanced parsing by protocol fails, fall-back to simplified parsing.
-                    _insecureProxyUri = _secureProxyUri = GetUriFromString(proxyHelper.Proxy);
-                }
 
                 if (!string.IsNullOrWhiteSpace(proxyHelper.ProxyBypass))
                 {
@@ -187,87 +182,82 @@ namespace System.Net.Http
         }
 
         /// <summary>
-        /// This function will evaluate given string and it will try to convert
-        /// 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.
+        /// 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:
+        /// ([&lt;scheme&gt;=][&lt;scheme&gt;"://"]&lt;server&gt;[":"&lt;port&gt;])
         /// </summary>
-        private static Uri GetUriFromString(string value)
+        private static void ParseProxyConfig(string value, out Uri insecureProxy, out Uri secureProxy)
         {
+            insecureProxy = null;
+            secureProxy = null;
             if (string.IsNullOrEmpty(value))
             {
-                return null;
+                return;
             }
 
-            if (!value.Contains("://"))
+            if (value.IndexOfAny(s_proxyDelimiters) == -1)
             {
-                value = "http://" + value;
+                // Optimize out an allocation from Split() when no delimiters are present.
+                ParseProxyConfigPart(value, ref insecureProxy, ref secureProxy);
+                return;
             }
 
-            if (Uri.TryCreate(value, UriKind.Absolute, out Uri uri) &&
-                    (uri.Scheme == Uri.UriSchemeHttp || uri.Scheme == Uri.UriSchemeHttps))
+            string[] parts = value.Split(s_proxyDelimiters, StringSplitOptions.RemoveEmptyEntries);
+            foreach (string part in parts)
             {
-                // We only support http and https for now.
-                return uri;
+                ParseProxyConfigPart(part, ref insecureProxy, ref secureProxy);
+
+                if (insecureProxy != null && secureProxy != null)
+                {
+                    return;
+                }
             }
-            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:
-        /// ([&lt;scheme&gt;=][&lt;scheme&gt;"://"]&lt;server&gt;[":"&lt;port&gt;])
-        /// </summary>
-        private static void ParseProxyConfig(string value, out Uri insecureProxy, out Uri secureProxy )
+        private static void ParseProxyConfigPart(ReadOnlySpan<char> part, ref Uri insecureProxy, ref Uri secureProxy)
         {
-            secureProxy = null;
-            insecureProxy = null;
-            if (string.IsNullOrEmpty(value))
+            ref Uri destUri = ref insecureProxy;
+            bool hasScheme = false;
+
+            if (part.StartsWith("http="))
             {
-                return;
+                hasScheme = true;
+                part = part.Slice(5);
             }
-
-            int idx = value.IndexOf("http://", StringComparison.Ordinal);
-            if (idx >= 0)
+            else if (part.StartsWith("https="))
             {
-                int proxyLength = GetProxySubstringLength(value, idx);
-                Uri.TryCreate(value.Substring(idx, proxyLength) , UriKind.Absolute, out insecureProxy);
+                destUri = ref secureProxy;
+                hasScheme = true;
+                part = part.Slice(6);
             }
 
-            if (insecureProxy == null)
+            if (part.StartsWith("http://"))
             {
-                idx = value.IndexOf("http=", StringComparison.Ordinal);
-                if (idx >= 0)
-                {
-                    idx += 5; // Skip "http=" so we can replace it with "http://"
-                    int proxyLength = GetProxySubstringLength(value, idx);
-                    Uri.TryCreate(string.Concat("http://", value.AsSpan(idx, proxyLength)), UriKind.Absolute, out insecureProxy);
-                }
+                destUri = ref insecureProxy;
+                hasScheme = true;
+                part = part.Slice(7);
             }
-
-            idx = value.IndexOf("https://", StringComparison.Ordinal);
-            if (idx >= 0)
+            else if (part.StartsWith("https://"))
             {
-                idx += 8; // Skip "https://" so we can replace it with "http://"
-                int proxyLength = GetProxySubstringLength(value, idx);
-                Uri.TryCreate(string.Concat("http://", value.AsSpan(idx, proxyLength)), UriKind.Absolute, out secureProxy);
+                destUri = ref secureProxy;
+                hasScheme = true;
+                part = part.Slice(8);
             }
 
-            if (secureProxy == null)
+            string proxyString = string.Concat("http://", part);
+
+            if (!hasScheme)
             {
-                idx = value.IndexOf("https=", StringComparison.Ordinal);
-                if (idx >= 0)
+                if (Uri.TryCreate(proxyString, UriKind.Absolute, out Uri proxyUri))
                 {
-                    idx += 6; // Skip "https=" so we can replace it with "http://"
-                    int proxyLength = GetProxySubstringLength(value, idx);
-                    Uri.TryCreate(string.Concat("http://", value.AsSpan(idx, proxyLength)), UriKind.Absolute, out secureProxy);
+                    insecureProxy ??= proxyUri;
+                    secureProxy ??= proxyUri;
                 }
             }
-        }
-
-        private static int GetProxySubstringLength(string proxyString, int idx)
-        {
-            int endOfProxy = proxyString.IndexOfAny(s_proxyDelimiters, idx);
-            return (endOfProxy == -1) ? proxyString.Length - idx : endOfProxy - idx;
+            else if (destUri == null)
+            {
+                Uri.TryCreate(proxyString, UriKind.Absolute, out destUri);
+            }
         }
 
         /// <summary>
@@ -294,7 +284,10 @@ namespace System.Net.Http
                         // we can return the Proxy uri directly.
                         if (proxyInfo.ProxyBypass == IntPtr.Zero)
                         {
-                            return GetUriFromString(Marshal.PtrToStringUni(proxyInfo.Proxy));
+                            string proxyString = Marshal.PtrToStringUni(proxyInfo.Proxy);
+                            ParseProxyConfig(proxyString, out Uri insecureProxy, out Uri secureProxy);
+
+                            return IsSecureUri(uri) ? secureProxy : insecureProxy;
                         }
 
                         // A bypass list was also specified. This means that WinHTTP has fallen back to
@@ -370,12 +363,17 @@ namespace System.Net.Http
                 }
 
                 // We did not find match on bypass list.
-                return (uri.Scheme == UriScheme.Https || uri.Scheme == UriScheme.Wss) ? _secureProxyUri : _insecureProxyUri;
+                return IsSecureUri(uri) ? _secureProxyUri : _insecureProxyUri;
             }
 
             return null;
         }
 
+        private static bool IsSecureUri(Uri uri)
+        {
+            return uri.Scheme == UriScheme.Https || uri.Scheme == UriScheme.Wss;
+        }
+
         /// <summary>
         /// Checks if URI is subject to proxy or not.
         /// </summary>
index 327ca7b..52b4c8f 100644 (file)
@@ -18,6 +18,7 @@ namespace System.Net.Http.Tests
         private const string FakeProxyString = "http://proxy.contoso.com";
         private const string insecureProxyUri = "http://proxy.insecure.com";
         private const string secureProxyUri = "http://proxy.secure.com";
+        private const string secureAndInsecureProxyUri = "http://proxy.secure-and-insecure.com";
         private const string fooHttp = "http://foo.com";
         private const string fooHttps = "https://foo.com";
         private const string fooWs = "ws://foo.com";
@@ -29,42 +30,91 @@ namespace System.Net.Http.Tests
         }
 
         [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_WindowsProxy_Loaded(string rawProxyString, bool hasInsecureProxy, bool hasSecureProxy)
+        [MemberData(nameof(ProxyParsingData))]
+        public void HttpProxy_WindowsProxy_Manual_Loaded(string rawProxyString, string rawInsecureUri, string rawSecureUri)
         {
             RemoteExecutor.Invoke((proxyString, insecureProxy, secureProxy) =>
             {
-                IWebProxy p;
-
                 FakeRegistry.Reset();
-                Assert.False(HttpWindowsProxy.TryCreate(out p));
+
+                Assert.False(HttpWindowsProxy.TryCreate(out IWebProxy p));
 
                 FakeRegistry.WinInetProxySettings.Proxy = proxyString;
                 WinInetProxyHelper proxyHelper = new WinInetProxyHelper();
+                Assert.Null(proxyHelper.AutoConfigUrl);
+                Assert.Equal(proxyString, proxyHelper.Proxy);
+                Assert.False(proxyHelper.AutoSettingsUsed);
+                Assert.True(proxyHelper.ManualSettingsUsed);
+
+                Assert.True(HttpWindowsProxy.TryCreate(out p));
+                Assert.NotNull(p);
+
+                Assert.Equal(!string.IsNullOrEmpty(insecureProxy) ? new Uri(insecureProxy) : null, p.GetProxy(new Uri(fooHttp)));
+                Assert.Equal(!string.IsNullOrEmpty(secureProxy) ? new Uri(secureProxy) : null, p.GetProxy(new Uri(fooHttps)));
+                Assert.Equal(!string.IsNullOrEmpty(insecureProxy) ? new Uri(insecureProxy) : null, p.GetProxy(new Uri(fooWs)));
+                Assert.Equal(!string.IsNullOrEmpty(secureProxy) ? new Uri(secureProxy) : null, p.GetProxy(new Uri(fooWss)));
+                return RemoteExecutor.SuccessExitCode;
+            }, rawProxyString, rawInsecureUri ?? string.Empty, rawSecureUri ?? string.Empty).Dispose();
+        }
+
+        [Theory]
+        [MemberData(nameof(ProxyParsingData))]
+        public void HttpProxy_WindowsProxy_PAC_Loaded(string rawProxyString, string rawInsecureUri, string rawSecureUri)
+        {
+            RemoteExecutor.Invoke((proxyString, insecureProxy, secureProxy) =>
+            {
+                TestControl.ResetAll();
+
+                Assert.False(HttpWindowsProxy.TryCreate(out IWebProxy p));
+
+                FakeRegistry.WinInetProxySettings.AutoConfigUrl = "http://127.0.0.1/proxy.pac";
+                WinInetProxyHelper proxyHelper = new WinInetProxyHelper();
+                Assert.Null(proxyHelper.Proxy);
+                Assert.Equal(FakeRegistry.WinInetProxySettings.AutoConfigUrl, proxyHelper.AutoConfigUrl);
+                Assert.False(proxyHelper.ManualSettingsUsed);
+                Assert.True(proxyHelper.AutoSettingsUsed);
 
                 Assert.True(HttpWindowsProxy.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)));
+                // With a HttpWindowsProxy created configured to use auto-config, now set Proxy so when it
+                // attempts to resolve a proxy, it resolves our string.
+                FakeRegistry.WinInetProxySettings.Proxy = proxyString;
+                proxyHelper = new WinInetProxyHelper();
+                Assert.Equal(proxyString, proxyHelper.Proxy);
+
+                Assert.Equal(!string.IsNullOrEmpty(insecureProxy) ? new Uri(insecureProxy) : null, p.GetProxy(new Uri(fooHttp)));
+                Assert.Equal(!string.IsNullOrEmpty(secureProxy) ? new Uri(secureProxy) : null, p.GetProxy(new Uri(fooHttps)));
+                Assert.Equal(!string.IsNullOrEmpty(insecureProxy) ? new Uri(insecureProxy) : null, p.GetProxy(new Uri(fooWs)));
+                Assert.Equal(!string.IsNullOrEmpty(secureProxy) ? new Uri(secureProxy) : null, p.GetProxy(new Uri(fooWss)));
                 return RemoteExecutor.SuccessExitCode;
-            }, rawProxyString, hasInsecureProxy.ToString(), hasSecureProxy.ToString()).Dispose();
+            }, rawProxyString, rawInsecureUri ?? string.Empty, rawSecureUri ?? string.Empty).Dispose();
         }
 
+        public static TheoryData<string, string, string> ProxyParsingData =>
+            new TheoryData<string, string, string>
+            {
+                { "http://proxy.insecure.com", insecureProxyUri, null },
+                { "http=http://proxy.insecure.com", insecureProxyUri, null },
+                { "http=proxy.insecure.com", insecureProxyUri, null },
+                { "http://proxy.insecure.com http://proxy.wrong.com", insecureProxyUri, null },
+                { "https=proxy.secure.com http=proxy.insecure.com", insecureProxyUri, secureProxyUri },
+                { "https://proxy.secure.com\nhttp://proxy.insecure.com", insecureProxyUri, secureProxyUri },
+                { "https=proxy.secure.com\nhttp=proxy.insecure.com", insecureProxyUri, secureProxyUri },
+                { "https://proxy.secure.com;http://proxy.insecure.com", insecureProxyUri, secureProxyUri },
+                { "https=proxy.secure.com;http=proxy.insecure.com", insecureProxyUri, secureProxyUri },
+                { ";http=proxy.insecure.com;;", insecureProxyUri, null },
+                { "    http=proxy.insecure.com    ", insecureProxyUri, null },
+                { "http=proxy.insecure.com;http=proxy.wrong.com", insecureProxyUri, null },
+                { "http=http://proxy.insecure.com", insecureProxyUri, null },
+                { "https://proxy.secure.com", null, secureProxyUri },
+                { "https=proxy.secure.com", null, secureProxyUri },
+                { "https=https://proxy.secure.com", null, secureProxyUri },
+                { "http=https://proxy.secure.com", null, secureProxyUri },
+                { "https=http://proxy.insecure.com", insecureProxyUri, null },
+                { "proxy.secure-and-insecure.com", secureAndInsecureProxyUri, secureAndInsecureProxyUri },
+            };
+
         [Theory]
         [InlineData("localhost:1234", "http://localhost:1234/")]
         [InlineData("123.123.123.123", "http://123.123.123.123/")]