diff --git a/src/Components/Components/src/Routing/SupplyParameterFromQueryValueProvider.cs b/src/Components/Components/src/Routing/SupplyParameterFromQueryValueProvider.cs index 9e1968a48c3b..eeae2040d2ed 100644 --- a/src/Components/Components/src/Routing/SupplyParameterFromQueryValueProvider.cs +++ b/src/Components/Components/src/Routing/SupplyParameterFromQueryValueProvider.cs @@ -17,6 +17,7 @@ internal sealed class SupplyParameterFromQueryValueProvider(NavigationManager na private string? _lastUri; private bool _isSubscribedToLocationChanges; private bool _queryChanged; + private bool _isProcessingLocationChange; public bool IsFixed => false; @@ -34,7 +35,7 @@ public bool CanSupplyValue(in CascadingParameterInfo parameterInfo) public void Subscribe(ComponentState subscriber, in CascadingParameterInfo parameterInfo) { - if (_pendingSubscribers?.Count > 0 || (TryUpdateUri() && _isSubscribedToLocationChanges)) + if (_pendingSubscribers?.Count > 0 || (TryUpdateUri() && _isSubscribedToLocationChanges) || _isProcessingLocationChange) { // Renderer.RenderInExistingBatch triggers Unsubscribe via ProcessDisposalQueueInExistingBatch after subscribing with any new components, // so this branch should be taken iff there's a pending OnLocationChanged event for the current Uri that we're already subscribed to. @@ -125,26 +126,34 @@ private void OnLocationChanged(object? sender, LocationChangedEventArgs args) { Debug.Assert(_subscribers is not null); - TryUpdateUri(); - - if (_queryChanged) + _isProcessingLocationChange = true; + try { - foreach (var subscriber in _subscribers) + TryUpdateUri(); + + if (_queryChanged) { - subscriber.NotifyCascadingValueChanged(ParameterViewLifetime.Unbound); - } + foreach (var subscriber in _subscribers) + { + subscriber.NotifyCascadingValueChanged(ParameterViewLifetime.Unbound); + } - _queryChanged = false; - } + _queryChanged = false; + } - if (_pendingSubscribers is not null) - { - foreach (var subscriber in _pendingSubscribers) + if (_pendingSubscribers is not null) { - _subscribers.Add(subscriber); - } + foreach (var subscriber in _pendingSubscribers) + { + _subscribers.Add(subscriber); + } - _pendingSubscribers.Clear(); + _pendingSubscribers.Clear(); + } + } + finally + { + _isProcessingLocationChange = false; } } diff --git a/src/Components/Components/test/Routing/SupplyParameterFromQueryValueProviderTest.cs b/src/Components/Components/test/Routing/SupplyParameterFromQueryValueProviderTest.cs new file mode 100644 index 000000000000..fafbf040cf91 --- /dev/null +++ b/src/Components/Components/test/Routing/SupplyParameterFromQueryValueProviderTest.cs @@ -0,0 +1,104 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +using Microsoft.AspNetCore.Components.Rendering; +using Microsoft.AspNetCore.Components.Test.Helpers; +using Microsoft.Extensions.DependencyInjection; + +namespace Microsoft.AspNetCore.Components.Routing; + +public class SupplyParameterFromQueryValueProviderTest +{ + [Fact] + public void NavigationWithNestedComponentsDoesNotThrowCollectionModifiedException() + { + // This test reproduces the bug where navigating causes a child component with + // [SupplyParameterFromQuery] to be rendered, which modifies the subscribers collection + // while it's being enumerated during OnLocationChanged + + // Arrange + var serviceCollection = new ServiceCollection(); + var navigationManager = new FakeNavigationManager(); + serviceCollection.AddSingleton(navigationManager); + serviceCollection.AddSupplyValueFromQueryProvider(); + var services = serviceCollection.BuildServiceProvider(); + + var renderer = new TestRenderer(services); + + // Create a parent component that conditionally renders a child based on query parameter + var parentComponent = new ConditionalParentComponent(); + var parentComponentId = renderer.AssignRootComponentId(parentComponent); + + // Initial render - parent without child + navigationManager.NotifyLocationChanged("http://localhost/test", false); + parentComponent.TriggerRender(); + + // Assert initial state + Assert.Single(renderer.Batches); + + // Act - Navigate to URL that causes child to be rendered + // This should trigger OnLocationChanged which enumerates subscribers, + // and the child component subscribes during that enumeration + var exception = Record.Exception(() => + { + navigationManager.NotifyLocationChanged("http://localhost/test?parentParam=showChild", false); + }); + + // Assert - No exception should be thrown + Assert.Null(exception); + + // Verify the components rendered correctly + Assert.True(renderer.Batches.Count >= 2, "Should have rendered at least twice"); + } + + private class FakeNavigationManager : NavigationManager + { + public FakeNavigationManager() + { + Initialize("http://localhost/", "http://localhost/test"); + } + + public void NotifyLocationChanged(string uri, bool isInterceptedLink) + { + Uri = uri; + NotifyLocationChanged(isInterceptedLink); + } + + protected override void NavigateToCore(string uri, NavigationOptions options) + { + throw new NotImplementedException(); + } + } + + private class ConditionalParentComponent : AutoRenderComponent + { + [SupplyParameterFromQuery] + public string ParentParam { get; set; } + + protected override void BuildRenderTree(RenderTreeBuilder builder) + { + builder.OpenElement(0, "div"); + builder.AddContent(1, $"Parent: {ParentParam}"); + + if (ParentParam == "showChild") + { + builder.OpenComponent(2); + builder.CloseComponent(); + } + + builder.CloseElement(); + } + } + + private class ChildWithQueryParamComponent : AutoRenderComponent + { + [SupplyParameterFromQuery] + public string ChildParam { get; set; } + + protected override void BuildRenderTree(RenderTreeBuilder builder) + { + builder.OpenElement(0, "div"); + builder.AddContent(1, $"Child: {ChildParam}"); + builder.CloseElement(); + } + } +}