From d030f6ae8a7a9d150d9859e700eccd051fa477b0 Mon Sep 17 00:00:00 2001 From: Cory Nelson Date: Fri, 12 Jul 2019 09:58:34 -0700 Subject: [PATCH] Read WinHTTP's proxy format correctly (dotnet/corefx#39371) 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 --- .../Http/SocketsHttpHandler/HttpWindowsProxy.cs | 118 ++++++++++----------- .../tests/UnitTests/HttpWindowsProxyTest.cs | 94 ++++++++++++---- 2 files changed, 130 insertions(+), 82 deletions(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpWindowsProxy.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpWindowsProxy.cs index 68d1034..41a0d41 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpWindowsProxy.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpWindowsProxy.cs @@ -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 } /// - /// 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: + /// ([<scheme>=][<scheme>"://"]<server>[":"<port>]) /// - 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; } - /// - /// 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>]) - /// - private static void ParseProxyConfig(string value, out Uri insecureProxy, out Uri secureProxy ) + private static void ParseProxyConfigPart(ReadOnlySpan 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); + } } /// @@ -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; + } + /// /// Checks if URI is subject to proxy or not. /// diff --git a/src/libraries/System.Net.Http/tests/UnitTests/HttpWindowsProxyTest.cs b/src/libraries/System.Net.Http/tests/UnitTests/HttpWindowsProxyTest.cs index 327ca7b..52b4c8f 100644 --- a/src/libraries/System.Net.Http/tests/UnitTests/HttpWindowsProxyTest.cs +++ b/src/libraries/System.Net.Http/tests/UnitTests/HttpWindowsProxyTest.cs @@ -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 ProxyParsingData => + new TheoryData + { + { "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/")] -- 2.7.4