From 2ce71e3bf9c8389bd0810d220db572d3f42fdb3f Mon Sep 17 00:00:00 2001 From: Eddy Nakamura Date: Wed, 29 Jul 2020 19:11:51 -0300 Subject: [PATCH] Fix #2003: Don't report missing snippet if file content is present. (#2019) * Checking artifacts before snippet * code review - 1 * code review - 1 * code review - 2 * Code review - 2 * code review - 3 * code review - 3 --- .../Rules/SARIF2010.ProvideCodeSnippets.cs | 84 ++++++++++++++++--- .../Multitool/ValidateCommandTests.cs | 4 + ...videCodeSnippets_WithEmbeddedContent.sarif | 28 +++++++ ...videCodeSnippets_WithEmbeddedContent.sarif | 71 ++++++++++++++++ 4 files changed, 175 insertions(+), 12 deletions(-) create mode 100644 src/Test.FunctionalTests.Sarif/TestData/Multitool/ValidateCommand/ExpectedOutputs/SARIF2010.ProvideCodeSnippets_WithEmbeddedContent.sarif create mode 100644 src/Test.FunctionalTests.Sarif/TestData/Multitool/ValidateCommand/Inputs/SARIF2010.ProvideCodeSnippets_WithEmbeddedContent.sarif diff --git a/src/Sarif.Multitool/Rules/SARIF2010.ProvideCodeSnippets.cs b/src/Sarif.Multitool/Rules/SARIF2010.ProvideCodeSnippets.cs index 0871df0ea..7d3e58096 100644 --- a/src/Sarif.Multitool/Rules/SARIF2010.ProvideCodeSnippets.cs +++ b/src/Sarif.Multitool/Rules/SARIF2010.ProvideCodeSnippets.cs @@ -1,8 +1,9 @@ // Copyright (c) Microsoft. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. +using System; using System.Collections.Generic; - +using System.Linq; using Microsoft.Json.Pointer; namespace Microsoft.CodeAnalysis.Sarif.Multitool.Rules @@ -26,6 +27,15 @@ public class ProvideCodeSnippets : SarifValidationSkimmerBase public override FailureLevel DefaultLevel => FailureLevel.Note; + private IList artifacts; + private IDictionary originalUriBaseIds; + + protected override void Analyze(Run run, string runPointer) + { + this.artifacts = run.Artifacts; + this.originalUriBaseIds = run.OriginalUriBaseIds; + } + protected override void Analyze(Result result, string resultPointer) { if (result.Locations != null) @@ -40,17 +50,20 @@ protected override void Analyze(Result result, string resultPointer) private void AnalyzeResultLocation(Location location, string locationPointer) { - AnalyzeRegion( - location.PhysicalLocation?.Region, - locationPointer - .AtProperty(SarifPropertyName.PhysicalLocation) - .AtProperty(SarifPropertyName.Region)); - - AnalyzeRegion( - location.PhysicalLocation?.ContextRegion, - locationPointer - .AtProperty(SarifPropertyName.PhysicalLocation) - .AtProperty(SarifPropertyName.ContextRegion)); + if (AnalyzeArtifactLocation(location.PhysicalLocation?.ArtifactLocation)) + { + AnalyzeRegion( + location.PhysicalLocation?.Region, + locationPointer + .AtProperty(SarifPropertyName.PhysicalLocation) + .AtProperty(SarifPropertyName.Region)); + + AnalyzeRegion( + location.PhysicalLocation?.ContextRegion, + locationPointer + .AtProperty(SarifPropertyName.PhysicalLocation) + .AtProperty(SarifPropertyName.ContextRegion)); + } } private void AnalyzeRegion(Region region, string regionPointer) @@ -65,5 +78,52 @@ private void AnalyzeRegion(Region region, string regionPointer) nameof(RuleResources.SARIF2010_ProvideCodeSnippets_Note_Default_Text)); } } + + private bool AnalyzeArtifactLocation(ArtifactLocation artifactLocation) + { + // No artifactLocation / no artifacts, so we should look for the snippet. + if (artifactLocation == null || this.artifacts == null) + { + return true; + } + + // Checking if we can reconstruct uri from artifactLocation + // If we can't, we still need to validate, since neither originalUriBaseIds + // nor artifactLocation.UriBaseId is required. + artifactLocation.TryReconstructAbsoluteUri(this.originalUriBaseIds, out Uri resolvedUri); + + foreach (Artifact artifact in this.artifacts) + { + // Content/text doesn't exist, continue to next + if (string.IsNullOrEmpty(artifact.Contents?.Text)) + { + continue; + } + + // Checking if we can reconstruct uri from artifact + // If we can't, we still need to validate, since originalUriBaseIds aren't + // required nether artifactLocation.UriBaseId. + artifact.Location.TryReconstructAbsoluteUri(this.originalUriBaseIds, out Uri artifactUri); + + if (resolvedUri != null && artifactUri != null) + { + if (artifactUri.AbsolutePath.Equals(resolvedUri.AbsolutePath)) + { + return false; + } + } + else + { + // Couldn't generate the absoluteUris, so let's compare everything. + if (this.artifacts.Any(a => a.Location?.Uri.OriginalString == artifactLocation.Uri.OriginalString + && a.Location?.UriBaseId == artifactLocation.UriBaseId)) + { + return false; + } + } + } + + return true; + } } } diff --git a/src/Test.FunctionalTests.Sarif/Multitool/ValidateCommandTests.cs b/src/Test.FunctionalTests.Sarif/Multitool/ValidateCommandTests.cs index 114c86ef3..e87823649 100644 --- a/src/Test.FunctionalTests.Sarif/Multitool/ValidateCommandTests.cs +++ b/src/Test.FunctionalTests.Sarif/Multitool/ValidateCommandTests.cs @@ -211,6 +211,10 @@ public void SARIF2009_ConsiderConventionalIdentifierValues_Invalid() public void SARIF2010_ProvideCodeSnippets_Valid() => RunValidTestForRule(RuleId.ProvideCodeSnippets); + [Fact] + public void SARIF2010_ProvideCodeSnippets_WithEmbeddedContent_Valid() + => RunTest("SARIF2010.ProvideCodeSnippets_WithEmbeddedContent.sarif"); + [Fact] public void SARIF2010_ProvideCodeSnippets_Invalid() => RunInvalidTestForRule(RuleId.ProvideCodeSnippets); diff --git a/src/Test.FunctionalTests.Sarif/TestData/Multitool/ValidateCommand/ExpectedOutputs/SARIF2010.ProvideCodeSnippets_WithEmbeddedContent.sarif b/src/Test.FunctionalTests.Sarif/TestData/Multitool/ValidateCommand/ExpectedOutputs/SARIF2010.ProvideCodeSnippets_WithEmbeddedContent.sarif new file mode 100644 index 000000000..74340eeba --- /dev/null +++ b/src/Test.FunctionalTests.Sarif/TestData/Multitool/ValidateCommand/ExpectedOutputs/SARIF2010.ProvideCodeSnippets_WithEmbeddedContent.sarif @@ -0,0 +1,28 @@ +{ + "$schema": "https://schemastore.azurewebsites.net/schemas/json/sarif-2.1.0-rtm.5.json", + "version": "2.1.0", + "runs": [ + { + "tool": { + "driver": { + "name": "SARIF Functional Testing" + } + }, + "invocations": [ + { + "executionSuccessful": true + } + ], + "artifacts": [ + { + "location": { + "uri": "FunctionalTestOutput.ValidateCommand/Inputs.SARIF2010.ProvideCodeSnippets_WithArtifacts_Valid.sarif", + "uriBaseId": "TEST_DIR" + } + } + ], + "results": [], + "columnKind": "utf16CodeUnits" + } + ] +} \ No newline at end of file diff --git a/src/Test.FunctionalTests.Sarif/TestData/Multitool/ValidateCommand/Inputs/SARIF2010.ProvideCodeSnippets_WithEmbeddedContent.sarif b/src/Test.FunctionalTests.Sarif/TestData/Multitool/ValidateCommand/Inputs/SARIF2010.ProvideCodeSnippets_WithEmbeddedContent.sarif new file mode 100644 index 000000000..269a0923d --- /dev/null +++ b/src/Test.FunctionalTests.Sarif/TestData/Multitool/ValidateCommand/Inputs/SARIF2010.ProvideCodeSnippets_WithEmbeddedContent.sarif @@ -0,0 +1,71 @@ +{ + "$schema": "https://schemastore.azurewebsites.net/schemas/json/sarif-2.1.0-rtm.5.json", + "version": "2.1.0", + "runs": [ + { + "tool": { + "driver": { + "name": "CodeScanner", + "version": "1.0" + } + }, + "originalUriBaseIds": { + "TEST_ROOT": { + "uri": "file://c:/test/" + } + }, + "artifacts": [ + { + "contents": { + "text": "File Source" + }, + "location": { + "uri": "src/test.c" + } + }, + { + "contents": { + "text": "File Source" + }, + "location": { + "uri": "test2.c", + "uriBaseId": "TEST_ROOT" + } + } + ], + "results": [ + { + "ruleId": "TST0001", + "level": "error", + "message": { + "text": "Some testing occurred." + }, + "locations": [ + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/test.c" + }, + "properties": { + "comment": "This test will NOT generate a valid AbsoluteUri, but it will find in artifacts. So it doesn't need a snippet." + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "test2.c", + "uriBaseId": "TEST_ROOT" + }, + "properties": { + "comment": "This test will generate a valid AbsoluteUri and it will find in artifacts. So it doesn't need a snippet." + } + } + } + ] + } + ], + "columnKind": "utf16CodeUnits" + } + ] +} \ No newline at end of file