[Fixes #7085] ApplicationModelConventionExtensions should make a copy of collections when iterating them

src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/ApplicationModelConventionExtensions.cs

@@ -2,6 +2,7 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
+using System.Linq;
 using System.Collections.Generic;
 using Microsoft.AspNetCore.Mvc.ApplicationModels;
 
@@ -148,11 +149,16 @@ namespace Microsoft.Extensions.DependencyInjection
                     throw new ArgumentNullException(nameof(application));
                 }
 
-                foreach (var controller in application.Controllers)
+                // Create copies of collections of controllers, actions and parameters as users could modify
+                // these collections from within the convention itself.
+                var controllers = application.Controllers.ToArray();
+                foreach (var controller in controllers)
                 {
-                    foreach (var action in controller.Actions)
+                    var actions = controller.Actions.ToArray();
+                    foreach (var action in actions)
                     {
-                        foreach (var parameter in action.Parameters)
+                        var parameters = action.Parameters.ToArray();
+                        foreach (var parameter in parameters)
                         {
                             _parameterModelConvention.Apply(parameter);
                         }
@@ -183,9 +189,13 @@ namespace Microsoft.Extensions.DependencyInjection
                     throw new ArgumentNullException(nameof(application));
                 }
 
-                foreach (var controller in application.Controllers)
+                // Create copies of collections of controllers, actions and parameters as users could modify
+                // these collections from within the convention itself.
+                var controllers = application.Controllers.ToArray();
+                foreach (var controller in controllers)
                 {
-                    foreach (var action in controller.Actions)
+                    var actions = controller.Actions.ToArray();
+                    foreach (var action in actions)
                     {
                         _actionModelConvention.Apply(action);
                     }
@@ -215,7 +225,8 @@ namespace Microsoft.Extensions.DependencyInjection
                     throw new ArgumentNullException(nameof(application));
                 }
 
-                foreach (var controller in application.Controllers)
+                var controllers = application.Controllers.ToArray();
+                foreach (var controller in controllers)
                 {
                     _controllerModelConvention.Apply(controller);
                 }

test/Microsoft.AspNetCore.Mvc.Core.Test/DependencyInjection/ApplicationModelConventionExtensionsTest.cs

@@ -6,6 +6,7 @@ using System.Collections.Generic;
 using System.Reflection;
 using Microsoft.AspNetCore.Mvc;
 using Microsoft.AspNetCore.Mvc.ApplicationModels;
+using Microsoft.AspNetCore.Mvc.Internal;
 using Xunit;
 
 namespace Microsoft.Extensions.DependencyInjection
@@ -108,6 +109,82 @@ namespace Microsoft.Extensions.DependencyInjection
             Assert.IsType<BarApplicationModelConvention>(convention);
         }
 
+        [Fact]
+        public void ApplicationModelConventions_CopiesControllerModelCollectionOnApply()
+        {
+            // Arrange
+            var applicationModel = new ApplicationModel();
+            applicationModel.Controllers.Add(
+                new ControllerModel(typeof(HelloController).GetTypeInfo(), new List<object>())
+                {
+                    Application = applicationModel
+                });
+
+            var controllerModelConvention = new ControllerModelCollectionModifyingConvention();
+            var conventions = new List<IApplicationModelConvention>();
+            conventions.Add(controllerModelConvention);
+
+            // Act & Assert
+            ApplicationModelConventions.ApplyConventions(applicationModel, conventions);
+        }
+
+        [Fact]
+        public void ApplicationModelConventions_CopiesActionModelCollectionOnApply()
+        {
+            // Arrange
+            var controllerType = typeof(HelloController).GetTypeInfo();
+            var applicationModel = new ApplicationModel();
+            var controllerModel = new ControllerModel(controllerType, new List<object>())
+            {
+                Application = applicationModel
+            };
+            controllerModel.Actions.Add(
+                new ActionModel(controllerType.GetMethod(nameof(HelloController.GetHello)), new List<object>())
+                {
+                    Controller = controllerModel
+                });
+            applicationModel.Controllers.Add(controllerModel);
+
+            var actionModelConvention = new ActionModelCollectionModifyingConvention();
+            var conventions = new List<IApplicationModelConvention>();
+            conventions.Add(actionModelConvention);
+
+            // Act & Assert
+            ApplicationModelConventions.ApplyConventions(applicationModel, conventions);
+        }
+
+        [Fact]
+        public void ApplicationModelConventions_CopiesParameterModelCollectionOnApply()
+        {
+            // Arrange
+            var controllerType = typeof(HelloController).GetTypeInfo();
+            var app = new ApplicationModel();
+            var controllerModel = new ControllerModel(controllerType, new List<object>())
+            {
+                Application = app
+            };
+            app.Controllers.Add(controllerModel);
+            var actionModel = new ActionModel(controllerType.GetMethod(nameof(HelloController.GetInfo)), new List<object>())
+            {
+                Controller = controllerModel
+            };
+            controllerModel.Actions.Add(actionModel);
+            var parameterModel = new ParameterModel(
+                controllerType.GetMethod(nameof(HelloController.GetInfo)).GetParameters()[0],
+                new List<object>())
+            {
+                Action = actionModel
+            };
+            actionModel.Parameters.Add(parameterModel);
+
+            var parameterModelConvention = new ParameterModelCollectionModifyingConvention();
+            var conventions = new List<IApplicationModelConvention>();
+            conventions.Add(parameterModelConvention);
+
+            // Act & Assert
+            ApplicationModelConventions.ApplyConventions(app, conventions);
+        }
+
         [Fact]
         public void GenericRemoveType_RemovesAllOfType()
         {
@@ -149,6 +226,11 @@ namespace Microsoft.Extensions.DependencyInjection
             {
                 return "Hello";
             }
+
+            public string GetInfo(int id)
+            {
+                return "GetInfo(int id)";
+            }
         }
 
         private class WorldController
@@ -182,5 +264,37 @@ namespace Microsoft.Extensions.DependencyInjection
                 controller.Properties.Add("TestProperty", "TestValue");
             }
         }
+
+        private class ControllerModelCollectionModifyingConvention : IControllerModelConvention
+        {
+            public void Apply(ControllerModel controller)
+            {
+                controller.Application.Controllers.Remove(controller);
+            }
+        }
+
+        private class TestApplicationModelConvention : IApplicationModelConvention
+        {
+            public void Apply(ApplicationModel application)
+            {
+                application.Controllers.RemoveAt(0);
+            }
+        }
+
+        private class ActionModelCollectionModifyingConvention : IActionModelConvention
+        {
+            public void Apply(ActionModel action)
+            {
+                action.Controller.Actions.Remove(action);
+            }
+        }
+
+        private class ParameterModelCollectionModifyingConvention : IParameterModelConvention
+        {
+            public void Apply(ParameterModel parameter)
+            {
+                parameter.Action.Parameters.Remove(parameter);
+            }
+        }
     }
 }