Skip to content

Commit

Permalink
Fix #2003: Don't report missing snippet if file content is present. (#…
Browse files Browse the repository at this point in the history
…2019)

* Checking artifacts before snippet

* code review - 1

* code review - 1

* code review - 2

* Code review - 2

* code review - 3

* code review - 3
  • Loading branch information
eddynaka authored Jul 29, 2020
1 parent 239efeb commit 2ce71e3
Show file tree
Hide file tree
Showing 4 changed files with 175 additions and 12 deletions.
84 changes: 72 additions & 12 deletions src/Sarif.Multitool/Rules/SARIF2010.ProvideCodeSnippets.cs
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -26,6 +27,15 @@ public class ProvideCodeSnippets : SarifValidationSkimmerBase

public override FailureLevel DefaultLevel => FailureLevel.Note;

private IList<Artifact> artifacts;
private IDictionary<string, ArtifactLocation> 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)
Expand All @@ -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)
Expand All @@ -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;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
@@ -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"
}
]
}
Original file line number Diff line number Diff line change
@@ -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"
}
]
}

0 comments on commit 2ce71e3

Please sign in to comment.