Prevent StackOverflows when assemblies have recursive references (dotnet/aspnetcore-tooling#901)

* Prevent StackOverflows when assemblies have recursive references

https://github.com/aspnet/AspNetCore/issues/12693

\n\nCommit migrated from 43776c1864

src/Razor/Microsoft.NET.Sdk.Razor/src/AssemblyItem.cs

@@ -7,7 +7,7 @@ namespace Microsoft.AspNetCore.Razor.Tasks
     {
         public string Path { get; set; }
 
-        public bool IsSystemReference { get; set; }
+        public bool IsFrameworkReference { get; set; }
 
         public string AssemblyName { get; set; }
     }

src/Razor/Microsoft.NET.Sdk.Razor/src/FindAssembliesWithReferencesTo.cs

@@ -38,7 +38,7 @@ namespace Microsoft.AspNetCore.Razor.Tasks
                 referenceItems.Add(new AssemblyItem
                 {
                     AssemblyName = assemblyName,
-                    IsSystemReference = item.GetMetadata("IsSystemReference") == "true",
+                    IsFrameworkReference = item.GetMetadata("IsFrameworkReference") == "true",
                     Path = item.ItemSpec,
                 });
             }

src/Razor/Microsoft.NET.Sdk.Razor/src/ReferenceResolver.cs

@@ -3,7 +3,9 @@
 
 using System;
 using System.Collections.Generic;
+using System.Diagnostics;
 using System.IO;
+using System.Linq;
 using System.Reflection.Metadata;
 using System.Reflection.PortableExecutable;
 
@@ -15,155 +17,141 @@ namespace Microsoft.AspNetCore.Razor.Tasks
     public class ReferenceResolver
     {
         private readonly HashSet<string> _mvcAssemblies;
-        private readonly Dictionary<string, ClassifiedAssemblyItem> _lookup = new Dictionary<string, ClassifiedAssemblyItem>(StringComparer.Ordinal);
+        private readonly IReadOnlyList<AssemblyItem> _assemblyItems;
+        private readonly Dictionary<AssemblyItem, Classification> _classifications;
 
         public ReferenceResolver(IReadOnlyList<string> targetAssemblies, IReadOnlyList<AssemblyItem> assemblyItems)
         {
             _mvcAssemblies = new HashSet<string>(targetAssemblies, StringComparer.Ordinal);
+            _assemblyItems = assemblyItems;
+            _classifications = new Dictionary<AssemblyItem, Classification>();
 
+            Lookup = new Dictionary<string, AssemblyItem>(StringComparer.Ordinal);
             foreach (var item in assemblyItems)
             {
-                var classifiedItem = new ClassifiedAssemblyItem(item);
+                Lookup[item.AssemblyName] = item;
-                _lookup[item.AssemblyName] = classifiedItem;
             }
         }
 
+        protected Dictionary<string, AssemblyItem> Lookup { get; }
+
         public IReadOnlyList<string> ResolveAssemblies()
         {
             var applicationParts = new List<string>();
-            foreach (var item in _lookup)
-            {
-                var classification = Resolve(item.Value);
-                if (classification == DependencyClassification.ReferencesMvc)
-                {
-                    applicationParts.Add(item.Key);
-                }
 
-                // It's not interesting for us to know if a dependency has a classification of MvcReference.
+            foreach (var item in _assemblyItems)
-                // All applications targeting the Microsoft.AspNetCore.App will have have a reference to Mvc.
+            {
+                var classification = Resolve(item);
+                if (classification == Classification.ReferencesMvc)
+                {
+                    applicationParts.Add(item.AssemblyName);
+                }
             }
 
             return applicationParts;
         }
 
-        private DependencyClassification Resolve(ClassifiedAssemblyItem classifiedItem)
+        private Classification Resolve(AssemblyItem assemblyItem)
         {
-            if (classifiedItem.DependencyClassification != DependencyClassification.Unknown)
+            if (_classifications.TryGetValue(assemblyItem, out var classification))
             {
-                return classifiedItem.DependencyClassification;
+                return classification;
             }
 
-            if (classifiedItem.AssemblyItem == null)
+            // Initialize the dictionary with a value to short-circuit recursive references.
+            classification = Classification.Unknown;
+            _classifications[assemblyItem] = classification;
+
+            if (assemblyItem.Path == null)
             {
-                // We encountered a dependency that isn't part of this assembly's dependency set. We'll see if it happens to be an MVC assembly.
+                // We encountered a dependency that isn't part of this assembly's dependency set. We'll see if it happens to be an MVC assembly
-                // This might be useful in scenarios where the app does not have a framework reference at the entry point,
+                // since that's the only useful determination we can make given the assembly name.
-                // but the transitive dependency does.
+                classification = _mvcAssemblies.Contains(assemblyItem.AssemblyName) ?
-                classifiedItem.DependencyClassification = _mvcAssemblies.Contains(classifiedItem.Name) ?
+                    Classification.IsMvc :
-                    DependencyClassification.MvcReference :
+                    Classification.DoesNotReferenceMvc;
-                    DependencyClassification.DoesNotReferenceMvc;
-
-                return classifiedItem.DependencyClassification;
             }
-
+            else if (assemblyItem.IsFrameworkReference)
-            if (classifiedItem.AssemblyItem.IsSystemReference)
             {
                 // We do not allow transitive references to MVC via a framework reference to count.
                 // e.g. depending on Microsoft.AspNetCore.SomeThingNewThatDependsOnMvc would not result in an assembly being treated as
                 // referencing MVC.
-                classifiedItem.DependencyClassification = _mvcAssemblies.Contains(classifiedItem.Name) ?
+                classification = _mvcAssemblies.Contains(assemblyItem.AssemblyName) ?
-                    DependencyClassification.MvcReference :
+                    Classification.IsMvc :
-                    DependencyClassification.DoesNotReferenceMvc;
+                    Classification.DoesNotReferenceMvc;
-
-                return classifiedItem.DependencyClassification;
             }
-
+            else if (_mvcAssemblies.Contains(assemblyItem.AssemblyName))
-            if (_mvcAssemblies.Contains(classifiedItem.Name))
             {
-                classifiedItem.DependencyClassification = DependencyClassification.MvcReference;
+                classification = Classification.IsMvc;
-                return classifiedItem.DependencyClassification;
             }
-
+            else
-            var dependencyClassification = DependencyClassification.DoesNotReferenceMvc;
-            foreach (var assemblyItem in GetReferences(classifiedItem.AssemblyItem.Path))
             {
-                var classification = Resolve(assemblyItem);
+                classification = Classification.DoesNotReferenceMvc;
-                if (classification == DependencyClassification.MvcReference || classification == DependencyClassification.ReferencesMvc)
+                foreach (var reference in GetReferences(assemblyItem.Path))
                 {
-                    dependencyClassification = DependencyClassification.ReferencesMvc;
+                    var referenceClassification = Resolve(reference);
-                    break;
+
+                    if (referenceClassification == Classification.IsMvc || referenceClassification == Classification.ReferencesMvc)
+                    {
+                        classification = Classification.ReferencesMvc;
+                        break;
+                    }
                 }
             }
 
-            classifiedItem.DependencyClassification = dependencyClassification;
+            Debug.Assert(classification != Classification.Unknown);
-            return dependencyClassification;
+            _classifications[assemblyItem] = classification;
+            return classification;
         }
 
-        protected virtual IReadOnlyList<ClassifiedAssemblyItem> GetReferences(string file)
+        protected virtual IReadOnlyList<AssemblyItem> GetReferences(string file)
         {
             try
             {
                 using var peReader = new PEReader(File.OpenRead(file));
                 if (!peReader.HasMetadata)
                 {
-                    return Array.Empty<ClassifiedAssemblyItem>(); // not a managed assembly
+                    return Array.Empty<AssemblyItem>(); // not a managed assembly
                 }
 
                 var metadataReader = peReader.GetMetadataReader();
 
-                var assemblyItems = new List<ClassifiedAssemblyItem>();
+                var references = new List<AssemblyItem>();
                 foreach (var handle in metadataReader.AssemblyReferences)
                 {
                     var reference = metadataReader.GetAssemblyReference(handle);
                     var referenceName = metadataReader.GetString(reference.Name);
 
-                    if (_lookup.TryGetValue(referenceName, out var classifiedItem))
+                    if (!Lookup.TryGetValue(referenceName, out var assemblyItem))
-                    {
-                        assemblyItems.Add(classifiedItem);
-                    }
-                    else
                     {
                         // A dependency references an item that isn't referenced by this project.
                         // We'll construct an item for so that we can calculate the classification based on it's name.
-                        assemblyItems.Add(new ClassifiedAssemblyItem(referenceName));
+                        assemblyItem = new AssemblyItem
+                        {
+                            AssemblyName = referenceName,
+                        };
+
+                        Lookup[referenceName] = assemblyItem;
                     }
+
+                    references.Add(assemblyItem);
                 }
 
-                return assemblyItems;
+                return references;
             }
             catch (BadImageFormatException)
             {
                 // not a PE file, or invalid metadata
             }
 
-            return Array.Empty<ClassifiedAssemblyItem>(); // not a managed assembly
+            return Array.Empty<AssemblyItem>(); // not a managed assembly
         }
 
-        protected enum DependencyClassification
+        protected enum Classification
         {
             Unknown,
             DoesNotReferenceMvc,
             ReferencesMvc,
-            MvcReference,
+            IsMvc,
-        }
-
-        protected class ClassifiedAssemblyItem
-        {
-            public ClassifiedAssemblyItem(AssemblyItem classifiedItem)
-                : this(classifiedItem.AssemblyName)
-            {
-                AssemblyItem = classifiedItem;
-            }
-
-            public ClassifiedAssemblyItem(string name)
-            {
-                Name = name;
-            }
-
-            public string Name { get; }
-
-            public AssemblyItem AssemblyItem { get; }
-
-            public DependencyClassification DependencyClassification { get; set; }
         }
     }
 }

src/Razor/Microsoft.NET.Sdk.Razor/test/ReferenceResolverTest.cs

@@ -59,13 +59,13 @@ namespace Microsoft.AspNetCore.Razor.Tasks
             var resolver = new TestReferencesToMvcResolver(new[]
             {
                 CreateAssemblyItem("MyApp.Models"),
-                CreateAssemblyItem("Microsoft.AspNetCore.Mvc", isSystemReference: true),
+                CreateAssemblyItem("Microsoft.AspNetCore.Mvc", isFrameworkReference: true),
-                CreateAssemblyItem("Microsoft.AspNetCore.Hosting", isSystemReference: true),
+                CreateAssemblyItem("Microsoft.AspNetCore.Hosting", isFrameworkReference: true),
-                CreateAssemblyItem("Microsoft.AspNetCore.HttpAbstractions", isSystemReference: true),
+                CreateAssemblyItem("Microsoft.AspNetCore.HttpAbstractions", isFrameworkReference: true),
-                CreateAssemblyItem("Microsoft.AspNetCore.KestrelHttpServer", isSystemReference: true),
+                CreateAssemblyItem("Microsoft.AspNetCore.KestrelHttpServer", isFrameworkReference: true),
-                CreateAssemblyItem("Microsoft.AspNetCore.StaticFiles", isSystemReference: true),
+                CreateAssemblyItem("Microsoft.AspNetCore.StaticFiles", isFrameworkReference: true),
-                CreateAssemblyItem("Microsoft.Extensions.Primitives", isSystemReference: true),
+                CreateAssemblyItem("Microsoft.Extensions.Primitives", isFrameworkReference: true),
-                CreateAssemblyItem("System.Net.Http", isSystemReference: true),
+                CreateAssemblyItem("System.Net.Http", isFrameworkReference: true),
                 CreateAssemblyItem("Microsoft.EntityFrameworkCore"),
             });
 
@@ -89,16 +89,16 @@ namespace Microsoft.AspNetCore.Razor.Tasks
             // Arrange
             var resolver = new TestReferencesToMvcResolver(new[]
             {
-                CreateAssemblyItem("Microsoft.AspNetCore.Mvc", isSystemReference: true),
+                CreateAssemblyItem("Microsoft.AspNetCore.Mvc", isFrameworkReference: true),
-                CreateAssemblyItem("Microsoft.AspNetCore.Mvc.TagHelpers", isSystemReference: true),
+                CreateAssemblyItem("Microsoft.AspNetCore.Mvc.TagHelpers", isFrameworkReference: true),
                 CreateAssemblyItem("MyTagHelpers"),
                 CreateAssemblyItem("MyControllers"),
                 CreateAssemblyItem("MyApp.Models"),
-                CreateAssemblyItem("Microsoft.AspNetCore.Hosting", isSystemReference: true),
+                CreateAssemblyItem("Microsoft.AspNetCore.Hosting", isFrameworkReference: true),
-                CreateAssemblyItem("Microsoft.AspNetCore.HttpAbstractions", isSystemReference: true),
+                CreateAssemblyItem("Microsoft.AspNetCore.HttpAbstractions", isFrameworkReference: true),
-                CreateAssemblyItem("Microsoft.AspNetCore.KestrelHttpServer", isSystemReference: true),
+                CreateAssemblyItem("Microsoft.AspNetCore.KestrelHttpServer", isFrameworkReference: true),
-                CreateAssemblyItem("Microsoft.AspNetCore.StaticFiles", isSystemReference: true),
+                CreateAssemblyItem("Microsoft.AspNetCore.StaticFiles", isFrameworkReference: true),
-                CreateAssemblyItem("Microsoft.Extensions.Primitives", isSystemReference: true),
+                CreateAssemblyItem("Microsoft.Extensions.Primitives", isFrameworkReference: true),
                 CreateAssemblyItem("Microsoft.EntityFrameworkCore"),
             });
 
@@ -126,13 +126,12 @@ namespace Microsoft.AspNetCore.Razor.Tasks
             {
                 CreateAssemblyItem("MyCMS"),
                 CreateAssemblyItem("MyCMS.Core"),
-                CreateAssemblyItem("Microsoft.AspNetCore.Mvc.ViewFeatures", isSystemReference: true),
+                CreateAssemblyItem("Microsoft.AspNetCore.Mvc.ViewFeatures", isFrameworkReference: true),
             });
 
             resolver.Add("MyCMS", "MyCMS.Core");
             resolver.Add("MyCMS.Core", "Microsoft.AspNetCore.Mvc.ViewFeatures");
 
-
             // Act
             var assemblies = resolver.ResolveAssemblies();
 
@@ -140,41 +139,93 @@ namespace Microsoft.AspNetCore.Razor.Tasks
             Assert.Equal(new[] { "MyCMS", "MyCMS.Core" }, assemblies.OrderBy(a => a));
         }
 
-        public AssemblyItem CreateAssemblyItem(string name, bool isSystemReference = false)
+        [Fact]
+        public void Resolve_Works_WhenAssemblyReferencesAreRecursive()
+        {
+            // Test for https://github.com/aspnet/AspNetCore/issues/12693
+            // Arrange
+            var resolver = new TestReferencesToMvcResolver(new[]
+            {
+                CreateAssemblyItem("PresentationFramework"),
+                CreateAssemblyItem("ReachFramework"),
+                CreateAssemblyItem("MyCMS"),
+                CreateAssemblyItem("MyCMS.Core"),
+                CreateAssemblyItem("Microsoft.AspNetCore.Mvc.ViewFeatures", isFrameworkReference: true),
+            });
+
+            resolver.Add("PresentationFramework", "ReachFramework");
+            resolver.Add("ReachFramework", "PresentationFramework");
+
+            resolver.Add("MyCMS", "MyCMS.Core");
+            resolver.Add("MyCMS.Core", "Microsoft.AspNetCore.Mvc.ViewFeatures");
+
+            // Act
+            var assemblies = resolver.ResolveAssemblies();
+
+            // Assert
+            Assert.Equal(new[] { "MyCMS", "MyCMS.Core" }, assemblies.OrderBy(a => a));
+        }
+
+        [Fact]
+        public void Resolve_Works_WhenAssemblyReferencesAreRecursive_ButAlsoReferencesMvc()
+        {
+            // Arrange
+            var resolver = new TestReferencesToMvcResolver(new[]
+            {
+                CreateAssemblyItem("MyCoolLibrary"),
+                CreateAssemblyItem("PresentationFramework"),
+                CreateAssemblyItem("ReachFramework"),
+                CreateAssemblyItem("MyCMS"),
+                CreateAssemblyItem("MyCMS.Core"),
+                CreateAssemblyItem("Microsoft.AspNetCore.Mvc.ViewFeatures", isFrameworkReference: true),
+            });
+
+            resolver.Add("MyCoolLibrary", "PresentationFramework");
+            resolver.Add("PresentationFramework", "ReachFramework");
+            resolver.Add("ReachFramework", "PresentationFramework", "MyCMS");
+
+            resolver.Add("MyCMS", "MyCMS.Core");
+            resolver.Add("MyCMS.Core", "Microsoft.AspNetCore.Mvc.ViewFeatures");
+
+            // Act
+            var assemblies = resolver.ResolveAssemblies();
+
+            // Assert
+            Assert.Equal(new[] { "MyCMS", "MyCMS.Core", "MyCoolLibrary", "PresentationFramework", "ReachFramework" }, assemblies.OrderBy(a => a));
+        }
+
+        public AssemblyItem CreateAssemblyItem(string name, bool isFrameworkReference = false)
         {
             return new AssemblyItem
             {
                 AssemblyName = name,
-                IsSystemReference = isSystemReference,
+                IsFrameworkReference = isFrameworkReference,
                 Path = name,
             };
         }
 
         private class TestReferencesToMvcResolver : ReferenceResolver
         {
-            private readonly Dictionary<string, List<ClassifiedAssemblyItem>> _references = new Dictionary<string, List<ClassifiedAssemblyItem>>();
+            private readonly Dictionary<string, string[]> _references = new Dictionary<string, string[]>();
-            private readonly Dictionary<string, ClassifiedAssemblyItem> _lookup;
 
             public TestReferencesToMvcResolver(AssemblyItem[] referenceItems)
                 : base(MvcAssemblies, referenceItems)
             {
-                _lookup = referenceItems.ToDictionary(r => r.AssemblyName, r => new ClassifiedAssemblyItem(r));
             }
 
             public void Add(string assembly, params string[] references)
             {
-                var assemblyItems = references.Select(r => _lookup[r]).ToList();
+                _references.Add(assembly, references);
-                _references[assembly] = assemblyItems;
             }
 
-            protected override IReadOnlyList<ClassifiedAssemblyItem> GetReferences(string file)
+            protected override IReadOnlyList<AssemblyItem> GetReferences(string file)
             {
                 if (_references.TryGetValue(file, out var result))
                 {
-                    return result;
+                    return result.Select(r => Lookup[r]).ToArray();
                 }
 
-                return Array.Empty<ClassifiedAssemblyItem>();
+                return Array.Empty<AssemblyItem>();
             }
         }
     }