Skip to content

Conversation

sebastienros
Copy link
Member

@sebastienros sebastienros commented Sep 12, 2025

Description

This PR is introducing a new property and obsoleting another. They both represent the same concept and are both used in the codebase. However users will only be able to update the new one, and clearing it won't affect the previous one which is still used throughout the code.

This PR adds tests to ensure that the expected behavior is not broken when using a single property. The previous tests were manipulating both, which users can't do. They successfully raise failing tests.
It's then adding a new collection that will back the two properties, such that altering either property will have the same effect (share collection).

Fixes #63627

Customer Impact

Http Overrides features is broken, failing customers app during auth.

Regression?

  • Yes
  • No

In 10.0-rc1

Risk

  • High
  • Medium
  • Low

The other option is to revert the obsoletion of the previous property described in this announcement

Verification

  • Manual (required)
  • Automated

@Copilot Copilot AI review requested due to automatic review settings September 12, 2025 16:02
@sebastienros
Copy link
Member Author

/cc @WeihanLi

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR implements a dual-list system for managing known networks in ForwardedHeadersOptions to ensure compatibility between the new KnownIPNetworks property and the obsolete KnownNetworks property. The changes ensure that modifications to either property are reflected in both, addressing issue #63627 where clearing one property didn't affect the other.

Key Changes

  • Introduces a new DualIPNetworkList class that synchronizes changes between both property types
  • Updates ForwardedHeadersOptions to use the dual-list as backing storage for both properties
  • Adds comprehensive tests to verify the synchronization behavior works correctly

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/Middleware/HttpOverrides/src/DualIPNetworkList.cs New dual-list implementation that maintains sync between System.Net.IPNetwork and obsolete AspNet IPNetwork types
src/Middleware/HttpOverrides/src/ForwardedHeadersOptions.cs Updates to use the dual-list as backing storage for both KnownIPNetworks and KnownNetworks properties
src/Middleware/HttpOverrides/src/ForwardedHeadersMiddleware.cs Simplifies known IP checking logic by removing redundant obsolete collection checks
src/Middleware/HttpOverrides/test/ForwardedHeadersMiddlewareTest.cs Adds new test variants to verify behavior when using only the new property API
src/Middleware/HttpOverrides/test/DualIPNetworkListTests.cs New test file covering dual-list synchronization scenarios
src/DefaultBuilder/src/ForwardedHeadersOptionsSetup.cs Removes redundant clearing of obsolete property since dual-list keeps them in sync

Copy link
Member

@joperezr joperezr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a minor question but this LGTM, thanks a bunch @sebastienros.

@danmoseley FYI this needs approval for RC2 merge. (fixing a regression introduced after one of the obsoletions we did in P7)

@danmoseley
Copy link
Member

Fixing regression - approved.

@danmoseley danmoseley added Servicing-approved Shiproom has approved the issue and removed Servicing-consider Shiproom approval is required for the issue labels Sep 15, 2025
@sebastienros
Copy link
Member Author

Summoning @wtgodbe or @captainsafia for your merging powers

@sebastienros
Copy link
Member Author

/backport to main

Copy link
Contributor

Started backporting to main: https://github.com/dotnet/aspnetcore/actions/runs/17777371720

@github-actions github-actions bot mentioned this pull request Sep 16, 2025
11 tasks
@sebastienros
Copy link
Member Author

I created a PR for main but if we expect to merge release/10.0 to main soon then it's useless.

@WeihanLi
Copy link
Contributor

I created a PR for main but if we expect to merge release/10.0 to main soon then it's useless.

Would the obsolete members be removed on .NET 11? Maybe no need to backport if we plan to remove in the next Major version.

@joperezr joperezr merged commit 72692ad into release/10.0 Sep 17, 2025
28 checks passed
@joperezr joperezr deleted the sebros/knownnetworks branch September 17, 2025 16:43
@dotnet-policy-service dotnet-policy-service bot added this to the 10.0-rc2 milestone Sep 17, 2025
wtgodbe added a commit that referenced this pull request Sep 19, 2025
* [release/10.0] Implement KnownNetworks dual list (#63658)

* Refactor unit tests to handle the obsolete property

* Implement KnownNetworks dual list

Fixes #63627

* Update dependencies from https://dev.azure.com/dnceng/internal/_git/dotnet-optimization build 20250908.1 (#63679)

On relative base path root
optimization.linux-arm64.MIBC.Runtime , optimization.linux-x64.MIBC.Runtime , optimization.windows_nt-arm64.MIBC.Runtime , optimization.windows_nt-x64.MIBC.Runtime , optimization.windows_nt-x86.MIBC.Runtime From Version 1.0.0-prerelease.25453.1 -> To Version 1.0.0-prerelease.25458.1

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>

* Update `Microsoft.Identity.Web` dependencies (#63705)

Co-authored-by: Mackinnon Buck <[email protected]>

* Update dependencies from https://github.com/dotnet/dotnet build 283666 (#63712)

Updated Dependencies:
Microsoft.NET.Runtime.WebAssembly.Sdk, Microsoft.NETCore.BrowserDebugHost.Transport, Microsoft.NET.Runtime.MonoAOTCompiler.Task, dotnet-ef, Microsoft.Bcl.AsyncInterfaces, Microsoft.Bcl.TimeProvider, Microsoft.EntityFrameworkCore, Microsoft.EntityFrameworkCore.Design, Microsoft.EntityFrameworkCore.InMemory, Microsoft.EntityFrameworkCore.Relational, Microsoft.EntityFrameworkCore.Sqlite, Microsoft.EntityFrameworkCore.SqlServer, Microsoft.EntityFrameworkCore.Tools, Microsoft.Extensions.Caching.Abstractions, Microsoft.Extensions.Caching.Memory, Microsoft.Extensions.Configuration, Microsoft.Extensions.Configuration.Abstractions, Microsoft.Extensions.Configuration.Binder, Microsoft.Extensions.Configuration.CommandLine, Microsoft.Extensions.Configuration.EnvironmentVariables, Microsoft.Extensions.Configuration.FileExtensions, Microsoft.Extensions.Configuration.Ini, Microsoft.Extensions.Configuration.Json, Microsoft.Extensions.Configuration.UserSecrets, Microsoft.Extensions.Configuration.Xml, Microsoft.Extensions.DependencyInjection, Microsoft.Extensions.DependencyInjection.Abstractions, Microsoft.Extensions.DependencyModel, Microsoft.Extensions.Diagnostics, Microsoft.Extensions.Diagnostics.Abstractions, Microsoft.Extensions.FileProviders.Abstractions, Microsoft.Extensions.FileProviders.Composite, Microsoft.Extensions.FileProviders.Physical, Microsoft.Extensions.FileSystemGlobbing, Microsoft.Extensions.HostFactoryResolver.Sources, Microsoft.Extensions.Hosting, Microsoft.Extensions.Hosting.Abstractions, Microsoft.Extensions.Http, Microsoft.Extensions.Logging, Microsoft.Extensions.Logging.Abstractions, Microsoft.Extensions.Logging.Configuration, Microsoft.Extensions.Logging.Console, Microsoft.Extensions.Logging.Debug, Microsoft.Extensions.Logging.EventLog, Microsoft.Extensions.Logging.EventSource, Microsoft.Extensions.Logging.TraceSource, Microsoft.Extensions.Options, Microsoft.Extensions.Options.ConfigurationExtensions, Microsoft.Extensions.Options.DataAnnotations, Microsoft.Extensions.Primitives, Microsoft.Internal.Runtime.AspNetCore.Transport, Microsoft.NETCore.App.Ref, Microsoft.NETCore.Platforms, System.Collections.Immutable, System.Composition, System.Configuration.ConfigurationManager, System.Diagnostics.DiagnosticSource, System.Diagnostics.EventLog, System.Diagnostics.PerformanceCounter, System.DirectoryServices.Protocols, System.Formats.Asn1, System.Formats.Cbor, System.IO.Hashing, System.IO.Pipelines, System.Memory.Data, System.Net.Http.Json, System.Net.Http.WinHttpHandler, System.Net.ServerSentEvents, System.Numerics.Tensors, System.Reflection.Metadata, System.Resources.Extensions, System.Runtime.Caching, System.Security.Cryptography.Pkcs, System.Security.Cryptography.Xml, System.Security.Permissions, System.ServiceProcess.ServiceController, System.Text.Encodings.Web, System.Text.Json, System.Threading.AccessControl, System.Threading.Channels, System.Threading.RateLimiting (Version 10.0.0-rc.2.25466.101 -> 10.0.0-rc.2.25467.107)
Microsoft.DotNet.Arcade.Sdk, Microsoft.DotNet.Build.Tasks.Archives, Microsoft.DotNet.Build.Tasks.Installers, Microsoft.DotNet.Build.Tasks.Templating, Microsoft.DotNet.Helix.Sdk, Microsoft.DotNet.RemoteExecutor, Microsoft.DotNet.SharedFramework.Sdk (Version 10.0.0-beta.25466.101 -> 10.0.0-beta.25467.107)
Microsoft.Web.Xdt (Version 3.2.0-preview.25466.101 -> 3.2.0-preview.25467.107)
NuGet.Frameworks, NuGet.Packaging, NuGet.Versioning (Version 7.0.0-preview.2.46701 -> 7.0.0-preview.2.46807)

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>

* [release/10.0] Fix ComponentStatePersistenceManager iteration to prevent AntiforgeryValidationException in Blazor WASM (#63694)

* Initial plan

* Fix ComponentStatePersistenceManager to iterate backwards in InferRenderModes

Co-authored-by: javiercn <[email protected]>

* Add unit test for ComponentStatePersistenceManager callback removal during iteration

Co-authored-by: javiercn <[email protected]>

---------

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: javiercn <[email protected]>

* [release/10.0] [Blazor] Clear RootTypeCache cache on HotReload (#63653)

* Clear RootTypeCache cache on HotReload

* ClearCache method

* Fix

---------

Co-authored-by: Marek Fišera <[email protected]>

* Resolve conflicts (#63711)

* [release/10.0] Fix Validation source generator deployment for non-Web SDKs (#63715)

* Proof-of-concept fix for getting validation SG to work in Blazor Wasm SDK projects

* Use special type symbol for missing type comparisons

---------

Co-authored-by: Ondřej Roztočil <[email protected]>

* [release/10.0] Enable spectre mitigations for ANCM binaries (#63728)

* Enable spectre mitigations for ANCM binaries

* Set property as well

* Revert "Set property as well"

This reverts commit b573d8a.

---------

Co-authored-by: wtgodbe <[email protected]>

* Update dependencies from https://github.com/dotnet/dotnet build 283828 (#63732)

Updated Dependencies:
Microsoft.NET.Runtime.WebAssembly.Sdk, Microsoft.NETCore.BrowserDebugHost.Transport, Microsoft.NET.Runtime.MonoAOTCompiler.Task, dotnet-ef, Microsoft.Bcl.AsyncInterfaces, Microsoft.Bcl.TimeProvider, Microsoft.EntityFrameworkCore, Microsoft.EntityFrameworkCore.Design, Microsoft.EntityFrameworkCore.InMemory, Microsoft.EntityFrameworkCore.Relational, Microsoft.EntityFrameworkCore.Sqlite, Microsoft.EntityFrameworkCore.SqlServer, Microsoft.EntityFrameworkCore.Tools, Microsoft.Extensions.Caching.Abstractions, Microsoft.Extensions.Caching.Memory, Microsoft.Extensions.Configuration, Microsoft.Extensions.Configuration.Abstractions, Microsoft.Extensions.Configuration.Binder, Microsoft.Extensions.Configuration.CommandLine, Microsoft.Extensions.Configuration.EnvironmentVariables, Microsoft.Extensions.Configuration.FileExtensions, Microsoft.Extensions.Configuration.Ini, Microsoft.Extensions.Configuration.Json, Microsoft.Extensions.Configuration.UserSecrets, Microsoft.Extensions.Configuration.Xml, Microsoft.Extensions.DependencyInjection, Microsoft.Extensions.DependencyInjection.Abstractions, Microsoft.Extensions.DependencyModel, Microsoft.Extensions.Diagnostics, Microsoft.Extensions.Diagnostics.Abstractions, Microsoft.Extensions.FileProviders.Abstractions, Microsoft.Extensions.FileProviders.Composite, Microsoft.Extensions.FileProviders.Physical, Microsoft.Extensions.FileSystemGlobbing, Microsoft.Extensions.HostFactoryResolver.Sources, Microsoft.Extensions.Hosting, Microsoft.Extensions.Hosting.Abstractions, Microsoft.Extensions.Http, Microsoft.Extensions.Logging, Microsoft.Extensions.Logging.Abstractions, Microsoft.Extensions.Logging.Configuration, Microsoft.Extensions.Logging.Console, Microsoft.Extensions.Logging.Debug, Microsoft.Extensions.Logging.EventLog, Microsoft.Extensions.Logging.EventSource, Microsoft.Extensions.Logging.TraceSource, Microsoft.Extensions.Options, Microsoft.Extensions.Options.ConfigurationExtensions, Microsoft.Extensions.Options.DataAnnotations, Microsoft.Extensions.Primitives, Microsoft.Internal.Runtime.AspNetCore.Transport, Microsoft.NETCore.App.Ref, Microsoft.NETCore.Platforms, System.Collections.Immutable, System.Composition, System.Configuration.ConfigurationManager, System.Diagnostics.DiagnosticSource, System.Diagnostics.EventLog, System.Diagnostics.PerformanceCounter, System.DirectoryServices.Protocols, System.Formats.Asn1, System.Formats.Cbor, System.IO.Hashing, System.IO.Pipelines, System.Memory.Data, System.Net.Http.Json, System.Net.Http.WinHttpHandler, System.Net.ServerSentEvents, System.Numerics.Tensors, System.Reflection.Metadata, System.Resources.Extensions, System.Runtime.Caching, System.Security.Cryptography.Pkcs, System.Security.Cryptography.Xml, System.Security.Permissions, System.ServiceProcess.ServiceController, System.Text.Encodings.Web, System.Text.Json, System.Threading.AccessControl, System.Threading.Channels, System.Threading.RateLimiting (Version 10.0.0-rc.2.25467.107 -> 10.0.0-rc.2.25468.104)
Microsoft.DotNet.Arcade.Sdk, Microsoft.DotNet.Build.Tasks.Archives, Microsoft.DotNet.Build.Tasks.Installers, Microsoft.DotNet.Build.Tasks.Templating, Microsoft.DotNet.Helix.Sdk, Microsoft.DotNet.RemoteExecutor, Microsoft.DotNet.SharedFramework.Sdk (Version 10.0.0-beta.25467.107 -> 10.0.0-beta.25468.104)
Microsoft.Web.Xdt (Version 3.2.0-preview.25467.107 -> 3.2.0-preview.25468.104)
NuGet.Frameworks, NuGet.Packaging, NuGet.Versioning (Version 7.0.0-preview.2.46807 -> 7.0.0-preview.2.46904)

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>

* Update dependencies from https://github.com/dotnet/extensions build 20250912.1 (#63681)

On relative base path root
Microsoft.Extensions.Caching.Hybrid , Microsoft.Extensions.Diagnostics.Testing , Microsoft.Extensions.TimeProvider.Testing From Version 9.10.0-preview.1.25456.3 -> To Version 9.10.0-preview.1.25462.1

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>

---------

Co-authored-by: Sébastien Ros <[email protected]>
Co-authored-by: dotnet-maestro[bot] <42748379+dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Mackinnon Buck <[email protected]>
Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: javiercn <[email protected]>
Co-authored-by: Marek Fišera <[email protected]>
Co-authored-by: William Godbe <[email protected]>
Co-authored-by: Ondřej Roztočil <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Servicing-approved Shiproom has approved the issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Changing KnownNetworks.Clear() to KnownIPNetworks.Clear() breaks forwarded headers
5 participants