Fix ArgumentException when recurse is enabled and two file target s…

…pecifiers generates the same file paths (#2438) * Fixing ArgumentException when passing two filePaths that generates duplicated file analysis * Fixing dotnet-format issues and updating releasehistory * Removing comments * Addressing PR feedback * Addressing PR feedback
microsoft · Feb 8, 2022 · 0727702 · 0727702
1 parent 9f140e4
commit 0727702
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 16 deletions.
diff --git a/src/ReleaseHistory.md b/src/ReleaseHistory.md
@@ -6,6 +6,7 @@
 * BUGFIX: Adjust Json Serialization field order for ReportingDescriptor and skip emit empty AutomationDetails node. [#2420](https://github.com/microsoft/sarif-sdk/pull/2420)
 * BREAKING: Fix `InvalidOperationException` when using PropertiesDictionary in a multithreaded application, and remove `[Serializable]` from it. Now use of BinaryFormatter on it will result in `SerializationException`: Type `PropertiesDictionary` is not marked as serializable. [#2415](https://github.com/microsoft/sarif-sdk/pull/2415)
 * BREAKING: `SarifLogger` now emits an artifacts table entry if `artifactLocation` is not null for tool configuration and tool execution notifications. [#2437](https://github.com/microsoft/sarif-sdk/pull/2437)
+* BUGFIX: Fix `ArgumentException` when `--recurse` is enabled and two file target specifiers generates the same file path. [#2438](https://github.com/microsoft/sarif-sdk/pull/2438)
 
 ## **v2.4.12** [Sdk](https://www.nuget.org/packages/Sarif.Sdk/2.4.12) | [Driver](https://www.nuget.org/packages/Sarif.Driver/2.4.12) | [Converters](https://www.nuget.org/packages/Sarif.Converters/2.4.12) | [Multitool](https://www.nuget.org/packages/Sarif.Multitool/2.4.12) | [Multitool Library](https://www.nuget.org/packages/Sarif.Multitool.Library/2.4.12)
 

diff --git a/src/Sarif.Driver/Sdk/MultithreadedAnalyzeCommandBase.cs b/src/Sarif.Driver/Sdk/MultithreadedAnalyzeCommandBase.cs
@@ -488,7 +488,11 @@ private async Task<bool> HashAsync()
 
                         paths.Add(localPath);
                         context.Hashes = hashData;
-                        _pathToHashDataMap?.Add(localPath, hashData);
+
+                        if (_pathToHashDataMap != null && !_pathToHashDataMap.ContainsKey(localPath))
+                        {
+                            _pathToHashDataMap.Add(localPath, hashData);
+                        }
                     }
 
                     await _fileEnumerationChannel.Writer.WriteAsync(index);

diff --git a/src/Test.UnitTests.Sarif.Driver/Sdk/AnalyzeCommandBaseTests.cs b/src/Test.UnitTests.Sarif.Driver/Sdk/AnalyzeCommandBaseTests.cs
@@ -1210,19 +1210,7 @@ public void AnalyzeCommandBase_ShouldOnlyLogArtifactsWhenResultsAreFound()
                 $@"{Environment.CurrentDirectory}\NoIssues.dll",
             };
 
-            var testCases = new[]
-            {
-                new
-                {
-                    IsMultithreaded = false
-                },
-                new
-                {
-                    IsMultithreaded = true
-                }
-            };
-
-            foreach (var testCase in testCases)
+            foreach (bool multithreaded in new bool[] { false, true })
             {
                 var resultsCachingTestCase = new ResultsCachingTestCase
                 {
@@ -1241,7 +1229,7 @@ public void AnalyzeCommandBase_ShouldOnlyLogArtifactsWhenResultsAreFound()
                     DataToInsert = new OptionallyEmittedData[] { OptionallyEmittedData.Hashes },
                 };
 
-                Run run = RunAnalyzeCommand(options, resultsCachingTestCase, multithreaded: testCase.IsMultithreaded);
+                Run run = RunAnalyzeCommand(options, resultsCachingTestCase, multithreaded: multithreaded);
 
                 // Hashes is enabled and we should expect to see two artifacts because we have:
                 // one result with Error level and one result with Warning level.
@@ -1251,6 +1239,46 @@ public void AnalyzeCommandBase_ShouldOnlyLogArtifactsWhenResultsAreFound()
             }
         }
 
+        [Fact]
+        public void AnalyzeCommandBase_ShouldNotThrowException_WhenAnalyzingSameFileBasedOnTwoTargetFileSpecifiers()
+        {
+            var files = new List<string>
+            {
+                $@"{Environment.CurrentDirectory}\Error.dll"
+            };
+
+            Action action = () =>
+            {
+                foreach (bool multithreaded in new bool[] { false, true })
+                {
+                    var resultsCachingTestCase = new ResultsCachingTestCase
+                    {
+                        Files = files,
+                        PersistLogFileToDisk = true,
+                        FileSystem = CreateDefaultFileSystemForResultsCaching(files, generateSameInput: true)
+                    };
+
+                    var options = new TestAnalyzeOptions
+                    {
+                        TestRuleBehaviors = resultsCachingTestCase.TestRuleBehaviors,
+                        OutputFilePath = resultsCachingTestCase.PersistLogFileToDisk ? Guid.NewGuid().ToString() : null,
+                        TargetFileSpecifiers = new string[] { Guid.NewGuid().ToString(), Guid.NewGuid().ToString() },
+                        Kind = new List<ResultKind> { ResultKind.Fail },
+                        Level = new List<FailureLevel> { FailureLevel.Warning, FailureLevel.Error },
+                        DataToInsert = new OptionallyEmittedData[] { OptionallyEmittedData.Hashes },
+                    };
+
+                    TestRule.s_testRuleBehaviors = resultsCachingTestCase.TestRuleBehaviors.AccessibleOutsideOfContextOnly();
+                    RunAnalyzeCommand(options,
+                                      resultsCachingTestCase.FileSystem,
+                                      resultsCachingTestCase.ExpectedReturnCode,
+                                      multithreaded: multithreaded);
+                }
+            };
+
+            action.Should().NotThrow();
+        }
+
         private void AnalyzeCommandBase_ShouldGenerateSameResultsWhenRunningSingleAndMultiThread_CoyoteHelper()
         {
             int[] scenarios = SetupScenarios(true);
@@ -1509,7 +1537,7 @@ private static IFileSystem CreateDefaultFileSystemForResultsCaching(IList<string
                 mockFileSystem.Setup(x => x.FileReadAllText(It.Is<string>(f => f == fullyQualifiedName))).Returns(logFileContents);
 
                 mockFileSystem.Setup(x => x.FileOpenRead(It.Is<string>(f => f == fullyQualifiedName)))
-                        .Returns(new MemoryStream(Encoding.UTF8.GetBytes(generateSameInput ? logFileContents : fileNameWithoutExtension)));
+                    .Returns(new NonDisposingDelegatingStream(new MemoryStream(Encoding.UTF8.GetBytes(generateSameInput ? logFileContents : fileNameWithoutExtension))));
             }
             return mockFileSystem.Object;
         }