Implement KnownNetworks dual list

sebastienros · sebastienros · commit f39e8043047c · 2025-09-12T08:57:53.000-07:00
Fixes #63627
diff --git a/src/DefaultBuilder/src/ForwardedHeadersOptionsSetup.cs b/src/DefaultBuilder/src/ForwardedHeadersOptionsSetup.cs
@@ -27,9 +27,6 @@ public void Configure(ForwardedHeadersOptions options)
         options.ForwardedHeaders = ForwardedHeaders.XForwardedFor | ForwardedHeaders.XForwardedProto;
         // Only loopback proxies are allowed by default. Clear that restriction because forwarders are
         // being enabled by explicit configuration.
-#pragma warning disable ASPDEPR005 // KnownNetworks is obsolete
-        options.KnownNetworks.Clear();
-#pragma warning restore ASPDEPR005 // KnownNetworks is obsolete
         options.KnownIPNetworks.Clear();
         options.KnownProxies.Clear();
     }
diff --git a/src/Middleware/HttpOverrides/src/DualIPNetworkList.cs b/src/Middleware/HttpOverrides/src/DualIPNetworkList.cs
@@ -0,0 +1,212 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+#pragma warning disable ASPDEPR005 // Type or member is obsolete
+
+using System.Globalization;
+using AspNetIPNetwork = Microsoft.AspNetCore.HttpOverrides.IPNetwork;
+using IPAddress = System.Net.IPAddress;
+using IPNetwork = System.Net.IPNetwork;
+
+namespace Microsoft.AspNetCore.Builder;
+
+/// <summary>
+/// Internal list implementation that keeps <see cref="System.Net.IPNetwork"/> and the obsolete
+/// <see cref="Microsoft.AspNetCore.HttpOverrides.IPNetwork"/> collections in sync. Modifications
+/// through either interface are reflected in the other.
+/// </summary>
+internal sealed class DualIPNetworkList : IList<IPNetwork>, IList<AspNetIPNetwork>
+{
+    private readonly List<(IPNetwork system, AspNetIPNetwork aspnet)> _items = new();
+
+    public DualIPNetworkList()
+    {
+        Add(new IPNetwork(IPAddress.Loopback, 8));
+    }
+
+    int ICollection<IPNetwork>.Count => _items.Count;
+    int ICollection<AspNetIPNetwork>.Count => _items.Count;
+
+    bool ICollection<IPNetwork>.IsReadOnly => false;
+    bool ICollection<AspNetIPNetwork>.IsReadOnly => false;
+
+    IPNetwork IList<IPNetwork>.this[int index]
+    {
+        get => _items[index].system;
+        set => _items[index] = (value, CreateAspNetNetworkFromSystem(value));
+    }
+
+    AspNetIPNetwork IList<AspNetIPNetwork>.this[int index]
+    {
+        get => _items[index].aspnet;
+        set => _items[index] = (new IPNetwork(value.Prefix, value.PrefixLength), value);
+    }
+
+    public void Add(IPNetwork item)
+    {
+        _items.Add((item, CreateAspNetNetworkFromSystem(item)));
+    }
+
+    public void Add(AspNetIPNetwork item)
+    {
+        _items.Add((new IPNetwork(item.Prefix, item.PrefixLength), item));
+    }
+
+    void ICollection<IPNetwork>.Add(IPNetwork item) => Add(item);
+    void ICollection<AspNetIPNetwork>.Add(AspNetIPNetwork item) => Add(item);
+
+    public void Clear() => _items.Clear();
+    void ICollection<IPNetwork>.Clear() => Clear();
+    void ICollection<AspNetIPNetwork>.Clear() => Clear();
+
+    public bool Contains(IPNetwork item)
+    {
+        var text = item.ToString();
+        for (var i = 0; i < _items.Count; i++)
+        {
+            if (string.Equals(_items[i].system.ToString(), text, StringComparison.Ordinal))
+            {
+                return true;
+            }
+        }
+        return false;
+    }
+
+    public bool Contains(AspNetIPNetwork item)
+    {
+        for (var i = 0; i < _items.Count; i++)
+        {
+            if (_items[i].aspnet.Prefix.Equals(item.Prefix) && _items[i].aspnet.PrefixLength == item.PrefixLength)
+            {
+                return true;
+            }
+        }
+        return false;
+    }
+
+    bool ICollection<IPNetwork>.Contains(IPNetwork item) => Contains(item);
+    bool ICollection<AspNetIPNetwork>.Contains(AspNetIPNetwork item) => Contains(item);
+
+    public void CopyTo(IPNetwork[] array, int arrayIndex)
+    {
+        for (var i = 0; i < _items.Count; i++)
+        {
+            array[arrayIndex + i] = _items[i].system;
+        }
+    }
+
+    public void CopyTo(AspNetIPNetwork[] array, int arrayIndex)
+    {
+        for (var i = 0; i < _items.Count; i++)
+        {
+            array[arrayIndex + i] = _items[i].aspnet;
+        }
+    }
+
+    void ICollection<IPNetwork>.CopyTo(IPNetwork[] array, int arrayIndex) => CopyTo(array, arrayIndex);
+    void ICollection<AspNetIPNetwork>.CopyTo(AspNetIPNetwork[] array, int arrayIndex) => CopyTo(array, arrayIndex);
+
+    public IEnumerator<IPNetwork> GetEnumerator()
+    {
+        for (var i = 0; i < _items.Count; i++)
+        {
+            yield return _items[i].system;
+        }
+    }
+
+    System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator() => GetEnumerator();
+
+    IEnumerator<AspNetIPNetwork> IEnumerable<AspNetIPNetwork>.GetEnumerator()
+    {
+        for (var i = 0; i < _items.Count; i++)
+        {
+            yield return _items[i].aspnet;
+        }
+    }
+
+    public int IndexOf(IPNetwork item)
+    {
+        var text = item.ToString();
+        for (var i = 0; i < _items.Count; i++)
+        {
+            if (string.Equals(_items[i].system.ToString(), text, StringComparison.Ordinal))
+            {
+                return i;
+            }
+        }
+        return -1;
+    }
+
+    public int IndexOf(AspNetIPNetwork item)
+    {
+        for (var i = 0; i < _items.Count; i++)
+        {
+            var n = _items[i].aspnet;
+            if (n.Prefix.Equals(item.Prefix) && n.PrefixLength == item.PrefixLength)
+            {
+                return i;
+            }
+        }
+        return -1;
+    }
+
+    int IList<IPNetwork>.IndexOf(IPNetwork item) => IndexOf(item);
+    int IList<AspNetIPNetwork>.IndexOf(AspNetIPNetwork item) => IndexOf(item);
+
+    public void Insert(int index, IPNetwork item)
+    {
+        _items.Insert(index, (item, CreateAspNetNetworkFromSystem(item)));
+    }
+
+    public void Insert(int index, AspNetIPNetwork item)
+    {
+        _items.Insert(index, (new IPNetwork(item.Prefix, item.PrefixLength), item));
+    }
+
+    void IList<IPNetwork>.Insert(int index, IPNetwork item) => Insert(index, item);
+    void IList<AspNetIPNetwork>.Insert(int index, AspNetIPNetwork item) => Insert(index, item);
+
+    public bool Remove(IPNetwork item)
+    {
+        var idx = IndexOf(item);
+        if (idx >= 0)
+        {
+            _items.RemoveAt(idx);
+            return true;
+        }
+        return false;
+    }
+
+    public bool Remove(AspNetIPNetwork item)
+    {
+        var idx = IndexOf(item);
+        if (idx >= 0)
+        {
+            _items.RemoveAt(idx);
+            return true;
+        }
+        return false;
+    }
+
+    bool ICollection<IPNetwork>.Remove(IPNetwork item) => Remove(item);
+    bool ICollection<AspNetIPNetwork>.Remove(AspNetIPNetwork item) => Remove(item);
+
+    public void RemoveAt(int index) => _items.RemoveAt(index);
+    void IList<IPNetwork>.RemoveAt(int index) => RemoveAt(index);
+    void IList<AspNetIPNetwork>.RemoveAt(int index) => RemoveAt(index);
+
+    private static AspNetIPNetwork CreateAspNetNetworkFromSystem(IPNetwork network)
+    {
+        var text = network.ToString();
+        var slash = text.LastIndexOf('/');
+        if (slash <= 0 || slash == text.Length - 1)
+        {
+            return new AspNetIPNetwork(IPAddress.Parse(text), 0);
+        }
+        var prefix = IPAddress.Parse(text.AsSpan(0, slash));
+        var prefixLength = int.Parse(text.AsSpan(slash + 1), CultureInfo.InvariantCulture);
+        return new AspNetIPNetwork(prefix, prefixLength);
+    }
+}
+
+#pragma warning restore ASPDEPR005 // Type or member is obsolete
diff --git a/src/Middleware/HttpOverrides/src/ForwardedHeadersMiddleware.cs b/src/Middleware/HttpOverrides/src/ForwardedHeadersMiddleware.cs
@@ -213,11 +213,7 @@ public void ApplyForwarders(HttpContext context)
             // Host and Scheme initial values are never inspected, no need to set them here.
         };
 
-        var checkKnownIps = _options.KnownIPNetworks.Count > 0
-#pragma warning disable ASPDEPR005 // KnownNetworks is obsolete
-            || _options.KnownNetworks.Count > 0
-#pragma warning restore ASPDEPR005 // KnownNetworks is obsolete
-            || _options.KnownProxies.Count > 0;
+        var checkKnownIps = _options.KnownIPNetworks.Count > 0 || _options.KnownProxies.Count > 0;
         bool applyChanges = false;
         int entriesConsumed = 0;
 
@@ -410,15 +406,6 @@ private bool CheckKnownAddress(IPAddress address)
                 return true;
             }
         }
-#pragma warning disable ASPDEPR005 // KnownNetworks is obsolete
-        foreach (var network in _options.KnownNetworks)
-        {
-            if (network.Contains(address))
-            {
-                return true;
-            }
-        }
-#pragma warning restore ASPDEPR005 // KnownNetworks is obsolete
         return false;
     }
 
diff --git a/src/Middleware/HttpOverrides/src/ForwardedHeadersOptions.cs b/src/Middleware/HttpOverrides/src/ForwardedHeadersOptions.cs
@@ -13,6 +13,9 @@ namespace Microsoft.AspNetCore.Builder;
 /// </summary>
 public class ForwardedHeadersOptions
 {
+    // Backing dual list that keeps the obsolete and new types in sync.
+    private readonly DualIPNetworkList _knownNetworks = new();
+
     /// <summary>
     /// Gets or sets the header used to retrieve the originating client IP. Defaults to the value specified by
     /// <see cref="ForwardedHeadersDefaults.XForwardedForHeaderName"/>.
@@ -87,12 +90,12 @@ public class ForwardedHeadersOptions
     /// Obsolete, please use <see cref="KnownIPNetworks"/> instead
     /// </summary>
     [Obsolete("Please use KnownIPNetworks instead. For more information, visit https://aka.ms/aspnet/deprecate/005.", DiagnosticId = "ASPDEPR005")]
-    public IList<AspNetIPNetwork> KnownNetworks { get; } = new List<AspNetIPNetwork>() { new(IPAddress.Loopback, 8) };
+    public IList<AspNetIPNetwork> KnownNetworks => _knownNetworks;
 
     /// <summary>
     /// Address ranges of known proxies to accept forwarded headers from.
     /// </summary>
-    public IList<IPNetwork> KnownIPNetworks { get; } = new List<IPNetwork>() { new(IPAddress.Loopback, 8) };
+    public IList<IPNetwork> KnownIPNetworks => _knownNetworks;
 
     /// <summary>
     /// The allowed values from x-forwarded-host. If the list is empty then all hosts are allowed.
diff --git a/src/Middleware/HttpOverrides/test/DualIPNetworkListTests.cs b/src/Middleware/HttpOverrides/test/DualIPNetworkListTests.cs
@@ -0,0 +1,95 @@
+// 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;
+using Microsoft.AspNetCore.Builder;
+using Xunit;
+using AspNetIPNetwork = Microsoft.AspNetCore.HttpOverrides.IPNetwork;
+
+namespace Microsoft.AspNetCore.HttpOverrides;
+
+public class DualIPNetworkListTests
+{
+    [Fact]
+    public void DefaultContainsLoopback()
+    {
+        var options = new ForwardedHeadersOptions();
+        Assert.Single(options.KnownIPNetworks);
+#pragma warning disable ASPDEPR005
+        Assert.Single(options.KnownNetworks);
+#pragma warning restore ASPDEPR005
+        Assert.Equal(IPAddress.Loopback, options.KnownIPNetworks[0].BaseAddress);
+        Assert.Equal(8, options.KnownIPNetworks[0].PrefixLength);
+    }
+
+    [Fact]
+    public void AddThroughSystemCollectionVisibleViaObsolete()
+    {
+        var options = new ForwardedHeadersOptions();
+        options.KnownIPNetworks.Add(global::System.Net.IPNetwork.Parse("10.0.0.0/8"));
+#pragma warning disable ASPDEPR005
+        var obsoleteList = options.KnownNetworks;
+        Assert.Equal(2, obsoleteList.Count);
+        Assert.Equal(IPAddress.Parse("10.0.0.0"), obsoleteList[1].Prefix);
+        Assert.Equal(8, obsoleteList[1].PrefixLength);
+#pragma warning restore ASPDEPR005
+    }
+
+    [Fact]
+    public void AddThroughObsoleteCollectionVisibleViaSystem()
+    {
+#pragma warning disable ASPDEPR005
+        var options = new ForwardedHeadersOptions();
+        options.KnownNetworks.Add(new AspNetIPNetwork(IPAddress.Parse("192.168.0.0"), 16));
+        Assert.Equal(2, options.KnownIPNetworks.Count);
+        Assert.Equal("192.168.0.0/16", options.KnownIPNetworks[1].ToString());
+#pragma warning restore ASPDEPR005
+    }
+
+    [Fact]
+    public void ReplaceViaSystemIndexerUpdatesObsolete()
+    {
+        var options = new ForwardedHeadersOptions();
+        ((IList<global::System.Net.IPNetwork>)options.KnownIPNetworks)[0] = global::System.Net.IPNetwork.Parse("172.16.0.0/12");
+#pragma warning disable ASPDEPR005
+        Assert.Equal(IPAddress.Parse("172.16.0.0"), options.KnownNetworks[0].Prefix);
+        Assert.Equal(12, options.KnownNetworks[0].PrefixLength);
+#pragma warning restore ASPDEPR005
+    }
+
+    [Fact]
+    public void ReplaceViaObsoleteIndexerUpdatesSystem()
+    {
+#pragma warning disable ASPDEPR005
+        var options = new ForwardedHeadersOptions();
+        options.KnownNetworks[0] = new AspNetIPNetwork(IPAddress.Parse("172.16.0.0"), 12);
+        Assert.Equal("172.16.0.0/12", options.KnownIPNetworks[0].ToString());
+#pragma warning restore ASPDEPR005
+    }
+
+    [Fact]
+    public void ClearClearsBoth()
+    {
+        var options = new ForwardedHeadersOptions();
+        options.KnownIPNetworks.Clear();
+#pragma warning disable ASPDEPR005
+        Assert.Empty(options.KnownNetworks);
+#pragma warning restore ASPDEPR005
+        Assert.Empty(options.KnownIPNetworks);
+    }
+
+    [Fact]
+    public void RemoveThroughEitherCollectionRemovesFromBoth()
+    {
+        var options = new ForwardedHeadersOptions();
+        options.KnownIPNetworks.Add(global::System.Net.IPNetwork.Parse("10.0.0.0/8"));
+        var first = options.KnownIPNetworks[0];
+        var removed = options.KnownIPNetworks.Remove(first);
+        Assert.True(removed);
+#pragma warning disable ASPDEPR005
+        var obsoleteList = options.KnownNetworks;
+        Assert.DoesNotContain(obsoleteList, n => n.Prefix.Equals(IPAddress.Loopback));
+#pragma warning restore ASPDEPR005
+        Assert.Single(options.KnownIPNetworks); // only the 10.0.0.0/8 entry should remain
+    }
+}
