Several WebProxy-related fixes (#50368)
authorStephen Toub <stoub@microsoft.com>
Tue, 30 Mar 2021 00:40:33 +0000 (20:40 -0400)
committerGitHub <noreply@github.com>
Tue, 30 Mar 2021 00:40:33 +0000 (20:40 -0400)
- Split browser from non-browser to avoid CA1416 suppressions, UnsupportedOSPlatform attributes, and PNSEs when used from browser
- Cache domain name and local addresses for use in IsLocal, along with clearing of the caches upon network change detection
- Removing redundant IPAddress.IsLoopback check that's covered by previous Uri.IsLoopback check
- Change IsMatchInBypassList to use string interpolation, such that the non-default port case will get better implicitly with C# 10 interpolated string improvements

src/libraries/System.Net.WebProxy/src/System.Net.WebProxy.csproj
src/libraries/System.Net.WebProxy/src/System/Net/WebProxy.Browser.cs [new file with mode: 0644]
src/libraries/System.Net.WebProxy/src/System/Net/WebProxy.NonBrowser.cs [new file with mode: 0644]
src/libraries/System.Net.WebProxy/src/System/Net/WebProxy.cs

index bd89d8a..5fd9532 100644 (file)
@@ -1,12 +1,14 @@
 <Project Sdk="Microsoft.NET.Sdk">
   <PropertyGroup>
     <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
-    <TargetFrameworks>$(NetCoreAppCurrent)</TargetFrameworks>
+    <TargetFrameworks>$(NetCoreAppCurrent);$(NetCoreAppCurrent)-Browser</TargetFrameworks>
     <Nullable>enable</Nullable>
   </PropertyGroup>
   <ItemGroup>
     <Compile Include="System\Net\IWebProxyScript.cs" />
     <Compile Include="System\Net\WebProxy.cs" />
+    <Compile Condition="'$(TargetsBrowser)' == 'true'" Include="System\Net\WebProxy.Browser.cs" />
+    <Compile Condition="'$(TargetsBrowser)' != 'true'" Include="System\Net\WebProxy.NonBrowser.cs" />
   </ItemGroup>
   <ItemGroup>
     <Reference Include="System.Net.NameResolution" />
@@ -15,5 +17,6 @@
     <Reference Include="System.Runtime" />
     <Reference Include="System.Runtime.Extensions" />
     <Reference Include="System.Text.RegularExpressions" />
+    <Reference Include="System.Threading" />
   </ItemGroup>
 </Project>
diff --git a/src/libraries/System.Net.WebProxy/src/System/Net/WebProxy.Browser.cs b/src/libraries/System.Net.WebProxy/src/System/Net/WebProxy.Browser.cs
new file mode 100644 (file)
index 0000000..38fb0d0
--- /dev/null
@@ -0,0 +1,23 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System.Runtime.Serialization;
+
+namespace System.Net
+{
+    public partial class WebProxy : IWebProxy, ISerializable
+    {
+        private bool IsLocal(Uri host)
+        {
+            if (host.IsLoopback)
+            {
+                return true;
+            }
+
+            string hostString = host.Host;
+            return
+                !IPAddress.TryParse(hostString, out _) &&
+                !hostString.Contains('.');
+        }
+    }
+}
diff --git a/src/libraries/System.Net.WebProxy/src/System/Net/WebProxy.NonBrowser.cs b/src/libraries/System.Net.WebProxy/src/System/Net/WebProxy.NonBrowser.cs
new file mode 100644 (file)
index 0000000..33894f9
--- /dev/null
@@ -0,0 +1,76 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System.Net.NetworkInformation;
+using System.Runtime.Serialization;
+using System.Threading;
+
+namespace System.Net
+{
+    public partial class WebProxy : IWebProxy, ISerializable
+    {
+        private static volatile string? s_domainName;
+        private static volatile IPAddress[]? s_localAddresses;
+        private static int s_networkChangeRegistered;
+
+        private bool IsLocal(Uri host)
+        {
+            if (host.IsLoopback)
+            {
+                return true;
+            }
+
+            string hostString = host.Host;
+
+            if (IPAddress.TryParse(hostString, out IPAddress? hostAddress))
+            {
+                EnsureNetworkChangeRegistration();
+                IPAddress[] localAddresses = s_localAddresses ??= Dns.GetHostEntry(Dns.GetHostName()).AddressList;
+                return Array.IndexOf(localAddresses, hostAddress) != -1;
+            }
+
+            // No dot?  Local.
+            int dot = hostString.IndexOf('.');
+            if (dot == -1)
+            {
+                return true;
+            }
+
+            // If it matches the primary domain, it's local (whether or not the hostname matches).
+            EnsureNetworkChangeRegistration();
+            string local = s_domainName ??= "." + IPGlobalProperties.GetIPGlobalProperties().DomainName;
+            return
+                local.Length == (hostString.Length - dot) &&
+                string.Compare(local, 0, hostString, dot, local.Length, StringComparison.OrdinalIgnoreCase) == 0;
+        }
+
+        /// <summary>Ensures we've registered with NetworkChange to clear out statically-cached state upon a network change notification.</summary>
+        private static void EnsureNetworkChangeRegistration()
+        {
+            if (s_networkChangeRegistered == 0)
+            {
+                Register();
+
+                static void Register()
+                {
+                    if (Interlocked.Exchange(ref s_networkChangeRegistered, 1) != 0)
+                    {
+                        return;
+                    }
+
+                    // Clear out cached state when we get notification of a network-related change.
+                    NetworkChange.NetworkAddressChanged += (s, e) =>
+                    {
+                        s_domainName = null;
+                        s_localAddresses = null;
+                    };
+                    NetworkChange.NetworkAvailabilityChanged += (s, e) =>
+                    {
+                        s_domainName = null;
+                        s_localAddresses = null;
+                    };
+                }
+            }
+        }
+    }
+}
index a5db1a0..2a35d10 100644 (file)
@@ -4,14 +4,12 @@
 using System.Collections;
 using System.Diagnostics.CodeAnalysis;
 using System.Globalization;
-using System.Net.NetworkInformation;
 using System.Runtime.Serialization;
-using System.Runtime.Versioning;
 using System.Text.RegularExpressions;
 
 namespace System.Net
 {
-    public class WebProxy : IWebProxy, ISerializable
+    public partial class WebProxy : IWebProxy, ISerializable
     {
         private ArrayList? _bypassList;
         private Regex[]? _regexBypassList;
@@ -68,7 +66,7 @@ namespace System.Net
         [AllowNull]
         public string[] BypassList
         {
-            get { return _bypassList != null ? (string[])_bypassList.ToArray(typeof(string)) : Array.Empty<string>(); }
+            get => _bypassList != null ? (string[])_bypassList.ToArray(typeof(string)) : Array.Empty<string>();
             set
             {
                 _bypassList = value != null ? new ArrayList(value) : null;
@@ -82,8 +80,8 @@ namespace System.Net
 
         public bool UseDefaultCredentials
         {
-            get { return Credentials == CredentialCache.DefaultCredentials; }
-            set { Credentials = value ? CredentialCache.DefaultCredentials : null; }
+            get => Credentials == CredentialCache.DefaultCredentials;
+            set => Credentials = value ? CredentialCache.DefaultCredentials : null;
         }
 
         public Uri? GetProxy(Uri destination)
@@ -132,13 +130,13 @@ namespace System.Net
 
         private bool IsMatchInBypassList(Uri input)
         {
-            UpdateRegexList(false);
+            UpdateRegexList(canThrow: false);
 
             if (_regexBypassList != null)
             {
                 string matchUriString = input.IsDefaultPort ?
-                    input.Scheme + "://" + input.Host :
-                    input.Scheme + "://" + input.Host + ":" + input.Port.ToString();
+                    $"{input.Scheme}://{input.Host}" :
+                    $"{input.Scheme}://{input.Host}:{(uint)input.Port}";
 
                 foreach (Regex r in _regexBypassList)
                 {
@@ -152,54 +150,6 @@ namespace System.Net
             return false;
         }
 
-        private bool IsLocal(Uri host)
-        {
-            if (host.IsLoopback)
-            {
-                return true;
-            }
-
-            string hostString = host.Host;
-
-#pragma warning disable CA1416 // Validate platform compatibility, issue: https://github.com/dotnet/runtime/issues/43751
-            if (IPAddress.TryParse(hostString, out IPAddress? hostAddress))
-            {
-                return IPAddress.IsLoopback(hostAddress) || IsAddressLocal(hostAddress);
-            }
-
-            // No dot?  Local.
-            int dot = hostString.IndexOf('.');
-            if (dot == -1)
-            {
-                return true;
-            }
-
-            // If it matches the primary domain, it's local.  (Whether or not the hostname matches.)
-            string local = "." + IPGlobalProperties.GetIPGlobalProperties().DomainName;
-#pragma warning restore CA1416 // Validate platform compatibility
-            return
-                local.Length == (hostString.Length - dot) &&
-                string.Compare(local, 0, hostString, dot, local.Length, StringComparison.OrdinalIgnoreCase) == 0;
-        }
-
-        [UnsupportedOSPlatform("browser")]
-        private static bool IsAddressLocal(IPAddress ipAddress)
-        {
-            // Perf note: The .NET Framework caches this and then uses network change notifications to track
-            // whether the set should be recomputed.  We could consider doing the same if this is observed as
-            // a bottleneck, but that tracking has its own costs.
-            IPAddress[] localAddresses = Dns.GetHostEntry(Dns.GetHostName()).AddressList;
-            for (int i = 0; i < localAddresses.Length; i++)
-            {
-                if (ipAddress.Equals(localAddresses[i]))
-                {
-                    return true;
-                }
-            }
-
-            return false;
-        }
-
         public bool IsBypassed(Uri host)
         {
             if (host == null)
@@ -213,27 +163,19 @@ namespace System.Net
                 IsMatchInBypassList(host);
         }
 
-        protected WebProxy(SerializationInfo serializationInfo, StreamingContext streamingContext)
-        {
+        protected WebProxy(SerializationInfo serializationInfo, StreamingContext streamingContext) =>
             throw new PlatformNotSupportedException();
-        }
 
-        void ISerializable.GetObjectData(SerializationInfo serializationInfo, StreamingContext streamingContext)
-        {
+        void ISerializable.GetObjectData(SerializationInfo serializationInfo, StreamingContext streamingContext) =>
             throw new PlatformNotSupportedException();
-        }
 
-        protected virtual void GetObjectData(SerializationInfo serializationInfo, StreamingContext streamingContext)
-        {
+        protected virtual void GetObjectData(SerializationInfo serializationInfo, StreamingContext streamingContext) =>
             throw new PlatformNotSupportedException();
-        }
 
         [Obsolete("This method has been deprecated. Please use the proxy selected for you by default. https://go.microsoft.com/fwlink/?linkid=14202")]
-        public static WebProxy GetDefaultProxy()
-        {
+        public static WebProxy GetDefaultProxy() =>
             // The .NET Framework here returns a proxy that fetches IE settings and
             // executes JavaScript to determine the correct proxy.
             throw new PlatformNotSupportedException();
-        }
     }
 }