From 1e59def89da5e5e8f7126157f68314bb78492243 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Fri, 12 Apr 2024 09:31:31 +0200 Subject: [PATCH 1/7] C#: Add some suppress nullable warning testcases and update expected output. --- .../ConstructorInitializers.expected | 1 + .../expressions/PrintAst.expected | 23 +++++++++++++++++++ .../expressions/QualifiableExpr.expected | 1 + .../SuppressNullableWarning.expected | 1 + .../expressions/SuppressNullableWarning.ql | 3 +++ .../library-tests/expressions/expressions.cs | 12 ++++++++++ .../Security Features/CWE-089/SqlInjection.cs | 11 +++++++++ 7 files changed, 52 insertions(+) create mode 100644 csharp/ql/test/library-tests/expressions/SuppressNullableWarning.expected create mode 100644 csharp/ql/test/library-tests/expressions/SuppressNullableWarning.ql diff --git a/csharp/ql/test/library-tests/expressions/ConstructorInitializers.expected b/csharp/ql/test/library-tests/expressions/ConstructorInitializers.expected index 4e3c34780fe..00e46f6359c 100644 --- a/csharp/ql/test/library-tests/expressions/ConstructorInitializers.expected +++ b/csharp/ql/test/library-tests/expressions/ConstructorInitializers.expected @@ -22,6 +22,7 @@ | file://:0:0:0:0 | Rectangle | expressions.cs:351:18:351:26 | call to constructor Object | file://:0:0:0:0 | Object | | file://:0:0:0:0 | Rectangle2 | expressions.cs:361:18:361:27 | call to constructor Object | file://:0:0:0:0 | Object | | file://:0:0:0:0 | ReducedClass | ReducedExpression.cs:2:7:2:18 | call to constructor Object | file://:0:0:0:0 | Object | +| file://:0:0:0:0 | SuppressNullableWarning | expressions.cs:522:11:522:33 | call to constructor Object | file://:0:0:0:0 | Object | | file://:0:0:0:0 | TestConversionOperator | expressions.cs:330:11:330:32 | call to constructor Object | file://:0:0:0:0 | Object | | file://:0:0:0:0 | TestCreations | expressions.cs:383:18:383:30 | call to constructor Object | file://:0:0:0:0 | Object | | file://:0:0:0:0 | TestUnaryOperator | expressions.cs:292:11:292:27 | call to constructor Object | file://:0:0:0:0 | Object | diff --git a/csharp/ql/test/library-tests/expressions/PrintAst.expected b/csharp/ql/test/library-tests/expressions/PrintAst.expected index ce25c57b0d9..1f3f8f298a3 100644 --- a/csharp/ql/test/library-tests/expressions/PrintAst.expected +++ b/csharp/ql/test/library-tests/expressions/PrintAst.expected @@ -1,3 +1,5 @@ +expressions.cs: +# 530| [MethodCall] call to method Api FoldedLiterals.cs: # 1| [Class] FoldedLiterals # 3| 5: [Method] Test @@ -2406,3 +2408,24 @@ expressions.cs: # 520| -1: [TypeMention] object # 520| 3: [ConstructorInitializer] call to constructor ClassC1 # 520| 0: [ParameterAccess] access to parameter oc2 +# 522| 24: [Class] SuppressNullableWarning +# 525| 5: [Method] Api +# 525| -1: [TypeMention] object +# 525| 4: [ObjectCreation] object creation of type Object +# 525| 0: [TypeMention] object +# 527| 6: [Method] Test +# 527| -1: [TypeMention] Void +#-----| 2: (Parameters) +# 527| 0: [Parameter] arg0 +# 527| -1: [TypeMention] object +# 528| 4: [BlockStmt] {...} +# 529| 0: [LocalVariableDeclStmt] ... ...; +# 529| 0: [LocalVariableDeclAndInitExpr] Object x = ... +# 529| -1: [TypeMention] object +# 529| 0: [LocalVariableAccess] access to local variable x +# 529| 1: [SuppressNullableWarningExpr] ...! +# 529| 0: [ParameterAccess] access to parameter arg0 +# 530| 1: [LocalVariableDeclStmt] ... ...; +# 530| 0: [LocalVariableDeclAndInitExpr] Object y = ... +# 530| -1: [TypeMention] object +# 530| 0: [LocalVariableAccess] access to local variable y diff --git a/csharp/ql/test/library-tests/expressions/QualifiableExpr.expected b/csharp/ql/test/library-tests/expressions/QualifiableExpr.expected index c78c83911a9..c85d73bd7f7 100644 --- a/csharp/ql/test/library-tests/expressions/QualifiableExpr.expected +++ b/csharp/ql/test/library-tests/expressions/QualifiableExpr.expected @@ -70,3 +70,4 @@ | expressions.cs:483:17:483:26 | access to field value | expressions.cs:483:17:483:20 | this access | | expressions.cs:488:32:488:39 | access to field value | expressions.cs:488:32:488:33 | access to parameter c1 | | expressions.cs:488:43:488:50 | access to field value | expressions.cs:488:43:488:44 | access to parameter c2 | +| expressions.cs:530:21:530:25 | call to method Api | expressions.cs:530:21:530:25 | this access | diff --git a/csharp/ql/test/library-tests/expressions/SuppressNullableWarning.expected b/csharp/ql/test/library-tests/expressions/SuppressNullableWarning.expected new file mode 100644 index 00000000000..f05b27e2f57 --- /dev/null +++ b/csharp/ql/test/library-tests/expressions/SuppressNullableWarning.expected @@ -0,0 +1 @@ +| expressions.cs:529:21:529:25 | ...! | diff --git a/csharp/ql/test/library-tests/expressions/SuppressNullableWarning.ql b/csharp/ql/test/library-tests/expressions/SuppressNullableWarning.ql new file mode 100644 index 00000000000..9eb66b0062e --- /dev/null +++ b/csharp/ql/test/library-tests/expressions/SuppressNullableWarning.ql @@ -0,0 +1,3 @@ +import csharp + +select any(SuppressNullableWarningExpr e) diff --git a/csharp/ql/test/library-tests/expressions/expressions.cs b/csharp/ql/test/library-tests/expressions/expressions.cs index b4079d5e9a3..6c46eb8519e 100644 --- a/csharp/ql/test/library-tests/expressions/expressions.cs +++ b/csharp/ql/test/library-tests/expressions/expressions.cs @@ -518,4 +518,16 @@ namespace Expressions class ClassC1(object oc1) { } class ClassC2(object oc2) : ClassC1(oc2) { } + + class SuppressNullableWarning + { + + public object? Api() => new object(); + + public void Test(object? arg0) + { + var x = arg0!; + var y = Api()!; + } + } } diff --git a/csharp/ql/test/query-tests/Security Features/CWE-089/SqlInjection.cs b/csharp/ql/test/query-tests/Security Features/CWE-089/SqlInjection.cs index 587121ffa48..b698edfddce 100644 --- a/csharp/ql/test/query-tests/Security Features/CWE-089/SqlInjection.cs +++ b/csharp/ql/test/query-tests/Security Features/CWE-089/SqlInjection.cs @@ -95,6 +95,17 @@ namespace Test var result = new DataSet(); adapter.Fill(result); } + + // BAD: Input from the command line. (also implicitly check flow via suppress nullable warning `!`) + using (var connection = new SqlConnection(connectionString)) + { + var queryString = "SELECT ITEM,PRICE FROM PRODUCT WHERE ITEM_CATEGORY='" + + Console.ReadLine()! + "' ORDER BY PRICE"; + var cmd = new SqlCommand(queryString); + var adapter = new SqlDataAdapter(cmd); + var result = new DataSet(); + adapter.Fill(result); + } } System.Windows.Forms.TextBox box1; From 19b7574c9cc03cdbc415bb07da0b33002b93c522 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Thu, 11 Apr 2024 16:17:40 +0200 Subject: [PATCH 2/7] C#: Fix issue with suppress nullable warning directly on a method call. --- .../Semmle.Extraction.CSharp/Entities/CallTypeExtensions.cs | 2 +- .../Entities/Expressions/PostfixUnary.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/CallTypeExtensions.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/CallTypeExtensions.cs index 6b9564e49dc..03c4324a5a0 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/CallTypeExtensions.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/CallTypeExtensions.cs @@ -9,7 +9,7 @@ namespace Semmle.Extraction.CSharp.Entities /// public static ExprKind AdjustKind(this Expression.CallType ct, ExprKind k) { - if (k == ExprKind.ADDRESS_OF) + if (k == ExprKind.ADDRESS_OF || k == ExprKind.SUPPRESS_NULLABLE_WARNING) { return k; } diff --git a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/PostfixUnary.cs b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/PostfixUnary.cs index 2b7cf36e1af..dbe5ecb3d18 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/PostfixUnary.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp/Entities/Expressions/PostfixUnary.cs @@ -21,11 +21,11 @@ namespace Semmle.Extraction.CSharp.Entities.Expressions protected override void PopulateExpression(TextWriter trapFile) { Create(Context, operand, this, 0); - OperatorCall(trapFile, Syntax); if ((operatorKind == ExprKind.POST_INCR || operatorKind == ExprKind.POST_DECR) && Kind == ExprKind.OPERATOR_INVOCATION) { + OperatorCall(trapFile, Syntax); trapFile.mutator_invocation_mode(this, 2); } } From a2bb3dd78b91df6a2a28869dc941c318d83a26dc Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Fri, 12 Apr 2024 09:40:25 +0200 Subject: [PATCH 3/7] C#: Update expected test output. --- .../library-tests/expressions/PrintAst.expected | 4 ++-- .../expressions/SuppressNullableWarning.expected | 1 + .../CWE-089/SqlInjection.expected | 15 +++++++++++++++ 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/csharp/ql/test/library-tests/expressions/PrintAst.expected b/csharp/ql/test/library-tests/expressions/PrintAst.expected index 1f3f8f298a3..bee0a1e429c 100644 --- a/csharp/ql/test/library-tests/expressions/PrintAst.expected +++ b/csharp/ql/test/library-tests/expressions/PrintAst.expected @@ -1,5 +1,3 @@ -expressions.cs: -# 530| [MethodCall] call to method Api FoldedLiterals.cs: # 1| [Class] FoldedLiterals # 3| 5: [Method] Test @@ -2429,3 +2427,5 @@ expressions.cs: # 530| 0: [LocalVariableDeclAndInitExpr] Object y = ... # 530| -1: [TypeMention] object # 530| 0: [LocalVariableAccess] access to local variable y +# 530| 1: [SuppressNullableWarningExpr] ...! +# 530| 0: [MethodCall] call to method Api diff --git a/csharp/ql/test/library-tests/expressions/SuppressNullableWarning.expected b/csharp/ql/test/library-tests/expressions/SuppressNullableWarning.expected index f05b27e2f57..d687eb5ae1a 100644 --- a/csharp/ql/test/library-tests/expressions/SuppressNullableWarning.expected +++ b/csharp/ql/test/library-tests/expressions/SuppressNullableWarning.expected @@ -1 +1,2 @@ | expressions.cs:529:21:529:25 | ...! | +| expressions.cs:530:21:530:26 | ...! | diff --git a/csharp/ql/test/query-tests/Security Features/CWE-089/SqlInjection.expected b/csharp/ql/test/query-tests/Security Features/CWE-089/SqlInjection.expected index 0474f0d5930..19abb425754 100644 --- a/csharp/ql/test/query-tests/Security Features/CWE-089/SqlInjection.expected +++ b/csharp/ql/test/query-tests/Security Features/CWE-089/SqlInjection.expected @@ -27,6 +27,12 @@ edges | SqlInjection.cs:93:21:93:23 | access to local variable cmd : SqlCommand | SqlInjection.cs:94:50:94:52 | access to local variable cmd | provenance | Sink:MaD:950 | | SqlInjection.cs:93:27:93:53 | object creation of type SqlCommand : SqlCommand | SqlInjection.cs:93:21:93:23 | access to local variable cmd : SqlCommand | provenance | | | SqlInjection.cs:93:42:93:52 | access to local variable queryString : String | SqlInjection.cs:93:27:93:53 | object creation of type SqlCommand : SqlCommand | provenance | MaD:953 | +| SqlInjection.cs:102:21:102:31 | access to local variable queryString : String | SqlInjection.cs:104:42:104:52 | access to local variable queryString | provenance | Sink:MaD:947 | +| SqlInjection.cs:102:21:102:31 | access to local variable queryString : String | SqlInjection.cs:104:42:104:52 | access to local variable queryString : String | provenance | | +| SqlInjection.cs:103:21:103:38 | call to method ReadLine : String | SqlInjection.cs:102:21:102:31 | access to local variable queryString : String | provenance | Src:MaD:2250 | +| SqlInjection.cs:104:21:104:23 | access to local variable cmd : SqlCommand | SqlInjection.cs:105:50:105:52 | access to local variable cmd | provenance | Sink:MaD:950 | +| SqlInjection.cs:104:27:104:53 | object creation of type SqlCommand : SqlCommand | SqlInjection.cs:104:21:104:23 | access to local variable cmd : SqlCommand | provenance | | +| SqlInjection.cs:104:42:104:52 | access to local variable queryString : String | SqlInjection.cs:104:27:104:53 | object creation of type SqlCommand : SqlCommand | provenance | MaD:953 | | SqlInjectionDapper.cs:20:21:20:25 | access to local variable query : String | SqlInjectionDapper.cs:21:55:21:59 | access to local variable query | provenance | Sink:MaD:27 | | SqlInjectionDapper.cs:20:86:20:94 | access to property Text : String | SqlInjectionDapper.cs:20:21:20:25 | access to local variable query : String | provenance | | | SqlInjectionDapper.cs:29:21:29:25 | access to local variable query : String | SqlInjectionDapper.cs:30:66:30:70 | access to local variable query | provenance | Sink:MaD:37 | @@ -97,6 +103,13 @@ nodes | SqlInjection.cs:93:42:93:52 | access to local variable queryString | semmle.label | access to local variable queryString | | SqlInjection.cs:93:42:93:52 | access to local variable queryString : String | semmle.label | access to local variable queryString : String | | SqlInjection.cs:94:50:94:52 | access to local variable cmd | semmle.label | access to local variable cmd | +| SqlInjection.cs:102:21:102:31 | access to local variable queryString : String | semmle.label | access to local variable queryString : String | +| SqlInjection.cs:103:21:103:38 | call to method ReadLine : String | semmle.label | call to method ReadLine : String | +| SqlInjection.cs:104:21:104:23 | access to local variable cmd : SqlCommand | semmle.label | access to local variable cmd : SqlCommand | +| SqlInjection.cs:104:27:104:53 | object creation of type SqlCommand : SqlCommand | semmle.label | object creation of type SqlCommand : SqlCommand | +| SqlInjection.cs:104:42:104:52 | access to local variable queryString | semmle.label | access to local variable queryString | +| SqlInjection.cs:104:42:104:52 | access to local variable queryString : String | semmle.label | access to local variable queryString : String | +| SqlInjection.cs:105:50:105:52 | access to local variable cmd | semmle.label | access to local variable cmd | | SqlInjectionDapper.cs:20:21:20:25 | access to local variable query : String | semmle.label | access to local variable query : String | | SqlInjectionDapper.cs:20:86:20:94 | access to property Text : String | semmle.label | access to property Text : String | | SqlInjectionDapper.cs:21:55:21:59 | access to local variable query | semmle.label | access to local variable query | @@ -154,6 +167,8 @@ subpaths | SqlInjection.cs:83:50:83:55 | access to local variable query1 | SqlInjection.cs:82:21:82:29 | access to property Text : String | SqlInjection.cs:83:50:83:55 | access to local variable query1 | This query depends on $@. | SqlInjection.cs:82:21:82:29 | access to property Text : String | this TextBox text | | SqlInjection.cs:93:42:93:52 | access to local variable queryString | SqlInjection.cs:92:21:92:29 | access to property Text : String | SqlInjection.cs:93:42:93:52 | access to local variable queryString | This query depends on $@. | SqlInjection.cs:92:21:92:29 | access to property Text : String | this TextBox text | | SqlInjection.cs:94:50:94:52 | access to local variable cmd | SqlInjection.cs:92:21:92:29 | access to property Text : String | SqlInjection.cs:94:50:94:52 | access to local variable cmd | This query depends on $@. | SqlInjection.cs:92:21:92:29 | access to property Text : String | this TextBox text | +| SqlInjection.cs:104:42:104:52 | access to local variable queryString | SqlInjection.cs:103:21:103:38 | call to method ReadLine : String | SqlInjection.cs:104:42:104:52 | access to local variable queryString | This query depends on $@. | SqlInjection.cs:103:21:103:38 | call to method ReadLine : String | this external | +| SqlInjection.cs:105:50:105:52 | access to local variable cmd | SqlInjection.cs:103:21:103:38 | call to method ReadLine : String | SqlInjection.cs:105:50:105:52 | access to local variable cmd | This query depends on $@. | SqlInjection.cs:103:21:103:38 | call to method ReadLine : String | this external | | SqlInjectionDapper.cs:21:55:21:59 | access to local variable query | SqlInjectionDapper.cs:20:86:20:94 | access to property Text : String | SqlInjectionDapper.cs:21:55:21:59 | access to local variable query | This query depends on $@. | SqlInjectionDapper.cs:20:86:20:94 | access to property Text : String | this TextBox text | | SqlInjectionDapper.cs:30:66:30:70 | access to local variable query | SqlInjectionDapper.cs:29:86:29:94 | access to property Text : String | SqlInjectionDapper.cs:30:66:30:70 | access to local variable query | This query depends on $@. | SqlInjectionDapper.cs:29:86:29:94 | access to property Text : String | this TextBox text | | SqlInjectionDapper.cs:39:63:39:67 | access to local variable query | SqlInjectionDapper.cs:38:86:38:94 | access to property Text : String | SqlInjectionDapper.cs:39:63:39:67 | access to local variable query | This query depends on $@. | SqlInjectionDapper.cs:38:86:38:94 | access to property Text : String | this TextBox text | From cbb5d433b14ff5566d144478701c94fdabc30298 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Fri, 12 Apr 2024 09:51:48 +0200 Subject: [PATCH 4/7] C#: Add change note. --- .../lib/change-notes/2024-04-12-suppress-nullable-warning.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 csharp/ql/lib/change-notes/2024-04-12-suppress-nullable-warning.md diff --git a/csharp/ql/lib/change-notes/2024-04-12-suppress-nullable-warning.md b/csharp/ql/lib/change-notes/2024-04-12-suppress-nullable-warning.md new file mode 100644 index 00000000000..241a67dddaf --- /dev/null +++ b/csharp/ql/lib/change-notes/2024-04-12-suppress-nullable-warning.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* Extracting suppress nullable warning expressions did not work when applied directly to a method call (like `System.Console.Readline()!`). This has been fixed. From 5406fac8341c80b88b0cc2033317c138a840ddd9 Mon Sep 17 00:00:00 2001 From: Tamas Vajk Date: Fri, 12 Apr 2024 13:58:37 +0200 Subject: [PATCH 5/7] C#: Move all file lookup to separate class --- .../DependencyManager.Nuget.cs | 48 +++---- .../DependencyManager.cs | 76 +++-------- .../FileInfoExtensions.cs | 14 -- .../FileProvider.cs | 120 ++++++++++++++++++ 4 files changed, 159 insertions(+), 99 deletions(-) create mode 100644 csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/FileProvider.cs diff --git a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependencyManager.Nuget.cs b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependencyManager.Nuget.cs index 298f462563b..e39d59690b5 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependencyManager.Nuget.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependencyManager.Nuget.cs @@ -13,15 +13,15 @@ namespace Semmle.Extraction.CSharp.DependencyFetching { public sealed partial class DependencyManager { - private void RestoreNugetPackages(List allNonBinaryFiles, IEnumerable allProjects, IEnumerable allSolutions, HashSet dllLocations) + private void RestoreNugetPackages(HashSet dllLocations) { var checkNugetFeedResponsiveness = EnvironmentVariables.GetBoolean(EnvironmentVariableNames.CheckNugetFeedResponsiveness); try { - if (checkNugetFeedResponsiveness && !CheckFeeds(allNonBinaryFiles)) + if (checkNugetFeedResponsiveness && !CheckFeeds()) { // todo: we could also check the reachability of the inherited nuget feeds, but to use those in the fallback we would need to handle authentication too. - DownloadMissingPackagesFromSpecificFeeds(allNonBinaryFiles, dllLocations); + DownloadMissingPackagesFromSpecificFeeds(dllLocations); return; } @@ -64,8 +64,8 @@ namespace Semmle.Extraction.CSharp.DependencyFetching logger.LogError($"Failed to restore Nuget packages with nuget.exe: {exc.Message}"); } - var restoredProjects = RestoreSolutions(allSolutions, out var assets1); - var projects = allProjects.Except(restoredProjects); + var restoredProjects = RestoreSolutions(out var assets1); + var projects = fileProvider.Projects.Except(restoredProjects); RestoreProjects(projects, out var assets2); var dependencies = Assets.GetCompilationDependencies(logger, assets1.Union(assets2)); @@ -80,11 +80,11 @@ namespace Semmle.Extraction.CSharp.DependencyFetching if (checkNugetFeedResponsiveness) { - DownloadMissingPackagesFromSpecificFeeds(allNonBinaryFiles, dllLocations); + DownloadMissingPackagesFromSpecificFeeds(dllLocations); } else { - DownloadMissingPackages(allNonBinaryFiles, dllLocations); + DownloadMissingPackages(dllLocations); } } @@ -122,13 +122,12 @@ namespace Semmle.Extraction.CSharp.DependencyFetching /// Populates assets with the relative paths to the assets files generated by the restore. /// Returns a list of projects that are up to date with respect to restore. /// - /// A list of paths to solution files. - private IEnumerable RestoreSolutions(IEnumerable solutions, out IEnumerable assets) + private IEnumerable RestoreSolutions(out IEnumerable assets) { var successCount = 0; var nugetSourceFailures = 0; var assetFiles = new List(); - var projects = solutions.SelectMany(solution => + var projects = fileProvider.Solutions.SelectMany(solution => { logger.LogInfo($"Restoring solution {solution}..."); var res = dotnet.Restore(new(solution, packageDirectory.DirInfo.FullName, ForceDotnetRefAssemblyFetching: true)); @@ -184,12 +183,12 @@ namespace Semmle.Extraction.CSharp.DependencyFetching CompilationInfos.Add(("Failed project restore with package source error", nugetSourceFailures.ToString())); } - private void DownloadMissingPackagesFromSpecificFeeds(List allNonBinaryFiles, HashSet dllLocations) + private void DownloadMissingPackagesFromSpecificFeeds(HashSet dllLocations) { var reachableFallbackFeeds = GetReachableFallbackNugetFeeds(); if (reachableFallbackFeeds.Count > 0) { - DownloadMissingPackages(allNonBinaryFiles, dllLocations, fallbackNugetFeeds: reachableFallbackFeeds); + DownloadMissingPackages(dllLocations, fallbackNugetFeeds: reachableFallbackFeeds); } else { @@ -197,7 +196,7 @@ namespace Semmle.Extraction.CSharp.DependencyFetching } } - private void DownloadMissingPackages(List allFiles, HashSet dllLocations, IEnumerable? fallbackNugetFeeds = null) + private void DownloadMissingPackages(HashSet dllLocations, IEnumerable? fallbackNugetFeeds = null) { var alreadyDownloadedPackages = GetRestoredPackageDirectoryNames(packageDirectory.DirInfo); var alreadyDownloadedLegacyPackages = GetRestoredLegacyPackageNames(); @@ -232,7 +231,7 @@ namespace Semmle.Extraction.CSharp.DependencyFetching logger.LogInfo($"Found {notYetDownloadedPackages.Count} packages that are not yet restored"); using var tempDir = new TemporaryDirectory(ComputeTempDirectory(sourceDir.FullName, "nugetconfig")); var nugetConfig = fallbackNugetFeeds is null - ? GetNugetConfig(allFiles) + ? GetNugetConfig() : CreateFallbackNugetConfig(fallbackNugetFeeds, tempDir.DirInfo.FullName); CompilationInfos.Add(("Fallback nuget restore", notYetDownloadedPackages.Count.ToString())); @@ -280,19 +279,14 @@ namespace Semmle.Extraction.CSharp.DependencyFetching return nugetConfigPath; } - private string[] GetAllNugetConfigs(List allFiles) => allFiles.SelectFileNamesByName("nuget.config").ToArray(); - - private string? GetNugetConfig(List allFiles) + private string? GetNugetConfig() { - var nugetConfigs = GetAllNugetConfigs(allFiles); + var nugetConfigs = fileProvider.NugetConfigs; string? nugetConfig; - if (nugetConfigs.Length > 1) + if (nugetConfigs.Count > 1) { logger.LogInfo($"Found multiple nuget.config files: {string.Join(", ", nugetConfigs)}."); - nugetConfig = allFiles - .SelectRootFiles(sourceDir) - .SelectFileNamesByName("nuget.config") - .FirstOrDefault(); + nugetConfig = fileProvider.RootNugetConfig; if (nugetConfig == null) { logger.LogInfo("Could not find a top-level nuget.config file."); @@ -512,10 +506,10 @@ namespace Semmle.Extraction.CSharp.DependencyFetching return (timeoutMilliSeconds, tryCount); } - private bool CheckFeeds(List allFiles) + private bool CheckFeeds() { logger.LogInfo("Checking Nuget feeds..."); - var (explicitFeeds, allFeeds) = GetAllFeeds(allFiles); + var (explicitFeeds, allFeeds) = GetAllFeeds(); var excludedFeeds = EnvironmentVariables.GetURLs(EnvironmentVariableNames.ExcludedNugetFeedsFromResponsivenessCheck) .ToHashSet() ?? []; @@ -581,13 +575,13 @@ namespace Semmle.Extraction.CSharp.DependencyFetching } } - private (HashSet explicitFeeds, HashSet allFeeds) GetAllFeeds(List allFiles) + private (HashSet explicitFeeds, HashSet allFeeds) GetAllFeeds() { IList GetNugetFeeds(string nugetConfig) => dotnet.GetNugetFeeds(nugetConfig); IList GetNugetFeedsFromFolder(string folderPath) => dotnet.GetNugetFeedsFromFolder(folderPath); - var nugetConfigs = GetAllNugetConfigs(allFiles); + var nugetConfigs = fileProvider.NugetConfigs; var explicitFeeds = nugetConfigs .SelectMany(config => GetFeeds(() => GetNugetFeeds(config))) .ToHashSet(); diff --git a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependencyManager.cs b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependencyManager.cs index 64ac19da204..26f5a33ea67 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependencyManager.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependencyManager.cs @@ -36,6 +36,7 @@ namespace Semmle.Extraction.CSharp.DependencyFetching private readonly TemporaryDirectory legacyPackageDirectory; private readonly TemporaryDirectory missingPackageDirectory; private readonly TemporaryDirectory tempWorkingDirectory; + private readonly FileProvider fileProvider; private readonly bool cleanupTempWorkingDirectory; private readonly Lazy runtimeLazy; @@ -79,20 +80,10 @@ namespace Semmle.Extraction.CSharp.DependencyFetching tempWorkingDirectory = new TemporaryDirectory(FileUtils.GetTemporaryWorkingDirectory(out cleanupTempWorkingDirectory)); - logger.LogInfo($"Finding files in {srcDir}..."); - - var allFiles = GetAllFiles().ToList(); - var binaryFileExtensions = new HashSet(new[] { ".dll", ".exe" }); // TODO: add more binary file extensions. - var allNonBinaryFiles = allFiles.Where(f => !binaryFileExtensions.Contains(f.Extension.ToLowerInvariant())).ToList(); - var smallNonBinaryFiles = allNonBinaryFiles.SelectSmallFiles(logger).SelectFileNames().ToList(); - this.fileContent = new FileContent(logger, smallNonBinaryFiles); - this.nonGeneratedSources = allNonBinaryFiles.SelectFileNamesByExtension(".cs").ToList(); - this.generatedSources = new(); - var allProjects = allNonBinaryFiles.SelectFileNamesByExtension(".csproj").ToList(); - var allSolutions = allNonBinaryFiles.SelectFileNamesByExtension(".sln").ToList(); - var dllLocations = allFiles.SelectFileNamesByExtension(".dll").Select(x => new AssemblyLookupLocation(x)).ToHashSet(); - - logger.LogInfo($"Found {allFiles.Count} files, {nonGeneratedSources.Count} source files, {allProjects.Count} project files, {allSolutions.Count} solution files, {dllLocations.Count} DLLs."); + this.fileProvider = new FileProvider(sourceDir, logger); + this.fileContent = new FileContent(logger, this.fileProvider.SmallNonBinary); + this.nonGeneratedSources = fileProvider.Sources.ToList(); + this.generatedSources = []; void startCallback(string s, bool silent) { @@ -104,7 +95,7 @@ namespace Semmle.Extraction.CSharp.DependencyFetching logger.Log(silent ? Severity.Debug : Severity.Info, $"Exit code {ret}{(string.IsNullOrEmpty(msg) ? "" : $": {msg}")}"); } - DotNet.WithDotNet(SystemBuildActions.Instance, logger, smallNonBinaryFiles, tempWorkingDirectory.ToString(), shouldCleanUp: false, ensureDotNetAvailable: true, version: null, installDir => + DotNet.WithDotNet(SystemBuildActions.Instance, logger, fileProvider.GlobalJsons, tempWorkingDirectory.ToString(), shouldCleanUp: false, ensureDotNetAvailable: true, version: null, installDir => { this.dotnetPath = installDir; return BuildScript.Success; @@ -121,13 +112,14 @@ namespace Semmle.Extraction.CSharp.DependencyFetching throw; } - RestoreNugetPackages(allNonBinaryFiles, allProjects, allSolutions, dllLocations); + var dllLocations = fileProvider.Dlls.Select(x => new AssemblyLookupLocation(x)).ToHashSet(); + RestoreNugetPackages(dllLocations); // Find DLLs in the .Net / Asp.Net Framework // This needs to come after the nuget restore, because the nuget restore might fetch the .NET Core/Framework reference assemblies. var frameworkLocations = AddFrameworkDlls(dllLocations); assemblyCache = new AssemblyCache(dllLocations, frameworkLocations, logger); - AnalyseSolutions(allSolutions); + AnalyseSolutions(fileProvider.Solutions); foreach (var filename in assemblyCache.AllAssemblies.Select(a => a.Filename)) { @@ -154,7 +146,7 @@ namespace Semmle.Extraction.CSharp.DependencyFetching shouldExtractWebViews) { CompilationInfos.Add(("WebView extraction enabled", "1")); - GenerateSourceFilesFromWebViews(allNonBinaryFiles); + GenerateSourceFilesFromWebViews(); } else { @@ -171,8 +163,8 @@ namespace Semmle.Extraction.CSharp.DependencyFetching logger.LogInfo("Build analysis summary:"); logger.LogInfo($"{nonGeneratedSources.Count,align} source files found on the filesystem"); logger.LogInfo($"{generatedSources.Count,align} source files have been generated"); - logger.LogInfo($"{allSolutions.Count,align} solution files found on the filesystem"); - logger.LogInfo($"{allProjects.Count,align} project files found on the filesystem"); + logger.LogInfo($"{fileProvider.Solutions.Count,align} solution files found on the filesystem"); + logger.LogInfo($"{fileProvider.Projects.Count,align} project files found on the filesystem"); logger.LogInfo($"{usedReferences.Keys.Count,align} resolved references"); logger.LogInfo($"{unresolvedReferences.Count,align} unresolved references"); logger.LogInfo($"{conflictedReferences,align} resolved assembly conflicts"); @@ -182,8 +174,8 @@ namespace Semmle.Extraction.CSharp.DependencyFetching CompilationInfos.AddRange([ ("Source files on filesystem", nonGeneratedSources.Count.ToString()), ("Source files generated", generatedSources.Count.ToString()), - ("Solution files on filesystem", allSolutions.Count.ToString()), - ("Project files on filesystem", allProjects.Count.ToString()), + ("Solution files on filesystem", fileProvider.Solutions.Count.ToString()), + ("Project files on filesystem", fileProvider.Projects.Count.ToString()), ("Resolved references", usedReferences.Keys.Count.ToString()), ("Unresolved references", unresolvedReferences.Count.ToString()), ("Resolved assembly conflicts", conflictedReferences.ToString()), @@ -467,15 +459,15 @@ namespace Semmle.Extraction.CSharp.DependencyFetching } } - private void GenerateSourceFilesFromWebViews(List allFiles) + private void GenerateSourceFilesFromWebViews() { - var views = allFiles.SelectFileNamesByExtension(".cshtml", ".razor").ToArray(); - if (views.Length == 0) + var views = fileProvider.RazorViews; + if (views.Count == 0) { return; } - logger.LogInfo($"Found {views.Length} cshtml and razor files."); + logger.LogInfo($"Found {views.Count} cshtml and razor files."); if (!IsAspNetCoreDetected()) { @@ -503,38 +495,6 @@ namespace Semmle.Extraction.CSharp.DependencyFetching } } - private IEnumerable GetAllFiles() - { - IEnumerable files = sourceDir.GetFiles("*.*", new EnumerationOptions { RecurseSubdirectories = true }); - - if (dotnetPath != null) - { - files = files.Where(f => !f.FullName.StartsWith(dotnetPath, StringComparison.OrdinalIgnoreCase)); - } - - files = files.Where(f => - { - try - { - if (f.Exists) - { - return true; - } - - logger.LogWarning($"File {f.FullName} could not be processed."); - return false; - } - catch (Exception ex) - { - logger.LogWarning($"File {f.FullName} could not be processed: {ex.Message}"); - return false; - } - }); - - files = new FilePathFilter(sourceDir, logger).Filter(files); - return files; - } - /// /// Computes a unique temp directory for the packages associated /// with this source tree. Use a SHA1 of the directory name. diff --git a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/FileInfoExtensions.cs b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/FileInfoExtensions.cs index e68ad8c0616..a562c685184 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/FileInfoExtensions.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/FileInfoExtensions.cs @@ -14,20 +14,6 @@ namespace Semmle.Extraction.CSharp.DependencyFetching public static IEnumerable SelectRootFiles(this IEnumerable files, DirectoryInfo dir) => files.Where(file => file.DirectoryName == dir.FullName); - internal static IEnumerable SelectSmallFiles(this IEnumerable files, ILogger logger) - { - const int oneMb = 1_048_576; - return files.Where(file => - { - if (file.Length > oneMb) - { - logger.LogDebug($"Skipping {file.FullName} because it is bigger than 1MB."); - return false; - } - return true; - }); - } - public static IEnumerable SelectFileNamesByExtension(this IEnumerable files, params string[] extensions) => files.SelectFilesAux(fi => extensions.Contains(fi.Extension)); diff --git a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/FileProvider.cs b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/FileProvider.cs new file mode 100644 index 00000000000..320b7d70435 --- /dev/null +++ b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/FileProvider.cs @@ -0,0 +1,120 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Security.Policy; +using Semmle.Util.Logging; + +namespace Semmle.Extraction.CSharp.DependencyFetching +{ + public class FileProvider + { + private static readonly HashSet binaryFileExtensions = [".dll", ".exe"]; // TODO: add more binary file extensions. + + private readonly ILogger logger; + private readonly Lazy all; + private readonly Lazy allNonBinary; + private readonly Lazy smallNonBinary; + private readonly Lazy sources; + private readonly Lazy projects; + private readonly Lazy solutions; + private readonly Lazy dlls; + private readonly Lazy nugetConfigs; + private readonly Lazy globalJsons; + private readonly Lazy razorViews; + private readonly Lazy rootNugetConfig; + + public FileProvider(DirectoryInfo sourceDir, ILogger logger) + { + SourceDir = sourceDir; + this.logger = logger; + + all = new Lazy(GetAllFiles); + allNonBinary = new Lazy(() => all.Value.Where(f => !binaryFileExtensions.Contains(f.Extension.ToLowerInvariant())).ToArray()); + smallNonBinary = new Lazy(() => + { + var ret = SelectSmallFiles(allNonBinary.Value).SelectFileNames().ToArray(); + logger.LogInfo($"Found {ret.Length} small non-binary files in {SourceDir}."); + return ret; + }); + sources = new Lazy(() => SelectTextFileNamesByExtension("source", ".cs")); + projects = new Lazy(() => SelectTextFileNamesByExtension("project", ".csproj")); + solutions = new Lazy(() => SelectTextFileNamesByExtension("solution", ".sln")); + dlls = new Lazy(() => SelectBinaryFileNamesByExtension("DLL", ".dll")); + nugetConfigs = new Lazy(() => allNonBinary.Value.SelectFileNamesByName("nuget.config").ToArray()); + globalJsons = new Lazy(() => allNonBinary.Value.SelectFileNamesByName("global.json").ToArray()); + razorViews = new Lazy(() => SelectTextFileNamesByExtension("razor view", ".cshtml", ".razor")); + + rootNugetConfig = new Lazy(() => all.Value.SelectRootFiles(SourceDir).SelectFileNamesByName("nuget.config").FirstOrDefault()); + } + + private string[] SelectTextFileNamesByExtension(string filetype, params string[] extensions) + { + var ret = allNonBinary.Value.SelectFileNamesByExtension(extensions).ToArray(); + logger.LogInfo($"Found {ret.Length} {filetype} files in {SourceDir}."); + return ret; + } + + private string[] SelectBinaryFileNamesByExtension(string filetype, params string[] extensions) + { + var ret = all.Value.SelectFileNamesByExtension(extensions).ToArray(); + logger.LogInfo($"Found {ret.Length} {filetype} files in {SourceDir}."); + return ret; + } + + private IEnumerable SelectSmallFiles(IEnumerable files) + { + const int oneMb = 1_048_576; + return files.Where(file => + { + if (file.Length > oneMb) + { + logger.LogDebug($"Skipping {file.FullName} because it is bigger than 1MB."); + return false; + } + return true; + }); + } + + private FileInfo[] GetAllFiles() + { + logger.LogInfo($"Finding files in {SourceDir}..."); + var files = SourceDir.GetFiles("*.*", new EnumerationOptions { RecurseSubdirectories = true }); + + var filteredFiles = files.Where(f => + { + try + { + if (f.Exists) + { + return true; + } + + logger.LogWarning($"File {f.FullName} could not be processed."); + return false; + } + catch (Exception ex) + { + logger.LogWarning($"File {f.FullName} could not be processed: {ex.Message}"); + return false; + } + }); + + var allFiles = new FilePathFilter(SourceDir, logger).Filter(filteredFiles).ToArray(); + + logger.LogInfo($"Found {allFiles.Length} files in {SourceDir}."); + return allFiles; + } + + public DirectoryInfo SourceDir { get; } + public IEnumerable SmallNonBinary => smallNonBinary.Value; + public IEnumerable Sources => sources.Value; + public ICollection Projects => projects.Value; + public ICollection Solutions => solutions.Value; + public IEnumerable Dlls => dlls.Value; + public ICollection NugetConfigs => nugetConfigs.Value; + public string? RootNugetConfig => rootNugetConfig.Value; + public IEnumerable GlobalJsons => globalJsons.Value; + public ICollection RazorViews => razorViews.Value; + } +} From e3fe9f7ca586f448baefc7ef71ed27005354a99c Mon Sep 17 00:00:00 2001 From: Tamas Vajk Date: Fri, 12 Apr 2024 15:04:42 +0200 Subject: [PATCH 6/7] Move Nuget restore logic from `DependencyManager` to dedicated class --- .../DependencyManager.cs | 92 ++++------ .../FileProvider.cs | 10 +- .../{NugetPackages.cs => NugetExeWrapper.cs} | 8 +- ...nager.Nuget.cs => NugetPackageRestorer.cs} | 166 ++++++++++++------ 4 files changed, 162 insertions(+), 114 deletions(-) rename csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/{NugetPackages.cs => NugetExeWrapper.cs} (96%) rename csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/{DependencyManager.Nuget.cs => NugetPackageRestorer.cs} (78%) diff --git a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependencyManager.cs b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependencyManager.cs index 26f5a33ea67..357bc0026cc 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependencyManager.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependencyManager.cs @@ -12,14 +12,26 @@ using Semmle.Util.Logging; namespace Semmle.Extraction.CSharp.DependencyFetching { + public interface ICompilationInfoContainer + { + /// + /// List of `(key, value)` tuples, that are stored in the DB for telemetry purposes. + /// + List<(string, string)> CompilationInfos { get; } + } + /// /// Main implementation of the build analysis. /// - public sealed partial class DependencyManager : IDisposable + public sealed partial class DependencyManager : IDisposable, ICompilationInfoContainer { private readonly AssemblyCache assemblyCache; private readonly ILogger logger; private readonly IDiagnosticsWriter diagnosticsWriter; + private readonly NugetPackageRestorer nugetPackageRestorer; + private readonly IDotNet dotnet; + private readonly FileContent fileContent; + private readonly FileProvider fileProvider; // Only used as a set, but ConcurrentDictionary is the only concurrent set in .NET. private readonly IDictionary usedReferences = new ConcurrentDictionary(); @@ -30,18 +42,14 @@ namespace Semmle.Extraction.CSharp.DependencyFetching private int conflictedReferences = 0; private readonly DirectoryInfo sourceDir; private string? dotnetPath; - private readonly IDotNet dotnet; - private readonly FileContent fileContent; - private readonly TemporaryDirectory packageDirectory; - private readonly TemporaryDirectory legacyPackageDirectory; - private readonly TemporaryDirectory missingPackageDirectory; + private readonly TemporaryDirectory tempWorkingDirectory; - private readonly FileProvider fileProvider; private readonly bool cleanupTempWorkingDirectory; private readonly Lazy runtimeLazy; private Runtime Runtime => runtimeLazy.Value; - private readonly int threads = EnvironmentVariables.GetDefaultNumberOfThreads(); + + internal static readonly int Threads = EnvironmentVariables.GetDefaultNumberOfThreads(); /// /// Performs C# dependency fetching. @@ -74,10 +82,6 @@ namespace Semmle.Extraction.CSharp.DependencyFetching $"dependency-manager-{DateTime.UtcNow:yyyyMMddHHmm}-{Environment.ProcessId}.jsonc")); this.sourceDir = new DirectoryInfo(srcDir); - packageDirectory = new TemporaryDirectory(ComputeTempDirectory(sourceDir.FullName, "packages")); - legacyPackageDirectory = new TemporaryDirectory(ComputeTempDirectory(sourceDir.FullName, "legacypackages")); - missingPackageDirectory = new TemporaryDirectory(ComputeTempDirectory(sourceDir.FullName, "missingpackages")); - tempWorkingDirectory = new TemporaryDirectory(FileUtils.GetTemporaryWorkingDirectory(out cleanupTempWorkingDirectory)); this.fileProvider = new FileProvider(sourceDir, logger); @@ -112,8 +116,10 @@ namespace Semmle.Extraction.CSharp.DependencyFetching throw; } + nugetPackageRestorer = new NugetPackageRestorer(fileProvider, fileContent, dotnet, diagnosticsWriter, logger, this); + var dllLocations = fileProvider.Dlls.Select(x => new AssemblyLookupLocation(x)).ToHashSet(); - RestoreNugetPackages(dllLocations); + dllLocations.UnionWith(nugetPackageRestorer.Restore()); // Find DLLs in the .Net / Asp.Net Framework // This needs to come after the nuget restore, because the nuget restore might fetch the .NET Core/Framework reference assemblies. var frameworkLocations = AddFrameworkDlls(dllLocations); @@ -221,11 +227,7 @@ namespace Semmle.Extraction.CSharp.DependencyFetching private void RemoveNugetAnalyzerReferences() { - var packageFolder = packageDirectory.DirInfo.FullName.ToLowerInvariant(); - if (packageFolder == null) - { - return; - } + var packageFolder = nugetPackageRestorer.PackageDirectory.DirInfo.FullName.ToLowerInvariant(); foreach (var filename in usedReferences.Keys) { @@ -299,7 +301,7 @@ namespace Semmle.Extraction.CSharp.DependencyFetching var packagesInPrioOrder = FrameworkPackageNames.NetFrameworks; var frameworkPaths = packagesInPrioOrder - .Select((s, index) => (Index: index, Path: GetPackageDirectory(s, packageDirectory))) + .Select((s, index) => (Index: index, Path: GetPackageDirectory(s))) .Where(pair => pair.Path is not null) .ToArray(); @@ -330,11 +332,7 @@ namespace Semmle.Extraction.CSharp.DependencyFetching if (runtimeLocation is null) { logger.LogInfo("No .NET Desktop Runtime location found. Attempting to restore the .NET Framework reference assemblies manually."); - - if (TryRestorePackageManually(FrameworkPackageNames.LatestNetFrameworkReferenceAssemblies)) - { - runtimeLocation = GetPackageDirectory(FrameworkPackageNames.LatestNetFrameworkReferenceAssemblies, missingPackageDirectory); - } + runtimeLocation = nugetPackageRestorer.TryRestoreLatestNetFrameworkReferenceAssemblies(); } } @@ -354,12 +352,7 @@ namespace Semmle.Extraction.CSharp.DependencyFetching private void RemoveNugetPackageReference(string packagePrefix, ISet dllLocations) { - var packageFolder = packageDirectory.DirInfo.FullName.ToLowerInvariant(); - if (packageFolder == null) - { - return; - } - + var packageFolder = nugetPackageRestorer.PackageDirectory.DirInfo.FullName.ToLowerInvariant(); var packagePathPrefix = Path.Combine(packageFolder, packagePrefix.ToLowerInvariant()); var toRemove = dllLocations.Where(s => s.Path.StartsWith(packagePathPrefix, StringComparison.InvariantCultureIgnoreCase)); foreach (var path in toRemove) @@ -382,7 +375,7 @@ namespace Semmle.Extraction.CSharp.DependencyFetching } // First try to find ASP.NET Core assemblies in the NuGet packages - if (GetPackageDirectory(FrameworkPackageNames.AspNetCoreFramework, packageDirectory) is string aspNetCorePackage) + if (GetPackageDirectory(FrameworkPackageNames.AspNetCoreFramework) is string aspNetCorePackage) { SelectNewestFrameworkPath(aspNetCorePackage, "ASP.NET Core", dllLocations, frameworkLocations); return; @@ -398,15 +391,20 @@ namespace Semmle.Extraction.CSharp.DependencyFetching private void AddMicrosoftWindowsDesktopDlls(ISet dllLocations, ISet frameworkLocations) { - if (GetPackageDirectory(FrameworkPackageNames.WindowsDesktopFramework, packageDirectory) is string windowsDesktopApp) + if (GetPackageDirectory(FrameworkPackageNames.WindowsDesktopFramework) is string windowsDesktopApp) { SelectNewestFrameworkPath(windowsDesktopApp, "Windows Desktop App", dllLocations, frameworkLocations); } } - private string? GetPackageDirectory(string packagePrefix, TemporaryDirectory root) + private string? GetPackageDirectory(string packagePrefix) { - return new DirectoryInfo(root.DirInfo.FullName) + return GetPackageDirectory(packagePrefix, nugetPackageRestorer.PackageDirectory.DirInfo); + } + + internal static string? GetPackageDirectory(string packagePrefix, DirectoryInfo root) + { + return new DirectoryInfo(root.FullName) .EnumerateDirectories(packagePrefix + "*", new EnumerationOptions { MatchCasing = MatchCasing.CaseInsensitive, RecurseSubdirectories = false }) .FirstOrDefault()? .FullName; @@ -495,22 +493,6 @@ namespace Semmle.Extraction.CSharp.DependencyFetching } } - /// - /// Computes a unique temp directory for the packages associated - /// with this source tree. Use a SHA1 of the directory name. - /// - /// The full path of the temp directory. - private static string ComputeTempDirectory(string srcDir, string subfolderName) - { - var bytes = Encoding.Unicode.GetBytes(srcDir); - var sha = SHA1.HashData(bytes); - var sb = new StringBuilder(); - foreach (var b in sha.Take(8)) - sb.AppendFormat("{0:x2}", b); - - return Path.Combine(FileUtils.GetTemporaryWorkingDirectory(out var _), sb.ToString(), subfolderName); - } - /// /// Creates a temporary directory with the given subfolder name. /// The created directory might be inside the repo folder, and it is deleted when the object is disposed. @@ -634,7 +616,7 @@ namespace Semmle.Extraction.CSharp.DependencyFetching private void AnalyseSolutions(IEnumerable solutions) { - Parallel.ForEach(solutions, new ParallelOptions { MaxDegreeOfParallelism = threads }, solutionFile => + Parallel.ForEach(solutions, new ParallelOptions { MaxDegreeOfParallelism = Threads }, solutionFile => { try { @@ -683,7 +665,7 @@ namespace Semmle.Extraction.CSharp.DependencyFetching } } - public void Dispose(TemporaryDirectory? dir, string name) + public static void DisposeTempDirectory(TemporaryDirectory? dir, string name, ILogger logger) { try { @@ -697,15 +679,13 @@ namespace Semmle.Extraction.CSharp.DependencyFetching public void Dispose() { - Dispose(packageDirectory, "package"); - Dispose(legacyPackageDirectory, "legacy package"); - Dispose(missingPackageDirectory, "missing package"); if (cleanupTempWorkingDirectory) { - Dispose(tempWorkingDirectory, "temporary working"); + DisposeTempDirectory(tempWorkingDirectory, "temporary working", logger); } diagnosticsWriter?.Dispose(); + nugetPackageRestorer?.Dispose(); } } } diff --git a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/FileProvider.cs b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/FileProvider.cs index 320b7d70435..098087c7424 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/FileProvider.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/FileProvider.cs @@ -12,7 +12,7 @@ namespace Semmle.Extraction.CSharp.DependencyFetching private static readonly HashSet binaryFileExtensions = [".dll", ".exe"]; // TODO: add more binary file extensions. private readonly ILogger logger; - private readonly Lazy all; + private readonly FileInfo[] all; private readonly Lazy allNonBinary; private readonly Lazy smallNonBinary; private readonly Lazy sources; @@ -29,8 +29,8 @@ namespace Semmle.Extraction.CSharp.DependencyFetching SourceDir = sourceDir; this.logger = logger; - all = new Lazy(GetAllFiles); - allNonBinary = new Lazy(() => all.Value.Where(f => !binaryFileExtensions.Contains(f.Extension.ToLowerInvariant())).ToArray()); + all = GetAllFiles(); + allNonBinary = new Lazy(() => all.Where(f => !binaryFileExtensions.Contains(f.Extension.ToLowerInvariant())).ToArray()); smallNonBinary = new Lazy(() => { var ret = SelectSmallFiles(allNonBinary.Value).SelectFileNames().ToArray(); @@ -45,7 +45,7 @@ namespace Semmle.Extraction.CSharp.DependencyFetching globalJsons = new Lazy(() => allNonBinary.Value.SelectFileNamesByName("global.json").ToArray()); razorViews = new Lazy(() => SelectTextFileNamesByExtension("razor view", ".cshtml", ".razor")); - rootNugetConfig = new Lazy(() => all.Value.SelectRootFiles(SourceDir).SelectFileNamesByName("nuget.config").FirstOrDefault()); + rootNugetConfig = new Lazy(() => all.SelectRootFiles(SourceDir).SelectFileNamesByName("nuget.config").FirstOrDefault()); } private string[] SelectTextFileNamesByExtension(string filetype, params string[] extensions) @@ -57,7 +57,7 @@ namespace Semmle.Extraction.CSharp.DependencyFetching private string[] SelectBinaryFileNamesByExtension(string filetype, params string[] extensions) { - var ret = all.Value.SelectFileNamesByExtension(extensions).ToArray(); + var ret = all.SelectFileNamesByExtension(extensions).ToArray(); logger.LogInfo($"Found {ret.Length} {filetype} files in {SourceDir}."); return ret; } diff --git a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackages.cs b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetExeWrapper.cs similarity index 96% rename from csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackages.cs rename to csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetExeWrapper.cs index 4926b64acd3..9db023a3b45 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackages.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetExeWrapper.cs @@ -8,11 +8,11 @@ using Semmle.Util; namespace Semmle.Extraction.CSharp.DependencyFetching { /// - /// Manage the downloading of NuGet packages. + /// Manage the downloading of NuGet packages with nuget.exe. /// Locates packages in a source tree and downloads all of the /// referenced assemblies to a temp folder. /// - internal class NugetPackages : IDisposable + internal class NugetExeWrapper : IDisposable { private readonly string? nugetExe; private readonly Util.Logging.ILogger logger; @@ -37,7 +37,7 @@ namespace Semmle.Extraction.CSharp.DependencyFetching /// /// Create the package manager for a specified source tree. /// - public NugetPackages(string sourceDir, TemporaryDirectory packageDirectory, Util.Logging.ILogger logger) + public NugetExeWrapper(string sourceDir, TemporaryDirectory packageDirectory, Util.Logging.ILogger logger) { this.packageDirectory = packageDirectory; this.logger = logger; @@ -243,7 +243,7 @@ namespace Semmle.Extraction.CSharp.DependencyFetching private void AddDefaultPackageSource(string nugetConfig) { logger.LogInfo("Adding default package source..."); - RunMonoNugetCommand($"sources add -Name DefaultNugetOrg -Source {DependencyManager.PublicNugetFeed} -ConfigFile \"{nugetConfig}\"", out var _); + RunMonoNugetCommand($"sources add -Name DefaultNugetOrg -Source {NugetPackageRestorer.PublicNugetOrgFeed} -ConfigFile \"{nugetConfig}\"", out var _); } public void Dispose() diff --git a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependencyManager.Nuget.cs b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs similarity index 78% rename from csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependencyManager.Nuget.cs rename to csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs index e39d59690b5..0e8d2f37063 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependencyManager.Nuget.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs @@ -3,36 +3,84 @@ using System.Collections.Generic; using System.IO; using System.Linq; using System.Net.Http; +using System.Security.Cryptography; using System.Text; using System.Text.RegularExpressions; using System.Threading; using System.Threading.Tasks; using Semmle.Util; +using Semmle.Util.Logging; namespace Semmle.Extraction.CSharp.DependencyFetching { - public sealed partial class DependencyManager + internal sealed partial class NugetPackageRestorer : IDisposable { - private void RestoreNugetPackages(HashSet dllLocations) + internal const string PublicNugetOrgFeed = "https://api.nuget.org/v3/index.json"; + + private readonly FileProvider fileProvider; + private readonly FileContent fileContent; + private readonly IDotNet dotnet; + private readonly IDiagnosticsWriter diagnosticsWriter; + private readonly TemporaryDirectory legacyPackageDirectory; + private readonly TemporaryDirectory missingPackageDirectory; + private readonly ILogger logger; + private readonly ICompilationInfoContainer compilationInfoContainer; + + public TemporaryDirectory PackageDirectory { get; } + + public NugetPackageRestorer( + FileProvider fileProvider, + FileContent fileContent, + IDotNet dotnet, + IDiagnosticsWriter diagnosticsWriter, + ILogger logger, + ICompilationInfoContainer compilationInfoContainer) { + this.fileProvider = fileProvider; + this.fileContent = fileContent; + this.dotnet = dotnet; + this.diagnosticsWriter = diagnosticsWriter; + this.logger = logger; + this.compilationInfoContainer = compilationInfoContainer; + + PackageDirectory = new TemporaryDirectory(ComputeTempDirectoryPath(fileProvider.SourceDir.FullName, "packages")); + legacyPackageDirectory = new TemporaryDirectory(ComputeTempDirectoryPath(fileProvider.SourceDir.FullName, "legacypackages")); + missingPackageDirectory = new TemporaryDirectory(ComputeTempDirectoryPath(fileProvider.SourceDir.FullName, "missingpackages")); + } + + public string? TryRestoreLatestNetFrameworkReferenceAssemblies() + { + if (TryRestorePackageManually(FrameworkPackageNames.LatestNetFrameworkReferenceAssemblies)) + { + return DependencyManager.GetPackageDirectory(FrameworkPackageNames.LatestNetFrameworkReferenceAssemblies, missingPackageDirectory.DirInfo); + } + + return null; + } + + public HashSet Restore() + { + var assemblyLookupLocations = new HashSet(); var checkNugetFeedResponsiveness = EnvironmentVariables.GetBoolean(EnvironmentVariableNames.CheckNugetFeedResponsiveness); try { if (checkNugetFeedResponsiveness && !CheckFeeds()) { // todo: we could also check the reachability of the inherited nuget feeds, but to use those in the fallback we would need to handle authentication too. - DownloadMissingPackagesFromSpecificFeeds(dllLocations); - return; + var unresponsiveMissingPackageLocation = DownloadMissingPackagesFromSpecificFeeds(); + return unresponsiveMissingPackageLocation is null + ? [] + : [unresponsiveMissingPackageLocation]; } - using (var nuget = new NugetPackages(sourceDir.FullName, legacyPackageDirectory, logger)) + using (var nuget = new NugetExeWrapper(fileProvider.SourceDir.FullName, legacyPackageDirectory, logger)) { var count = nuget.InstallPackages(); if (nuget.PackageCount > 0) { - CompilationInfos.Add(("packages.config files", nuget.PackageCount.ToString())); - CompilationInfos.Add(("Successfully restored packages.config files", count.ToString())); + compilationInfoContainer.CompilationInfos.Add(("packages.config files", nuget.PackageCount.ToString())); + compilationInfoContainer.CompilationInfos.Add(("Successfully restored packages.config files", count.ToString())); } } @@ -57,7 +105,7 @@ namespace Semmle.Extraction.CSharp.DependencyFetching } nugetPackageDllPaths.ExceptWith(excludedPaths); - dllLocations.UnionWith(nugetPackageDllPaths.Select(p => new AssemblyLookupLocation(p))); + assemblyLookupLocations.UnionWith(nugetPackageDllPaths.Select(p => new AssemblyLookupLocation(p))); } catch (Exception exc) { @@ -72,31 +120,30 @@ namespace Semmle.Extraction.CSharp.DependencyFetching var paths = dependencies .Paths - .Select(d => Path.Combine(packageDirectory.DirInfo.FullName, d)) + .Select(d => Path.Combine(PackageDirectory.DirInfo.FullName, d)) .ToList(); - dllLocations.UnionWith(paths.Select(p => new AssemblyLookupLocation(p))); + assemblyLookupLocations.UnionWith(paths.Select(p => new AssemblyLookupLocation(p))); LogAllUnusedPackages(dependencies); - if (checkNugetFeedResponsiveness) - { - DownloadMissingPackagesFromSpecificFeeds(dllLocations); - } - else - { - DownloadMissingPackages(dllLocations); - } - } + var missingPackageLocation = checkNugetFeedResponsiveness + ? DownloadMissingPackagesFromSpecificFeeds() + : DownloadMissingPackages(); - internal const string PublicNugetFeed = "https://api.nuget.org/v3/index.json"; + if (missingPackageLocation is not null) + { + assemblyLookupLocations.Add(missingPackageLocation); + } + return assemblyLookupLocations; + } private List GetReachableFallbackNugetFeeds() { var fallbackFeeds = EnvironmentVariables.GetURLs(EnvironmentVariableNames.FallbackNugetFeeds).ToHashSet(); if (fallbackFeeds.Count == 0) { - fallbackFeeds.Add(PublicNugetFeed); - logger.LogInfo($"No fallback Nuget feeds specified. Using default feed: {PublicNugetFeed}"); + fallbackFeeds.Add(PublicNugetOrgFeed); + logger.LogInfo($"No fallback Nuget feeds specified. Using default feed: {PublicNugetOrgFeed}"); } logger.LogInfo($"Checking fallback Nuget feed reachability on feeds: {string.Join(", ", fallbackFeeds.OrderBy(f => f))}"); @@ -130,7 +177,7 @@ namespace Semmle.Extraction.CSharp.DependencyFetching var projects = fileProvider.Solutions.SelectMany(solution => { logger.LogInfo($"Restoring solution {solution}..."); - var res = dotnet.Restore(new(solution, packageDirectory.DirInfo.FullName, ForceDotnetRefAssemblyFetching: true)); + var res = dotnet.Restore(new(solution, PackageDirectory.DirInfo.FullName, ForceDotnetRefAssemblyFetching: true)); if (res.Success) { successCount++; @@ -143,9 +190,9 @@ namespace Semmle.Extraction.CSharp.DependencyFetching return res.RestoredProjects; }).ToList(); assets = assetFiles; - CompilationInfos.Add(("Successfully restored solution files", successCount.ToString())); - CompilationInfos.Add(("Failed solution restore with package source error", nugetSourceFailures.ToString())); - CompilationInfos.Add(("Restored projects through solution files", projects.Count.ToString())); + compilationInfoContainer.CompilationInfos.Add(("Successfully restored solution files", successCount.ToString())); + compilationInfoContainer.CompilationInfos.Add(("Failed solution restore with package source error", nugetSourceFailures.ToString())); + compilationInfoContainer.CompilationInfos.Add(("Restored projects through solution files", projects.Count.ToString())); return projects; } @@ -161,10 +208,10 @@ namespace Semmle.Extraction.CSharp.DependencyFetching var nugetSourceFailures = 0; var assetFiles = new List(); var sync = new object(); - Parallel.ForEach(projects, new ParallelOptions { MaxDegreeOfParallelism = threads }, project => + Parallel.ForEach(projects, new ParallelOptions { MaxDegreeOfParallelism = DependencyManager.Threads }, project => { logger.LogInfo($"Restoring project {project}..."); - var res = dotnet.Restore(new(project, packageDirectory.DirInfo.FullName, ForceDotnetRefAssemblyFetching: true)); + var res = dotnet.Restore(new(project, PackageDirectory.DirInfo.FullName, ForceDotnetRefAssemblyFetching: true)); lock (sync) { if (res.Success) @@ -179,26 +226,25 @@ namespace Semmle.Extraction.CSharp.DependencyFetching } }); assets = assetFiles; - CompilationInfos.Add(("Successfully restored project files", successCount.ToString())); - CompilationInfos.Add(("Failed project restore with package source error", nugetSourceFailures.ToString())); + compilationInfoContainer.CompilationInfos.Add(("Successfully restored project files", successCount.ToString())); + compilationInfoContainer.CompilationInfos.Add(("Failed project restore with package source error", nugetSourceFailures.ToString())); } - private void DownloadMissingPackagesFromSpecificFeeds(HashSet dllLocations) + private AssemblyLookupLocation? DownloadMissingPackagesFromSpecificFeeds() { var reachableFallbackFeeds = GetReachableFallbackNugetFeeds(); if (reachableFallbackFeeds.Count > 0) { - DownloadMissingPackages(dllLocations, fallbackNugetFeeds: reachableFallbackFeeds); - } - else - { - logger.LogWarning("Skipping download of missing packages from specific feeds as no fallback Nuget feeds are reachable."); + return DownloadMissingPackages(fallbackNugetFeeds: reachableFallbackFeeds); } + + logger.LogWarning("Skipping download of missing packages from specific feeds as no fallback Nuget feeds are reachable."); + return null; } - private void DownloadMissingPackages(HashSet dllLocations, IEnumerable? fallbackNugetFeeds = null) + private AssemblyLookupLocation? DownloadMissingPackages(IEnumerable? fallbackNugetFeeds = null) { - var alreadyDownloadedPackages = GetRestoredPackageDirectoryNames(packageDirectory.DirInfo); + var alreadyDownloadedPackages = GetRestoredPackageDirectoryNames(PackageDirectory.DirInfo); var alreadyDownloadedLegacyPackages = GetRestoredLegacyPackageNames(); var notYetDownloadedPackages = new HashSet(fileContent.AllPackages); @@ -213,7 +259,7 @@ namespace Semmle.Extraction.CSharp.DependencyFetching if (notYetDownloadedPackages.Count == 0) { - return; + return null; } var multipleVersions = notYetDownloadedPackages @@ -229,17 +275,17 @@ namespace Semmle.Extraction.CSharp.DependencyFetching } logger.LogInfo($"Found {notYetDownloadedPackages.Count} packages that are not yet restored"); - using var tempDir = new TemporaryDirectory(ComputeTempDirectory(sourceDir.FullName, "nugetconfig")); + using var tempDir = new TemporaryDirectory(ComputeTempDirectoryPath(fileProvider.SourceDir.FullName, "nugetconfig")); var nugetConfig = fallbackNugetFeeds is null ? GetNugetConfig() : CreateFallbackNugetConfig(fallbackNugetFeeds, tempDir.DirInfo.FullName); - CompilationInfos.Add(("Fallback nuget restore", notYetDownloadedPackages.Count.ToString())); + compilationInfoContainer.CompilationInfos.Add(("Fallback nuget restore", notYetDownloadedPackages.Count.ToString())); var successCount = 0; var sync = new object(); - Parallel.ForEach(notYetDownloadedPackages, new ParallelOptions { MaxDegreeOfParallelism = threads }, package => + Parallel.ForEach(notYetDownloadedPackages, new ParallelOptions { MaxDegreeOfParallelism = DependencyManager.Threads }, package => { var success = TryRestorePackageManually(package.Name, nugetConfig, package.PackageReferenceSource, tryWithoutNugetConfig: fallbackNugetFeeds is null); if (!success) @@ -253,9 +299,9 @@ namespace Semmle.Extraction.CSharp.DependencyFetching } }); - CompilationInfos.Add(("Successfully ran fallback nuget restore", successCount.ToString())); + compilationInfoContainer.CompilationInfos.Add(("Successfully ran fallback nuget restore", successCount.ToString())); - dllLocations.Add(missingPackageDirectory.DirInfo.FullName); + return missingPackageDirectory.DirInfo.FullName; } private string? CreateFallbackNugetConfig(IEnumerable fallbackNugetFeeds, string folderPath) @@ -318,10 +364,9 @@ namespace Semmle.Extraction.CSharp.DependencyFetching .ForEach(package => logger.LogInfo($"Unused package: {package}")); } - private ICollection GetAllPackageDirectories() { - return new DirectoryInfo(packageDirectory.DirInfo.FullName) + return new DirectoryInfo(PackageDirectory.DirInfo.FullName) .EnumerateDirectories("*", new EnumerationOptions { MatchCasing = MatchCasing.CaseInsensitive, RecurseSubdirectories = false }) .Select(d => d.Name) .ToList(); @@ -366,7 +411,7 @@ namespace Semmle.Extraction.CSharp.DependencyFetching private bool TryRestorePackageManually(string package, string? nugetConfig = null, PackageReferenceSource packageReferenceSource = PackageReferenceSource.SdkCsProj, bool tryWithoutNugetConfig = true) { logger.LogInfo($"Restoring package {package}..."); - using var tempDir = new TemporaryDirectory(ComputeTempDirectory(package, "missingpackages_workingdir")); + using var tempDir = new TemporaryDirectory(ComputeTempDirectoryPath(package, "missingpackages_workingdir")); var success = dotnet.New(tempDir.DirInfo.FullName); if (!success) { @@ -534,14 +579,14 @@ namespace Semmle.Extraction.CSharp.DependencyFetching severity: DiagnosticMessage.TspSeverity.Warning )); } - CompilationInfos.Add(("All Nuget feeds reachable", allFeedsReachable ? "1" : "0")); + compilationInfoContainer.CompilationInfos.Add(("All Nuget feeds reachable", allFeedsReachable ? "1" : "0")); var inheritedFeeds = allFeeds.Except(explicitFeeds).ToHashSet(); if (inheritedFeeds.Count > 0) { logger.LogInfo($"Inherited Nuget feeds (not checked for reachability): {string.Join(", ", inheritedFeeds.OrderBy(f => f))}"); - CompilationInfos.Add(("Inherited Nuget feed count", inheritedFeeds.Count.ToString())); + compilationInfoContainer.CompilationInfos.Add(("Inherited Nuget feed count", inheritedFeeds.Count.ToString())); } return allFeedsReachable; @@ -627,5 +672,28 @@ namespace Semmle.Extraction.CSharp.DependencyFetching [GeneratedRegex(@"^E\s(.*)$", RegexOptions.IgnoreCase | RegexOptions.Compiled | RegexOptions.Singleline)] private static partial Regex EnabledNugetFeed(); + + public void Dispose() + { + DependencyManager.DisposeTempDirectory(PackageDirectory, "package", logger); + DependencyManager.DisposeTempDirectory(legacyPackageDirectory, "legacy package", logger); + DependencyManager.DisposeTempDirectory(missingPackageDirectory, "missing package", logger); + } + + /// + /// Computes a unique temp directory for the packages associated + /// with this source tree. Use a SHA1 of the directory name. + /// + /// The full path of the temp directory. + private static string ComputeTempDirectoryPath(string srcDir, string subfolderName) + { + var bytes = Encoding.Unicode.GetBytes(srcDir); + var sha = SHA1.HashData(bytes); + var sb = new StringBuilder(); + foreach (var b in sha.Take(8)) + sb.AppendFormat("{0:x2}", b); + + return Path.Combine(FileUtils.GetTemporaryWorkingDirectory(out var _), sb.ToString(), subfolderName); + } } } From 69c43094ba5016e6ba83b5069ff7a662e4a2bf48 Mon Sep 17 00:00:00 2001 From: Tamas Vajk Date: Mon, 15 Apr 2024 09:32:18 +0200 Subject: [PATCH 7/7] Fix review findings --- .../DependencyManager.cs | 23 ++++--------------- .../NugetPackageRestorer.cs | 17 +++++++------- .../Semmle.Util/TemporaryDirectory.cs | 17 ++++++++++++-- 3 files changed, 29 insertions(+), 28 deletions(-) diff --git a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependencyManager.cs b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependencyManager.cs index 357bc0026cc..efb200c6822 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependencyManager.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/DependencyManager.cs @@ -82,7 +82,10 @@ namespace Semmle.Extraction.CSharp.DependencyFetching $"dependency-manager-{DateTime.UtcNow:yyyyMMddHHmm}-{Environment.ProcessId}.jsonc")); this.sourceDir = new DirectoryInfo(srcDir); - tempWorkingDirectory = new TemporaryDirectory(FileUtils.GetTemporaryWorkingDirectory(out cleanupTempWorkingDirectory)); + tempWorkingDirectory = new TemporaryDirectory( + FileUtils.GetTemporaryWorkingDirectory(out cleanupTempWorkingDirectory), + "temporary working", + logger); this.fileProvider = new FileProvider(sourceDir, logger); this.fileContent = new FileContent(logger, this.fileProvider.SmallNonBinary); @@ -665,25 +668,9 @@ namespace Semmle.Extraction.CSharp.DependencyFetching } } - public static void DisposeTempDirectory(TemporaryDirectory? dir, string name, ILogger logger) - { - try - { - dir?.Dispose(); - } - catch (Exception exc) - { - logger.LogInfo($"Couldn't delete {name} directory {exc.Message}"); - } - } - public void Dispose() { - if (cleanupTempWorkingDirectory) - { - DisposeTempDirectory(tempWorkingDirectory, "temporary working", logger); - } - + tempWorkingDirectory?.Dispose(); diagnosticsWriter?.Dispose(); nugetPackageRestorer?.Dispose(); } diff --git a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs index 0e8d2f37063..313912df88e 100644 --- a/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs +++ b/csharp/extractor/Semmle.Extraction.CSharp.DependencyFetching/NugetPackageRestorer.cs @@ -43,9 +43,9 @@ namespace Semmle.Extraction.CSharp.DependencyFetching this.logger = logger; this.compilationInfoContainer = compilationInfoContainer; - PackageDirectory = new TemporaryDirectory(ComputeTempDirectoryPath(fileProvider.SourceDir.FullName, "packages")); - legacyPackageDirectory = new TemporaryDirectory(ComputeTempDirectoryPath(fileProvider.SourceDir.FullName, "legacypackages")); - missingPackageDirectory = new TemporaryDirectory(ComputeTempDirectoryPath(fileProvider.SourceDir.FullName, "missingpackages")); + PackageDirectory = new TemporaryDirectory(ComputeTempDirectoryPath(fileProvider.SourceDir.FullName, "packages"), "package", logger); + legacyPackageDirectory = new TemporaryDirectory(ComputeTempDirectoryPath(fileProvider.SourceDir.FullName, "legacypackages"), "legacy package", logger); + missingPackageDirectory = new TemporaryDirectory(ComputeTempDirectoryPath(fileProvider.SourceDir.FullName, "missingpackages"), "missing package", logger); } public string? TryRestoreLatestNetFrameworkReferenceAssemblies() @@ -275,7 +275,7 @@ namespace Semmle.Extraction.CSharp.DependencyFetching } logger.LogInfo($"Found {notYetDownloadedPackages.Count} packages that are not yet restored"); - using var tempDir = new TemporaryDirectory(ComputeTempDirectoryPath(fileProvider.SourceDir.FullName, "nugetconfig")); + using var tempDir = new TemporaryDirectory(ComputeTempDirectoryPath(fileProvider.SourceDir.FullName, "nugetconfig"), "generated nuget config", logger); var nugetConfig = fallbackNugetFeeds is null ? GetNugetConfig() : CreateFallbackNugetConfig(fallbackNugetFeeds, tempDir.DirInfo.FullName); @@ -411,7 +411,8 @@ namespace Semmle.Extraction.CSharp.DependencyFetching private bool TryRestorePackageManually(string package, string? nugetConfig = null, PackageReferenceSource packageReferenceSource = PackageReferenceSource.SdkCsProj, bool tryWithoutNugetConfig = true) { logger.LogInfo($"Restoring package {package}..."); - using var tempDir = new TemporaryDirectory(ComputeTempDirectoryPath(package, "missingpackages_workingdir")); + using var tempDir = new TemporaryDirectory( + ComputeTempDirectoryPath(package, "missingpackages_workingdir"), "missing package working", logger); var success = dotnet.New(tempDir.DirInfo.FullName); if (!success) { @@ -675,9 +676,9 @@ namespace Semmle.Extraction.CSharp.DependencyFetching public void Dispose() { - DependencyManager.DisposeTempDirectory(PackageDirectory, "package", logger); - DependencyManager.DisposeTempDirectory(legacyPackageDirectory, "legacy package", logger); - DependencyManager.DisposeTempDirectory(missingPackageDirectory, "missing package", logger); + PackageDirectory?.Dispose(); + legacyPackageDirectory?.Dispose(); + missingPackageDirectory?.Dispose(); } /// diff --git a/csharp/extractor/Semmle.Util/TemporaryDirectory.cs b/csharp/extractor/Semmle.Util/TemporaryDirectory.cs index ac5653afc78..a499209cdbe 100644 --- a/csharp/extractor/Semmle.Util/TemporaryDirectory.cs +++ b/csharp/extractor/Semmle.Util/TemporaryDirectory.cs @@ -1,5 +1,6 @@ using System; using System.IO; +using Semmle.Util.Logging; namespace Semmle.Util { @@ -9,17 +10,29 @@ namespace Semmle.Util /// public sealed class TemporaryDirectory : IDisposable { + private readonly string userReportedDirectoryPurpose; + private readonly ILogger logger; + public DirectoryInfo DirInfo { get; } - public TemporaryDirectory(string name) + public TemporaryDirectory(string name, string userReportedDirectoryPurpose, ILogger logger) { DirInfo = new DirectoryInfo(name); DirInfo.Create(); + this.userReportedDirectoryPurpose = userReportedDirectoryPurpose; + this.logger = logger; } public void Dispose() { - DirInfo.Delete(true); + try + { + DirInfo.Delete(true); + } + catch (Exception exc) + { + logger.LogInfo($"Couldn't delete {userReportedDirectoryPurpose} directory {exc.Message}"); + } } public override string ToString() => DirInfo.FullName.ToString();