From 0cc94b3a460bec536b4f8927b936e340ac6176d4 Mon Sep 17 00:00:00 2001 From: Tamas Vajk Date: Thu, 30 Nov 2023 10:54:19 +0100 Subject: [PATCH 1/2] C#: Prefer framework assemblies over arbitrary nuget equivalents --- .../AssemblyCache.cs | 17 ++----- .../AssemblyCacheExtensions.cs | 29 +++++++++++ .../DependencyManager.cs | 49 +++++++++++++------ 3 files changed, 67 insertions(+), 28 deletions(-) create mode 100644 csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/AssemblyCacheExtensions.cs diff --git a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/AssemblyCache.cs b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/AssemblyCache.cs index 7adceebc7eb..3a124d13e0e 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/AssemblyCache.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/AssemblyCache.cs @@ -1,5 +1,4 @@ -using System; -using System.Collections.Generic; +using System.Collections.Generic; using System.IO; using System.Linq; @@ -20,7 +19,7 @@ namespace Semmle.Extraction.CSharp.DependencyFetching /// assembly cache. /// /// Callback for progress. - public AssemblyCache(IEnumerable paths, ProgressMonitor progressMonitor) + public AssemblyCache(IEnumerable paths, IEnumerable frameworkPaths, ProgressMonitor progressMonitor) { foreach (var path in paths) { @@ -40,7 +39,7 @@ namespace Semmle.Extraction.CSharp.DependencyFetching progressMonitor.LogInfo("AssemblyCache: Path not found: " + path); } } - IndexReferences(); + IndexReferences(frameworkPaths); } /// @@ -57,13 +56,11 @@ namespace Semmle.Extraction.CSharp.DependencyFetching } } - private static readonly Version emptyVersion = new Version(0, 0, 0, 0); - /// /// Indexes all DLLs we have located. /// Because this is a potentially time-consuming operation, it is put into a separate stage. /// - private void IndexReferences() + private void IndexReferences(IEnumerable frameworkPaths) { // Read all of the files foreach (var filename in pendingDllsToIndex) @@ -71,13 +68,9 @@ namespace Semmle.Extraction.CSharp.DependencyFetching IndexReference(filename); } - // Index "assemblyInfo" by version string - // The OrderBy is used to ensure that we by default select the highest version number. foreach (var info in assemblyInfoByFileName.Values .OrderBy(info => info.Name) - .ThenBy(info => info.NetCoreVersion ?? emptyVersion) - .ThenBy(info => info.Version ?? emptyVersion) - .ThenBy(info => info.Filename)) + .OrderAssemblyInfosByPreference(frameworkPaths)) { foreach (var index in info.IndexStrings) { diff --git a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/AssemblyCacheExtensions.cs b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/AssemblyCacheExtensions.cs new file mode 100644 index 00000000000..0626f14e9c5 --- /dev/null +++ b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/AssemblyCacheExtensions.cs @@ -0,0 +1,29 @@ +using System; +using System.Collections.Generic; +using System.Linq; + +namespace Semmle.Extraction.CSharp.DependencyFetching +{ + internal static class AssemblyCacheExtensions + { + private static readonly Version emptyVersion = new Version(0, 0, 0, 0); + + /// + /// This method orders AssemblyInfos by version numbers (.net core version first, then assembly version). Finally, it orders by filename to make the order deterministic. + /// + public static IOrderedEnumerable OrderAssemblyInfosByPreference(this IEnumerable assemblies, IEnumerable frameworkPaths) + { + // prefer framework assemblies over others + bool initialOrdering(AssemblyInfo info) => frameworkPaths.Any(framework => info.Filename.StartsWith(framework, StringComparison.OrdinalIgnoreCase)); + + var ordered = assemblies is IOrderedEnumerable o + ? o.ThenBy(initialOrdering) + : assemblies.OrderBy(initialOrdering); + + return ordered + .ThenBy(info => info.NetCoreVersion ?? emptyVersion) + .ThenBy(info => info.Version ?? emptyVersion) + .ThenBy(info => info.Filename); + } + } +} diff --git a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependencyManager.cs b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependencyManager.cs index 9bb78cc8d7c..fb73e2fcc4c 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependencyManager.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependencyManager.cs @@ -128,16 +128,27 @@ namespace Semmle.Extraction.CSharp.DependencyFetching DownloadMissingPackages(allNonBinaryFiles, dllPaths); } + var frameworkLocations = new List(); + // Find DLLs in the .Net / Asp.Net Framework // This block needs to come after the nuget restore, because the nuget restore might fetch the .NET Core/Framework reference assemblies. if (options.ScanNetFrameworkDlls) { - AddNetFrameworkDlls(dllPaths); - AddAspNetCoreFrameworkDlls(dllPaths); - AddMicrosoftWindowsDesktopDlls(dllPaths); + var path = AddNetFrameworkDlls(dllPaths); + frameworkLocations.Add(path); + path = AddAspNetCoreFrameworkDlls(dllPaths); + if (path != null) + { + frameworkLocations.Add(path); + } + path = AddMicrosoftWindowsDesktopDlls(dllPaths); + if (path != null) + { + frameworkLocations.Add(path); + } } - assemblyCache = new AssemblyCache(dllPaths, progressMonitor); + assemblyCache = new AssemblyCache(dllPaths, frameworkLocations, progressMonitor); AnalyseSolutions(solutions); foreach (var filename in assemblyCache.AllAssemblies.Select(a => a.Filename)) @@ -146,7 +157,7 @@ namespace Semmle.Extraction.CSharp.DependencyFetching } RemoveNugetAnalyzerReferences(); - ResolveConflicts(); + ResolveConflicts(frameworkLocations); // Output the findings foreach (var r in usedReferences.Keys.OrderBy(r => r)) @@ -228,7 +239,7 @@ namespace Semmle.Extraction.CSharp.DependencyFetching } } - private void AddNetFrameworkDlls(ISet dllPaths) + private string AddNetFrameworkDlls(ISet dllPaths) { // Multiple dotnet framework packages could be present. // The order of the packages is important, we're adding the first one that is present in the nuget cache. @@ -248,7 +259,7 @@ namespace Semmle.Extraction.CSharp.DependencyFetching RemoveNugetPackageReference(packagesInPrioOrder[i], dllPaths); } - return; + return frameworkPath.Path; } string? runtimeLocation = null; @@ -270,6 +281,7 @@ namespace Semmle.Extraction.CSharp.DependencyFetching progressMonitor.LogInfo($".NET runtime location selected: {runtimeLocation}"); dllPaths.Add(runtimeLocation); + return runtimeLocation; } private void RemoveNugetPackageReference(string packagePrefix, ISet dllPaths) @@ -294,11 +306,11 @@ namespace Semmle.Extraction.CSharp.DependencyFetching } } - private void AddAspNetCoreFrameworkDlls(ISet dllPaths) + private string? AddAspNetCoreFrameworkDlls(ISet dllPaths) { if (!fileContent.IsNewProjectStructureUsed || !fileContent.UseAspNetCoreDlls) { - return; + return null; } // First try to find ASP.NET Core assemblies in the NuGet packages @@ -306,21 +318,29 @@ namespace Semmle.Extraction.CSharp.DependencyFetching { progressMonitor.LogInfo($"Found ASP.NET Core in NuGet packages. Not adding installation directory."); dllPaths.Add(aspNetCorePackage); + return aspNetCorePackage; } - else if (Runtime.AspNetCoreRuntime is string aspNetCoreRuntime) + + if (Runtime.AspNetCoreRuntime is string aspNetCoreRuntime) { progressMonitor.LogInfo($"ASP.NET runtime location selected: {aspNetCoreRuntime}"); dllPaths.Add(aspNetCoreRuntime); + return aspNetCoreRuntime; } + + return null; } - private void AddMicrosoftWindowsDesktopDlls(ISet dllPaths) + private string? AddMicrosoftWindowsDesktopDlls(ISet dllPaths) { if (GetPackageDirectory(FrameworkPackageNames.WindowsDesktopFramework) is string windowsDesktopApp) { progressMonitor.LogInfo($"Found Windows Desktop App in NuGet packages."); dllPaths.Add(windowsDesktopApp); + return windowsDesktopApp; } + + return null; } private string? GetPackageDirectory(string packagePrefix) @@ -472,7 +492,7 @@ namespace Semmle.Extraction.CSharp.DependencyFetching /// If the same assembly name is duplicated with different versions, /// resolve to the higher version number. /// - private void ResolveConflicts() + private void ResolveConflicts(IEnumerable frameworkPaths) { var sortedReferences = new List(); foreach (var usedReference in usedReferences) @@ -488,11 +508,8 @@ namespace Semmle.Extraction.CSharp.DependencyFetching } } - var emptyVersion = new Version(0, 0); sortedReferences = sortedReferences - .OrderBy(r => r.NetCoreVersion ?? emptyVersion) - .ThenBy(r => r.Version ?? emptyVersion) - .ThenBy(r => r.Filename) + .OrderAssemblyInfosByPreference(frameworkPaths) .ToList(); var finalAssemblyList = new Dictionary(); From 31c1caf518e6e1431a6618957f07bbb9174d9b5c Mon Sep 17 00:00:00 2001 From: Tamas Vajk Date: Mon, 4 Dec 2023 10:16:13 +0100 Subject: [PATCH 2/2] Code quality improvements --- .../AssemblyCacheExtensions.cs | 2 +- .../DependencyManager.cs | 41 +++++++------------ 2 files changed, 16 insertions(+), 27 deletions(-) diff --git a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/AssemblyCacheExtensions.cs b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/AssemblyCacheExtensions.cs index 0626f14e9c5..a4e7723266b 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/AssemblyCacheExtensions.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/AssemblyCacheExtensions.cs @@ -14,7 +14,7 @@ namespace Semmle.Extraction.CSharp.DependencyFetching public static IOrderedEnumerable OrderAssemblyInfosByPreference(this IEnumerable assemblies, IEnumerable frameworkPaths) { // prefer framework assemblies over others - bool initialOrdering(AssemblyInfo info) => frameworkPaths.Any(framework => info.Filename.StartsWith(framework, StringComparison.OrdinalIgnoreCase)); + int initialOrdering(AssemblyInfo info) => frameworkPaths.Any(framework => info.Filename.StartsWith(framework, StringComparison.OrdinalIgnoreCase)) ? 1 : 0; var ordered = assemblies is IOrderedEnumerable o ? o.ThenBy(initialOrdering) diff --git a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependencyManager.cs b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependencyManager.cs index fb73e2fcc4c..04f526eb699 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependencyManager.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependencyManager.cs @@ -128,24 +128,15 @@ namespace Semmle.Extraction.CSharp.DependencyFetching DownloadMissingPackages(allNonBinaryFiles, dllPaths); } - var frameworkLocations = new List(); + var frameworkLocations = new HashSet(); // Find DLLs in the .Net / Asp.Net Framework // This block needs to come after the nuget restore, because the nuget restore might fetch the .NET Core/Framework reference assemblies. if (options.ScanNetFrameworkDlls) { - var path = AddNetFrameworkDlls(dllPaths); - frameworkLocations.Add(path); - path = AddAspNetCoreFrameworkDlls(dllPaths); - if (path != null) - { - frameworkLocations.Add(path); - } - path = AddMicrosoftWindowsDesktopDlls(dllPaths); - if (path != null) - { - frameworkLocations.Add(path); - } + AddNetFrameworkDlls(dllPaths, frameworkLocations); + AddAspNetCoreFrameworkDlls(dllPaths, frameworkLocations); + AddMicrosoftWindowsDesktopDlls(dllPaths, frameworkLocations); } assemblyCache = new AssemblyCache(dllPaths, frameworkLocations, progressMonitor); @@ -239,7 +230,7 @@ namespace Semmle.Extraction.CSharp.DependencyFetching } } - private string AddNetFrameworkDlls(ISet dllPaths) + private void AddNetFrameworkDlls(ISet dllPaths, ISet frameworkLocations) { // Multiple dotnet framework packages could be present. // The order of the packages is important, we're adding the first one that is present in the nuget cache. @@ -252,6 +243,7 @@ namespace Semmle.Extraction.CSharp.DependencyFetching if (frameworkPath.Path is not null) { dllPaths.Add(frameworkPath.Path); + frameworkLocations.Add(frameworkPath.Path); progressMonitor.LogInfo($"Found .NET Core/Framework DLLs in NuGet packages at {frameworkPath.Path}. Not adding installation directory."); for (var i = frameworkPath.Index + 1; i < packagesInPrioOrder.Length; i++) @@ -259,7 +251,7 @@ namespace Semmle.Extraction.CSharp.DependencyFetching RemoveNugetPackageReference(packagesInPrioOrder[i], dllPaths); } - return frameworkPath.Path; + return; } string? runtimeLocation = null; @@ -281,7 +273,7 @@ namespace Semmle.Extraction.CSharp.DependencyFetching progressMonitor.LogInfo($".NET runtime location selected: {runtimeLocation}"); dllPaths.Add(runtimeLocation); - return runtimeLocation; + frameworkLocations.Add(runtimeLocation); } private void RemoveNugetPackageReference(string packagePrefix, ISet dllPaths) @@ -306,11 +298,11 @@ namespace Semmle.Extraction.CSharp.DependencyFetching } } - private string? AddAspNetCoreFrameworkDlls(ISet dllPaths) + private void AddAspNetCoreFrameworkDlls(ISet dllPaths, ISet frameworkLocations) { if (!fileContent.IsNewProjectStructureUsed || !fileContent.UseAspNetCoreDlls) { - return null; + return; } // First try to find ASP.NET Core assemblies in the NuGet packages @@ -318,29 +310,26 @@ namespace Semmle.Extraction.CSharp.DependencyFetching { progressMonitor.LogInfo($"Found ASP.NET Core in NuGet packages. Not adding installation directory."); dllPaths.Add(aspNetCorePackage); - return aspNetCorePackage; + frameworkLocations.Add(aspNetCorePackage); + return; } if (Runtime.AspNetCoreRuntime is string aspNetCoreRuntime) { progressMonitor.LogInfo($"ASP.NET runtime location selected: {aspNetCoreRuntime}"); dllPaths.Add(aspNetCoreRuntime); - return aspNetCoreRuntime; + frameworkLocations.Add(aspNetCoreRuntime); } - - return null; } - private string? AddMicrosoftWindowsDesktopDlls(ISet dllPaths) + private void AddMicrosoftWindowsDesktopDlls(ISet dllPaths, ISet frameworkLocations) { if (GetPackageDirectory(FrameworkPackageNames.WindowsDesktopFramework) is string windowsDesktopApp) { progressMonitor.LogInfo($"Found Windows Desktop App in NuGet packages."); dllPaths.Add(windowsDesktopApp); - return windowsDesktopApp; + frameworkLocations.Add(windowsDesktopApp); } - - return null; } private string? GetPackageDirectory(string packagePrefix)