From efac2ca70bfbcfaf4c6e7690f87dbdc826f4271d Mon Sep 17 00:00:00 2001 From: wfurt Date: Wed, 12 Jun 2024 11:17:08 -0700 Subject: [PATCH 01/12] update proxy setting on registry changes --- .../Interop.RegNotifyChangeKeyValue.cs | 27 ++++ .../src/System.Net.Http.csproj | 5 +- .../SocketsHttpHandler/HttpWindowsProxy.cs | 119 +++++++++++------- 3 files changed, 106 insertions(+), 45 deletions(-) create mode 100644 src/libraries/Common/src/Interop/Windows/Advapi32/Interop.RegNotifyChangeKeyValue.cs diff --git a/src/libraries/Common/src/Interop/Windows/Advapi32/Interop.RegNotifyChangeKeyValue.cs b/src/libraries/Common/src/Interop/Windows/Advapi32/Interop.RegNotifyChangeKeyValue.cs new file mode 100644 index 00000000000000..57d04bfda98962 --- /dev/null +++ b/src/libraries/Common/src/Interop/Windows/Advapi32/Interop.RegNotifyChangeKeyValue.cs @@ -0,0 +1,27 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.Win32.SafeHandles; +using System; +using System.Runtime.InteropServices; + +internal static partial class Interop +{ + internal static partial class Advapi32 + { + + internal const int REG_NOTIFY_CHANGE_NAME = 0x1; + internal const int REG_NOTIFY_CHANGE_ATTRIBUTES = 0x2; + internal const int REG_NOTIFY_CHANGE_LAST_SET = 0x4; + internal const int REG_NOTIFY_CHANGE_SECURITY = 0x8; + internal const int REG_NOTIFY_THREAD_AGNOSTIC = 0x10000000; + + [LibraryImport(Libraries.Advapi32, EntryPoint = "RegNotifyChangeKeyValue", StringMarshalling = StringMarshalling.Utf16)] + internal static partial int RegNotifyChangeKeyValue( + SafeHandle hKey, + [MarshalAs(UnmanagedType.Bool)] bool watchSubtree, + uint notifyFilter, + SafeHandle hEvent, + [MarshalAs(UnmanagedType.Bool)] bool asynchronous); + } +} diff --git a/src/libraries/System.Net.Http/src/System.Net.Http.csproj b/src/libraries/System.Net.Http/src/System.Net.Http.csproj index e4ffec94743f35..980dc33464b550 100644 --- a/src/libraries/System.Net.Http/src/System.Net.Http.csproj +++ b/src/libraries/System.Net.Http/src/System.Net.Http.csproj @@ -384,7 +384,9 @@ + Link="Common\Interop\Windows\WinHttp\Interop.winhttp.cs" /> + + 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 3a2c4ee6ff5de8..f0bbe3ff3fa8f7 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 @@ -6,45 +6,74 @@ using System.Collections.Generic; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; -using System.IO.Compression; using System.Net.NetworkInformation; using System.Runtime.InteropServices; using System.Text; using System.Threading; +using Microsoft.Win32; using SafeWinHttpHandle = Interop.WinHttp.SafeWinHttpHandle; namespace System.Net.Http { internal sealed class HttpWindowsProxy : IMultiWebProxy, IDisposable { - private readonly MultiProxy _insecureProxy; // URI of the http system proxy if set - private readonly MultiProxy _secureProxy; // URI of the https system proxy if set - private readonly FailedProxyCache _failedProxies = new FailedProxyCache(); - private readonly List? _bypass; // list of domains not to proxy - private readonly bool _bypassLocal; // we should bypass domain considered local - private readonly List? _localIp; + private readonly RegistryKey? _internetSettingsRegistry = Registry.CurrentUser.OpenSubKey("Software\\Microsoft\\Windows\\CurrentVersion\\Internet Settings"); + private MultiProxy _insecureProxy; // URI of the http system proxy if set + private MultiProxy _secureProxy; // URI of the https system proxy if set + private FailedProxyCache _failedProxies = new FailedProxyCache(); + private List? _bypass; // list of domains not to proxy + //private bool _bypassLocal; // we should bypass domain considered local + private List? _localIp; private ICredentials? _credentials; - private readonly WinInetProxyHelper _proxyHelper; + private WinInetProxyHelper _proxyHelper; private SafeWinHttpHandle? _sessionHandle; private bool _disposed; + private EventWaitHandle _waitHandle = new EventWaitHandle(false, EventResetMode.AutoReset); + private const int _registrationFlags = Interop.Advapi32.REG_NOTIFY_CHANGE_NAME | Interop.Advapi32.REG_NOTIFY_CHANGE_LAST_SET | Interop.Advapi32.REG_NOTIFY_CHANGE_ATTRIBUTES | Interop.Advapi32.REG_NOTIFY_THREAD_AGNOSTIC; + private RegisteredWaitHandle? _registeredWaitHandle; public static bool TryCreate([NotNullWhen(true)] out IWebProxy? proxy) { - // This will get basic proxy setting from system using existing - // WinInetProxyHelper functions. If no proxy is enabled, it will return null. - SafeWinHttpHandle? sessionHandle = null; - proxy = null; + proxy = new HttpWindowsProxy(); + return true; + } - WinInetProxyHelper proxyHelper = new WinInetProxyHelper(); - if (!proxyHelper.ManualSettingsOnly && !proxyHelper.AutoSettingsUsed) + public HttpWindowsProxy() + { + + if (_internetSettingsRegistry != null) { - return false; + // we register for change notifications so we can react to changes during lifetime. + if (Interop.Advapi32.RegNotifyChangeKeyValue(_internetSettingsRegistry.Handle, true, _registrationFlags, _waitHandle.SafeWaitHandle, true) == 0) + { + _registeredWaitHandle = ThreadPool.RegisterWaitForSingleObject(_waitHandle, RegistryChangeNotificationCallback, this, -1, false); + } } + _proxyHelper = new WinInetProxyHelper(); + UpdateConfiguration(); + } + + private static void RegistryChangeNotificationCallback(object? state, bool timedOut) + { + Console.WriteLine("WaitOrTimerCallback called!!!! {0}", timedOut); + + HttpWindowsProxy proxy = (HttpWindowsProxy)state!; + // We need to register for notification every time + Interop.Advapi32.RegNotifyChangeKeyValue(proxy._internetSettingsRegistry!.Handle, true, _registrationFlags, proxy._waitHandle.SafeWaitHandle, true); + proxy.UpdateConfiguration(); + } + + private void UpdateConfiguration() + { + + WinInetProxyHelper proxyHelper = new WinInetProxyHelper(); + Console.WriteLine("UpateProxies {0} {1}", proxyHelper.ManualSettingsUsed, proxyHelper.AutoSettingsUsed); + if (proxyHelper.AutoSettingsUsed) { if (NetEventSource.Log.IsEnabled()) NetEventSource.Info(proxyHelper, $"AutoSettingsUsed, calling {nameof(Interop.WinHttp.WinHttpOpen)}"); - sessionHandle = Interop.WinHttp.WinHttpOpen( + SafeWinHttpHandle? sessionHandle = Interop.WinHttp.WinHttpOpen( IntPtr.Zero, Interop.WinHttp.WINHTTP_ACCESS_TYPE_NO_PROXY, Interop.WinHttp.WINHTTP_NO_PROXY_NAME, @@ -56,18 +85,10 @@ public static bool TryCreate([NotNullWhen(true)] out IWebProxy? proxy) // Proxy failures are currently ignored by managed handler. if (NetEventSource.Log.IsEnabled()) NetEventSource.Error(proxyHelper, $"{nameof(Interop.WinHttp.WinHttpOpen)} returned invalid handle"); sessionHandle.Dispose(); - return false; } - } - - proxy = new HttpWindowsProxy(proxyHelper, sessionHandle); - return true; - } - private HttpWindowsProxy(WinInetProxyHelper proxyHelper, SafeWinHttpHandle? sessionHandle) - { - _proxyHelper = proxyHelper; - _sessionHandle = sessionHandle; + _sessionHandle = sessionHandle; + } if (proxyHelper.ManualSettingsUsed) { @@ -80,10 +101,12 @@ private HttpWindowsProxy(WinInetProxyHelper proxyHelper, SafeWinHttpHandle? sess { int idx = 0; string? tmp; + bool bypassLocal = false; + List? localIp = null; // Process bypass list for manual setting. // Initial list size is best guess based on string length assuming each entry is at least 5 characters on average. - _bypass = new List(proxyHelper.ProxyBypass.Length / 5); + List? bypass = new List(proxyHelper.ProxyBypass.Length / 5); while (idx < proxyHelper.ProxyBypass.Length) { @@ -114,7 +137,7 @@ private HttpWindowsProxy(WinInetProxyHelper proxyHelper, SafeWinHttpHandle? sess } else if (string.Compare(proxyHelper.ProxyBypass, start, "", 0, 7, StringComparison.OrdinalIgnoreCase) == 0) { - _bypassLocal = true; + bypassLocal = true; tmp = null; } else @@ -137,28 +160,29 @@ private HttpWindowsProxy(WinInetProxyHelper proxyHelper, SafeWinHttpHandle? sess continue; } - _bypass.Add(tmp); + bypass.Add(tmp); } - if (_bypass.Count == 0) - { - // Bypass string only had garbage we did not parse. - _bypass = null; - } - } - if (_bypassLocal) - { - _localIp = new List(); - foreach (NetworkInterface netInterface in NetworkInterface.GetAllNetworkInterfaces()) + _bypass = bypass.Count > 0 ? bypass : null; + + if (bypassLocal) { - IPInterfaceProperties ipProps = netInterface.GetIPProperties(); - foreach (UnicastIPAddressInformation addr in ipProps.UnicastAddresses) + localIp = new List(); + foreach (NetworkInterface netInterface in NetworkInterface.GetAllNetworkInterfaces()) { - _localIp.Add(addr.Address); + IPInterfaceProperties ipProps = netInterface.GetIPProperties(); + foreach (UnicastIPAddressInformation addr in ipProps.UnicastAddresses) + { + localIp.Add(addr.Address); + } } } + + _localIp = localIp?.Count > 0 ? localIp : null; } } + + _proxyHelper = proxyHelper; } public void Dispose() @@ -171,6 +195,8 @@ public void Dispose() { SafeWinHttpHandle.DisposeAndClearHandle(ref _sessionHandle); } + + _registeredWaitHandle?.Unregister(null); } } @@ -179,6 +205,11 @@ public void Dispose() /// public Uri? GetProxy(Uri uri) { + if (!_proxyHelper.AutoSettingsUsed && !_proxyHelper.AutoSettingsUsed) + { + return null; + } + GetMultiProxy(uri).ReadNext(out Uri? proxyUri, out _); return proxyUri; } @@ -240,7 +271,7 @@ public MultiProxy GetMultiProxy(Uri uri) // Fallback to manual settings if present. if (_proxyHelper.ManualSettingsUsed) { - if (_bypassLocal) + if (_localIp != null) { IPAddress? address; @@ -261,7 +292,7 @@ public MultiProxy GetMultiProxy(Uri uri) { // Host is valid IP address. // Check if it belongs to local system. - foreach (IPAddress a in _localIp!) + foreach (IPAddress a in _localIp) { if (a.Equals(address)) { From c58cac2cb7038a220f553cfef852742fb9b1f465 Mon Sep 17 00:00:00 2001 From: wfurt Date: Wed, 12 Jun 2024 11:31:33 -0700 Subject: [PATCH 02/12] udpate --- .../src/System/Net/Http/SocketsHttpHandler/HttpWindowsProxy.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 f0bbe3ff3fa8f7..43215a1a9e9669 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 @@ -22,7 +22,6 @@ internal sealed class HttpWindowsProxy : IMultiWebProxy, IDisposable private MultiProxy _secureProxy; // URI of the https system proxy if set private FailedProxyCache _failedProxies = new FailedProxyCache(); private List? _bypass; // list of domains not to proxy - //private bool _bypassLocal; // we should bypass domain considered local private List? _localIp; private ICredentials? _credentials; private WinInetProxyHelper _proxyHelper; @@ -197,6 +196,7 @@ public void Dispose() } _registeredWaitHandle?.Unregister(null); + _internetSettingsRegistry?.Dispose(); } } From 1605f7fbfdae122bcb41bac49d3b1fda444b3df9 Mon Sep 17 00:00:00 2001 From: wfurt Date: Wed, 12 Jun 2024 20:58:35 +0000 Subject: [PATCH 03/12] build fixes --- src/libraries/System.Net.Http/src/System.Net.Http.csproj | 4 ++-- .../tests/UnitTests/System.Net.Http.Unit.Tests.csproj | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Net.Http/src/System.Net.Http.csproj b/src/libraries/System.Net.Http/src/System.Net.Http.csproj index 980dc33464b550..84bb480ac10115 100644 --- a/src/libraries/System.Net.Http/src/System.Net.Http.csproj +++ b/src/libraries/System.Net.Http/src/System.Net.Http.csproj @@ -384,9 +384,9 @@ + Link="Common\Interop\Windows\WinHttp\Interop.winhttp.cs" /> + Link="Common\Interop\Windows\Advapi32\Interop.RegNotifyChangeKeyValue.cs" /> + Date: Wed, 12 Jun 2024 14:52:32 -0700 Subject: [PATCH 04/12] console --- .../src/System/Net/Http/SocketsHttpHandler/HttpWindowsProxy.cs | 3 --- 1 file changed, 3 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 43215a1a9e9669..35d8c00c84ff6d 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 @@ -55,8 +55,6 @@ public HttpWindowsProxy() private static void RegistryChangeNotificationCallback(object? state, bool timedOut) { - Console.WriteLine("WaitOrTimerCallback called!!!! {0}", timedOut); - HttpWindowsProxy proxy = (HttpWindowsProxy)state!; // We need to register for notification every time Interop.Advapi32.RegNotifyChangeKeyValue(proxy._internetSettingsRegistry!.Handle, true, _registrationFlags, proxy._waitHandle.SafeWaitHandle, true); @@ -67,7 +65,6 @@ private void UpdateConfiguration() { WinInetProxyHelper proxyHelper = new WinInetProxyHelper(); - Console.WriteLine("UpateProxies {0} {1}", proxyHelper.ManualSettingsUsed, proxyHelper.AutoSettingsUsed); if (proxyHelper.AutoSettingsUsed) { From 208217c330fb6d1578ff1a1f259729a6784d6e57 Mon Sep 17 00:00:00 2001 From: wfurt Date: Wed, 12 Jun 2024 14:55:11 -0700 Subject: [PATCH 05/12] registry --- .../src/System/Net/Http/SocketsHttpHandler/HttpWindowsProxy.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 35d8c00c84ff6d..2d9f9ca593340c 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 @@ -17,7 +17,7 @@ namespace System.Net.Http { internal sealed class HttpWindowsProxy : IMultiWebProxy, IDisposable { - private readonly RegistryKey? _internetSettingsRegistry = Registry.CurrentUser.OpenSubKey("Software\\Microsoft\\Windows\\CurrentVersion\\Internet Settings"); + private readonly RegistryKey? _internetSettingsRegistry = Registry.CurrentUser?.OpenSubKey("Software\\Microsoft\\Windows\\CurrentVersion\\Internet Settings"); private MultiProxy _insecureProxy; // URI of the http system proxy if set private MultiProxy _secureProxy; // URI of the https system proxy if set private FailedProxyCache _failedProxies = new FailedProxyCache(); From af18353c8d4bece5c83806cdd41282b44b0ff3bd Mon Sep 17 00:00:00 2001 From: wfurt Date: Wed, 12 Jun 2024 16:03:11 -0700 Subject: [PATCH 06/12] winhttp --- .../tests/UnitTests/HttpWindowsProxyTest.cs | 2 +- .../System.Net.Http.WinHttpHandler.Unit.Tests.csproj | 4 +++- .../System/Net/Http/SocketsHttpHandler/HttpWindowsProxy.cs | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Net.Http.WinHttpHandler/tests/UnitTests/HttpWindowsProxyTest.cs b/src/libraries/System.Net.Http.WinHttpHandler/tests/UnitTests/HttpWindowsProxyTest.cs index aa5cccbb28886e..1cc65f05ce94ef 100644 --- a/src/libraries/System.Net.Http.WinHttpHandler/tests/UnitTests/HttpWindowsProxyTest.cs +++ b/src/libraries/System.Net.Http.WinHttpHandler/tests/UnitTests/HttpWindowsProxyTest.cs @@ -31,7 +31,7 @@ public static IEnumerable ManualSettingsMemberData() [Fact] public void TryCreate_WinInetProxySettingsAllOff_ReturnsFalse() { - Assert.False(HttpWindowsProxy.TryCreate(out IWebProxy webProxy)); + Assert.True(HttpWindowsProxy.TryCreate(out IWebProxy webProxy)); } [Theory] diff --git a/src/libraries/System.Net.Http.WinHttpHandler/tests/UnitTests/System.Net.Http.WinHttpHandler.Unit.Tests.csproj b/src/libraries/System.Net.Http.WinHttpHandler/tests/UnitTests/System.Net.Http.WinHttpHandler.Unit.Tests.csproj index 4cd4a30a287e99..551c222e67f1ea 100644 --- a/src/libraries/System.Net.Http.WinHttpHandler/tests/UnitTests/System.Net.Http.WinHttpHandler.Unit.Tests.csproj +++ b/src/libraries/System.Net.Http.WinHttpHandler/tests/UnitTests/System.Net.Http.WinHttpHandler.Unit.Tests.csproj @@ -95,7 +95,9 @@ + Link="ProductionCode\MultiProxy.cs" /> + 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 2d9f9ca593340c..e6ee1a67253d7b 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 @@ -202,7 +202,7 @@ public void Dispose() /// public Uri? GetProxy(Uri uri) { - if (!_proxyHelper.AutoSettingsUsed && !_proxyHelper.AutoSettingsUsed) + if (!_proxyHelper.AutoSettingsUsed && !_proxyHelper.ManualSettingsOnly) { return null; } From 4fcfb12e180afbd8112d2379c3305db1bf3f3738 Mon Sep 17 00:00:00 2001 From: wfurt Date: Thu, 13 Jun 2024 03:17:47 +0000 Subject: [PATCH 07/12] invalid --- .../tests/UnitTests/HttpWindowsProxyTest.cs | 36 ++----------------- 1 file changed, 2 insertions(+), 34 deletions(-) diff --git a/src/libraries/System.Net.Http/tests/UnitTests/HttpWindowsProxyTest.cs b/src/libraries/System.Net.Http/tests/UnitTests/HttpWindowsProxyTest.cs index df6648aab27300..f4bda04aff0793 100644 --- a/src/libraries/System.Net.Http/tests/UnitTests/HttpWindowsProxyTest.cs +++ b/src/libraries/System.Net.Http/tests/UnitTests/HttpWindowsProxyTest.cs @@ -39,8 +39,6 @@ await RemoteExecutor.Invoke((proxyString, insecureProxy, secureProxy) => { FakeRegistry.Reset(); - Assert.False(HttpWindowsProxy.TryCreate(out IWebProxy p)); - FakeRegistry.WinInetProxySettings.Proxy = proxyString; WinInetProxyHelper proxyHelper = new WinInetProxyHelper(); Assert.Null(proxyHelper.AutoConfigUrl); @@ -48,7 +46,7 @@ await RemoteExecutor.Invoke((proxyString, insecureProxy, secureProxy) => Assert.False(proxyHelper.AutoSettingsUsed); Assert.True(proxyHelper.ManualSettingsUsed); - Assert.True(HttpWindowsProxy.TryCreate(out p)); + Assert.True(HttpWindowsProxy.TryCreate(out IWebProxy p)); Assert.NotNull(p); Assert.Equal(!string.IsNullOrEmpty(insecureProxy) ? new Uri(insecureProxy) : null, p.GetProxy(new Uri(fooHttp))); @@ -66,8 +64,6 @@ await 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); @@ -75,7 +71,7 @@ await RemoteExecutor.Invoke((proxyString, insecureProxy, secureProxy) => Assert.False(proxyHelper.ManualSettingsUsed); Assert.True(proxyHelper.AutoSettingsUsed); - Assert.True(HttpWindowsProxy.TryCreate(out p)); + Assert.True(HttpWindowsProxy.TryCreate(out IWebProxy p)); Assert.NotNull(p); // With a HttpWindowsProxy created configured to use auto-config, now set Proxy so when it @@ -207,34 +203,6 @@ await RemoteExecutor.Invoke((bypassValue, expected) => }, bypass, count.ToString()).DisposeAsync(); } - [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - [InlineData("http://")] - [InlineData("http=")] - [InlineData("http://;")] - [InlineData("http=;")] - [InlineData(" ; ")] - public async Task HttpProxy_InvalidWindowsProxy_Null(string rawProxyString) - { - await RemoteExecutor.Invoke((proxyString) => - { - IWebProxy p; - - FakeRegistry.Reset(); - Assert.False(HttpWindowsProxy.TryCreate(out p)); - - FakeRegistry.WinInetProxySettings.Proxy = proxyString; - WinInetProxyHelper proxyHelper = new WinInetProxyHelper(); - - Assert.True(HttpWindowsProxy.TryCreate(out p)); - Assert.NotNull(p); - - Assert.Null(p.GetProxy(new Uri(fooHttp))); - Assert.Null(p.GetProxy(new Uri(fooHttps))); - Assert.Null(p.GetProxy(new Uri(fooWs))); - Assert.Null(p.GetProxy(new Uri(fooWss))); - }, rawProxyString).DisposeAsync(); - } - [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] [MemberData(nameof(HttpProxy_Multi_Data))] public async Task HttpProxy_Multi_Success(bool manualConfig, string proxyConfig, string url, string expected) From 690fe3572adb342c15606eebbd659d87f051fa38 Mon Sep 17 00:00:00 2001 From: wfurt Date: Thu, 13 Jun 2024 21:42:58 -0700 Subject: [PATCH 08/12] feedback --- .../Net/Http/HttpClientHandlerTest.Proxy.cs | 2 +- .../SocketsHttpHandler/HttpWindowsProxy.cs | 45 +++++++++++-------- .../SystemProxyInfo.Windows.cs | 5 ++- 3 files changed, 30 insertions(+), 22 deletions(-) diff --git a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.Proxy.cs b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.Proxy.cs index 1946228bf7fad8..07c226b7c214e5 100644 --- a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.Proxy.cs +++ b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.Proxy.cs @@ -505,7 +505,7 @@ public async Task MultiProxy_PAC_Failover_Succeeds() winInetProxyHelperType.GetField("_proxyBypass", Reflection.BindingFlags.Instance | Reflection.BindingFlags.NonPublic).SetValue(winInetProxyHelper, null); // Create a HttpWindowsProxy with our custom WinInetProxyHelper. - IWebProxy httpWindowsProxy = (IWebProxy)Activator.CreateInstance(Type.GetType("System.Net.Http.HttpWindowsProxy, System.Net.Http", true), Reflection.BindingFlags.NonPublic | Reflection.BindingFlags.Instance, null, new[] { winInetProxyHelper, null }, null); + IWebProxy httpWindowsProxy = (IWebProxy)Activator.CreateInstance(Type.GetType("System.Net.Http.HttpWindowsProxy, System.Net.Http", true), Reflection.BindingFlags.Public | Reflection.BindingFlags.NonPublic| Reflection.BindingFlags.Instance, null, new[] { winInetProxyHelper }, null); Task nextFailedConnection = null; 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 e6ee1a67253d7b..aff10d45f33750 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 @@ -19,7 +19,7 @@ internal sealed class HttpWindowsProxy : IMultiWebProxy, IDisposable { private readonly RegistryKey? _internetSettingsRegistry = Registry.CurrentUser?.OpenSubKey("Software\\Microsoft\\Windows\\CurrentVersion\\Internet Settings"); private MultiProxy _insecureProxy; // URI of the http system proxy if set - private MultiProxy _secureProxy; // URI of the https system proxy if set + private MultiProxy _secureProxy; // URI of the https system proxy if set private FailedProxyCache _failedProxies = new FailedProxyCache(); private List? _bypass; // list of domains not to proxy private List? _localIp; @@ -28,43 +28,50 @@ internal sealed class HttpWindowsProxy : IMultiWebProxy, IDisposable private SafeWinHttpHandle? _sessionHandle; private bool _disposed; private EventWaitHandle _waitHandle = new EventWaitHandle(false, EventResetMode.AutoReset); - private const int _registrationFlags = Interop.Advapi32.REG_NOTIFY_CHANGE_NAME | Interop.Advapi32.REG_NOTIFY_CHANGE_LAST_SET | Interop.Advapi32.REG_NOTIFY_CHANGE_ATTRIBUTES | Interop.Advapi32.REG_NOTIFY_THREAD_AGNOSTIC; + private const int RegistrationFlags = Interop.Advapi32.REG_NOTIFY_CHANGE_NAME | Interop.Advapi32.REG_NOTIFY_CHANGE_LAST_SET | Interop.Advapi32.REG_NOTIFY_CHANGE_ATTRIBUTES | Interop.Advapi32.REG_NOTIFY_THREAD_AGNOSTIC; private RegisteredWaitHandle? _registeredWaitHandle; - public static bool TryCreate([NotNullWhen(true)] out IWebProxy? proxy) - { - proxy = new HttpWindowsProxy(); - return true; - } - - public HttpWindowsProxy() + // 'proxy' used from tests via Reflection + public HttpWindowsProxy(WinInetProxyHelper? proxy = null) { - if (_internetSettingsRegistry != null) + if (_internetSettingsRegistry != null && proxy == null) { // we register for change notifications so we can react to changes during lifetime. - if (Interop.Advapi32.RegNotifyChangeKeyValue(_internetSettingsRegistry.Handle, true, _registrationFlags, _waitHandle.SafeWaitHandle, true) == 0) + if (Interop.Advapi32.RegNotifyChangeKeyValue(_internetSettingsRegistry.Handle, true, RegistrationFlags, _waitHandle.SafeWaitHandle, true) == 0) { _registeredWaitHandle = ThreadPool.RegisterWaitForSingleObject(_waitHandle, RegistryChangeNotificationCallback, this, -1, false); } } - _proxyHelper = new WinInetProxyHelper(); - UpdateConfiguration(); + _proxyHelper = proxy ?? new WinInetProxyHelper(); + UpdateConfiguration(_proxyHelper); } private static void RegistryChangeNotificationCallback(object? state, bool timedOut) { HttpWindowsProxy proxy = (HttpWindowsProxy)state!; - // We need to register for notification every time - Interop.Advapi32.RegNotifyChangeKeyValue(proxy._internetSettingsRegistry!.Handle, true, _registrationFlags, proxy._waitHandle.SafeWaitHandle, true); - proxy.UpdateConfiguration(); + if (!proxy._disposed) + { + + // This is executed from threadpool. we should not ever throw here. + try + { + // We need to register for notification every time + Interop.Advapi32.RegNotifyChangeKeyValue(proxy._internetSettingsRegistry!.Handle, true, RegistrationFlags, proxy._waitHandle.SafeWaitHandle, true); + proxy.UpdateConfiguration(); + } + catch (Exception ex) + { + if (NetEventSource.Log.IsEnabled()) NetEventSource.Error(proxy, $"Failed to refresh proxy configuration: {ex.Message}"); + } + } } - private void UpdateConfiguration() + private void UpdateConfiguration(WinInetProxyHelper? proxyHelper = null) { - WinInetProxyHelper proxyHelper = new WinInetProxyHelper(); + proxyHelper ??= new WinInetProxyHelper(); if (proxyHelper.AutoSettingsUsed) { @@ -192,8 +199,8 @@ public void Dispose() SafeWinHttpHandle.DisposeAndClearHandle(ref _sessionHandle); } - _registeredWaitHandle?.Unregister(null); _internetSettingsRegistry?.Dispose(); + _registeredWaitHandle?.Unregister(null); } } diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/SystemProxyInfo.Windows.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/SystemProxyInfo.Windows.cs index c49163a90602bd..4f51c72ac666a4 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/SystemProxyInfo.Windows.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/SystemProxyInfo.Windows.cs @@ -10,10 +10,11 @@ public static IWebProxy ConstructSystemProxy() { if (!HttpEnvironmentProxy.TryCreate(out IWebProxy? proxy)) { - HttpWindowsProxy.TryCreate(out proxy); + // We create instance even if there is currently no proxy as that can change during application run. + proxy = new HttpWindowsProxy(); } - return proxy ?? new HttpNoProxy(); + return proxy; } } } From 5fc1f3f728e44e2490a67830b938214767d2eb93 Mon Sep 17 00:00:00 2001 From: wfurt Date: Fri, 14 Jun 2024 11:31:30 -0700 Subject: [PATCH 09/12] feedback --- .../SocketsHttpHandler/HttpWindowsProxy.cs | 8 +++++-- .../tests/UnitTests/HttpWindowsProxyTest.cs | 23 ++++++------------- 2 files changed, 13 insertions(+), 18 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 aff10d45f33750..85c7311609f1a4 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 @@ -57,9 +57,13 @@ private static void RegistryChangeNotificationCallback(object? state, bool timed // This is executed from threadpool. we should not ever throw here. try { - // We need to register for notification every time + // We need to register for notification every time. We regisrerand lock before we process configuration + // so if there is update it would be serialized to ensure consistency. Interop.Advapi32.RegNotifyChangeKeyValue(proxy._internetSettingsRegistry!.Handle, true, RegistrationFlags, proxy._waitHandle.SafeWaitHandle, true); - proxy.UpdateConfiguration(); + lock (proxy) + { + proxy.UpdateConfiguration(); + } } catch (Exception ex) { diff --git a/src/libraries/System.Net.Http/tests/UnitTests/HttpWindowsProxyTest.cs b/src/libraries/System.Net.Http/tests/UnitTests/HttpWindowsProxyTest.cs index f4bda04aff0793..383bd81f8ada21 100644 --- a/src/libraries/System.Net.Http/tests/UnitTests/HttpWindowsProxyTest.cs +++ b/src/libraries/System.Net.Http/tests/UnitTests/HttpWindowsProxyTest.cs @@ -46,8 +46,7 @@ await RemoteExecutor.Invoke((proxyString, insecureProxy, secureProxy) => Assert.False(proxyHelper.AutoSettingsUsed); Assert.True(proxyHelper.ManualSettingsUsed); - Assert.True(HttpWindowsProxy.TryCreate(out IWebProxy p)); - Assert.NotNull(p); + IWebProxy p = new HttpWindowsProxy(proxyHelper); 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))); @@ -71,8 +70,7 @@ await RemoteExecutor.Invoke((proxyString, insecureProxy, secureProxy) => Assert.False(proxyHelper.ManualSettingsUsed); Assert.True(proxyHelper.AutoSettingsUsed); - Assert.True(HttpWindowsProxy.TryCreate(out IWebProxy p)); - Assert.NotNull(p); + IWebProxy p = new HttpWindowsProxy(proxyHelper); // With a HttpWindowsProxy created configured to use auto-config, now set Proxy so when it // attempts to resolve a proxy, it resolves our string. @@ -118,15 +116,12 @@ public async Task HttpProxy_WindowsProxy_Loaded(string rawProxyString, string ex { await RemoteExecutor.Invoke((proxyString, expectedString) => { - IWebProxy p; - FakeRegistry.Reset(); FakeRegistry.WinInetProxySettings.Proxy = proxyString; WinInetProxyHelper proxyHelper = new WinInetProxyHelper(); - Assert.True(HttpWindowsProxy.TryCreate(out p)); - Assert.NotNull(p); + IWebProxy p = new HttpWindowsProxy(proxyHelper); Assert.Equal(expectedString, p.GetProxy(new Uri(fooHttp)).ToString()); Assert.Equal(expectedString, p.GetProxy(new Uri(fooHttps)).ToString()); }, rawProxyString, expectedUri).DisposeAsync(); @@ -155,14 +150,12 @@ public async Task HttpProxy_Local_Bypassed(string name, bool shouldBypass) await RemoteExecutor.Invoke((url, expected) => { bool expectedResult = Boolean.Parse(expected); - IWebProxy p; FakeRegistry.Reset(); FakeRegistry.WinInetProxySettings.Proxy = insecureProxyUri; FakeRegistry.WinInetProxySettings.ProxyBypass = "23.23.86.44;*.foo.com;;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(HttpWindowsProxy.TryCreate(out p)); - Assert.NotNull(p); + IWebProxy p = new HttpWindowsProxy(); Uri u = new Uri(url); Assert.Equal(expectedResult, p.GetProxy(u) == null); @@ -180,14 +173,12 @@ public async Task HttpProxy_Local_Parsing(string bypass, int count) await RemoteExecutor.Invoke((bypassValue, expected) => { int expectedCount = Convert.ToInt32(expected); - IWebProxy p; FakeRegistry.Reset(); FakeRegistry.WinInetProxySettings.Proxy = insecureProxyUri; FakeRegistry.WinInetProxySettings.ProxyBypass = bypassValue; - Assert.True(HttpWindowsProxy.TryCreate(out p)); - Assert.NotNull(p); + IWebProxy p = new HttpWindowsProxy(); HttpWindowsProxy sp = p as HttpWindowsProxy; Assert.NotNull(sp); @@ -224,7 +215,7 @@ await RemoteExecutor.Invoke((manualConfigValue, proxyConfigValue, urlValue, expe FakeRegistry.WinInetProxySettings.AutoConfigUrl = "http://dummy.com"; } - Assert.True(HttpWindowsProxy.TryCreate(out IWebProxy p)); + IWebProxy p = new HttpWindowsProxy(); HttpWindowsProxy wp = Assert.IsType(p); if (!manual) @@ -287,7 +278,7 @@ await RemoteExecutor.Invoke(manualValue => FakeRegistry.WinInetProxySettings.AutoConfigUrl = "http://dummy.com"; } - Assert.True(HttpWindowsProxy.TryCreate(out IWebProxy p)); + IWebProxy p = new HttpWindowsProxy(); HttpWindowsProxy wp = Assert.IsType(p); if (!manual) From f9a321abda127577a0a3279f49b75a33fb46fefa Mon Sep 17 00:00:00 2001 From: wfurt Date: Fri, 14 Jun 2024 13:45:31 -0700 Subject: [PATCH 10/12] 'feedback' --- .../Advapi32/Interop.RegNotifyChangeKeyValue.cs | 1 - .../tests/UnitTests/HttpWindowsProxyTest.cs | 11 +++-------- .../System.Net.Http.WinHttpHandler.Unit.Tests.csproj | 4 ++-- 3 files changed, 5 insertions(+), 11 deletions(-) diff --git a/src/libraries/Common/src/Interop/Windows/Advapi32/Interop.RegNotifyChangeKeyValue.cs b/src/libraries/Common/src/Interop/Windows/Advapi32/Interop.RegNotifyChangeKeyValue.cs index 57d04bfda98962..547ad136ccb61f 100644 --- a/src/libraries/Common/src/Interop/Windows/Advapi32/Interop.RegNotifyChangeKeyValue.cs +++ b/src/libraries/Common/src/Interop/Windows/Advapi32/Interop.RegNotifyChangeKeyValue.cs @@ -9,7 +9,6 @@ internal static partial class Interop { internal static partial class Advapi32 { - internal const int REG_NOTIFY_CHANGE_NAME = 0x1; internal const int REG_NOTIFY_CHANGE_ATTRIBUTES = 0x2; internal const int REG_NOTIFY_CHANGE_LAST_SET = 0x4; diff --git a/src/libraries/System.Net.Http.WinHttpHandler/tests/UnitTests/HttpWindowsProxyTest.cs b/src/libraries/System.Net.Http.WinHttpHandler/tests/UnitTests/HttpWindowsProxyTest.cs index 1cc65f05ce94ef..f0f28e9fc205dc 100644 --- a/src/libraries/System.Net.Http.WinHttpHandler/tests/UnitTests/HttpWindowsProxyTest.cs +++ b/src/libraries/System.Net.Http.WinHttpHandler/tests/UnitTests/HttpWindowsProxyTest.cs @@ -28,11 +28,6 @@ public static IEnumerable ManualSettingsMemberData() yield return new object[] { new Uri("http://localhost"), true }; } - [Fact] - public void TryCreate_WinInetProxySettingsAllOff_ReturnsFalse() - { - Assert.True(HttpWindowsProxy.TryCreate(out IWebProxy webProxy)); - } [Theory] [MemberData(nameof(ManualSettingsMemberData))] @@ -44,7 +39,7 @@ public void GetProxy_BothAutoDetectAndManualSettingsButFailedAutoDetect_ManualSe FakeRegistry.WinInetProxySettings.ProxyBypass = ManualSettingsProxyBypassList; TestControl.PACFileNotDetectedOnNetwork = true; - Assert.True(HttpWindowsProxy.TryCreate(out IWebProxy webProxy)); + IWebProxy webProxy = new HttpWindowsProxy(); // The first GetProxy() call will try using WinInetProxyHelper (and thus WinHTTP) since AutoDetect is on. Uri proxyUri1 = webProxy.GetProxy(destination); @@ -74,7 +69,7 @@ public void GetProxy_ManualSettingsOnly_ManualSettingsUsed( FakeRegistry.WinInetProxySettings.Proxy = ManualSettingsProxyHost; FakeRegistry.WinInetProxySettings.ProxyBypass = ManualSettingsProxyBypassList; - Assert.True(HttpWindowsProxy.TryCreate(out IWebProxy webProxy)); + IWebProxy webProxy = new HttpWindowsProxy(); Uri proxyUri = webProxy.GetProxy(destination); if (bypassProxy) { @@ -90,7 +85,7 @@ public void GetProxy_ManualSettingsOnly_ManualSettingsUsed( public void IsBypassed_ReturnsFalse() { FakeRegistry.WinInetProxySettings.AutoDetect = true; - Assert.True(HttpWindowsProxy.TryCreate(out IWebProxy webProxy)); + IWebProxy webProxy = new HttpWindowsProxy(); Assert.False(webProxy.IsBypassed(new Uri("http://www.microsoft.com/"))); } } diff --git a/src/libraries/System.Net.Http.WinHttpHandler/tests/UnitTests/System.Net.Http.WinHttpHandler.Unit.Tests.csproj b/src/libraries/System.Net.Http.WinHttpHandler/tests/UnitTests/System.Net.Http.WinHttpHandler.Unit.Tests.csproj index 551c222e67f1ea..988c1cc1014224 100644 --- a/src/libraries/System.Net.Http.WinHttpHandler/tests/UnitTests/System.Net.Http.WinHttpHandler.Unit.Tests.csproj +++ b/src/libraries/System.Net.Http.WinHttpHandler/tests/UnitTests/System.Net.Http.WinHttpHandler.Unit.Tests.csproj @@ -95,9 +95,9 @@ + Link="ProductionCode\MultiProxy.cs" /> + Link="Common\Interop\Windows\Advapi32\Interop.RegNotifyChangeKeyValue.cs" /> From 3ff07402cd71bcc4dd467bcb04a74618d90ea9e1 Mon Sep 17 00:00:00 2001 From: Tomas Weinfurt Date: Mon, 17 Jun 2024 16:44:02 -0700 Subject: [PATCH 11/12] Apply suggestions from code review Co-authored-by: Jan Kotas --- .../System/Net/Http/SocketsHttpHandler/HttpWindowsProxy.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 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 85c7311609f1a4..a3958c6f51a0ae 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 @@ -44,8 +44,7 @@ public HttpWindowsProxy(WinInetProxyHelper? proxy = null) } } - _proxyHelper = proxy ?? new WinInetProxyHelper(); - UpdateConfiguration(_proxyHelper); + UpdateConfiguration(proxy); } private static void RegistryChangeNotificationCallback(object? state, bool timedOut) @@ -203,6 +202,7 @@ public void Dispose() SafeWinHttpHandle.DisposeAndClearHandle(ref _sessionHandle); } + _waitHandle?.Dispose(); _internetSettingsRegistry?.Dispose(); _registeredWaitHandle?.Unregister(null); } From 92adec55a35cb1313004b170bc5c6fad347ec09e Mon Sep 17 00:00:00 2001 From: wfurt Date: Thu, 20 Jun 2024 14:54:10 -0700 Subject: [PATCH 12/12] MemberNotNull --- .../src/System/Net/Http/SocketsHttpHandler/HttpWindowsProxy.cs | 1 + 1 file changed, 1 insertion(+) 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 a3958c6f51a0ae..fba738ca1490cf 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 @@ -71,6 +71,7 @@ private static void RegistryChangeNotificationCallback(object? state, bool timed } } + [MemberNotNull(nameof(_proxyHelper))] private void UpdateConfiguration(WinInetProxyHelper? proxyHelper = null) {