Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding Rule SARIF2006 #1942

Merged
5 commits merged into from
Jun 30, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/Sarif.Multitool/Rules/RuleId.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ public static class RuleId
public const string AuthorHighQualityMessages = "SARIF2001";
public const string OptimizeFileSize = "SARIF2004";
public const string ProvideHelpfulToolInformation = "SARIF2005";
public const string UrisShouldBeReachable = "SARIF2006";
public const string ProvideSchema = "SARIF2008";
public const string ConsiderConventionalIdentifierValues = "SARIF2009";
}
Expand Down
18 changes: 18 additions & 0 deletions src/Sarif.Multitool/Rules/RuleResources.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions src/Sarif.Multitool/Rules/RuleResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -283,4 +283,10 @@ Many tool use similar names for 'uriBaseId' symbols. We suggest 'REPOROOT' for t
<data name="SARIF2004_OptimizeFileSize_Warning_EliminateLocationOnlyArtifacts_Text" xml:space="preserve">
<value>{0}: Placeholder_SARIF2004_OptimizeFileSize_Warning_EliminateLocationOnlyArtifacts_Text</value>
</data>
<data name="SARIF2006_UrisShouldBeReachable_FullDescription_Text" xml:space="preserve">
<value>Placeholder</value>
</data>
<data name="SARIF2006_UrisShouldBeReachable_Warning_Default_Text" xml:space="preserve">
<value>{0}: Placeholder {1}</value>
</data>
</root>
105 changes: 105 additions & 0 deletions src/Sarif.Multitool/Rules/SARIF2006.UrisShouldBeReachable.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
// 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 System.Net.Http;

using Microsoft.Json.Pointer;

namespace Microsoft.CodeAnalysis.Sarif.Multitool.Rules
{
public class UrisShouldBeReachable : SarifValidationSkimmerBase
{
/// <summary>
/// SARIF2006
/// </summary>
public override string Id => RuleId.UrisShouldBeReachable;

/// <summary>
/// Placeholder
/// </summary>
public override MultiformatMessageString FullDescription => new MultiformatMessageString { Text = RuleResources.SARIF2006_UrisShouldBeReachable_FullDescription_Text };

protected override IEnumerable<string> MessageResourceNames => new string[] {
nameof(RuleResources.SARIF2006_UrisShouldBeReachable_Warning_Default_Text)
};

public override FailureLevel DefaultLevel => FailureLevel.Warning;

private static readonly HttpClient s_httpClient = new HttpClient();
private static readonly Dictionary<string, bool> s_checkedUris = new Dictionary<string, bool>();

protected override void Analyze(SarifLog log, string logPointer)
{
AnalyzeUri(log.SchemaUri, logPointer.AtProperty(SarifPropertyName.Schema));
}

protected override void Analyze(Result result, string resultPointer)
{
if (result.WorkItemUris != null)
{
Uri[] workItemUris = result.WorkItemUris.ToArray();
string workItemUrisPointer = resultPointer.AtProperty(SarifPropertyName.WorkItemUris);

for (int i = 0; i < workItemUris.Length; ++i)
{
AnalyzeUri(workItemUris[i], workItemUrisPointer.AtIndex(i));
}
}
}

protected override void Analyze(ReportingDescriptor reportingDescriptor, string reportingDescriptorPointer)
{
AnalyzeUri(reportingDescriptor.HelpUri, reportingDescriptorPointer.AtProperty(SarifPropertyName.HelpUri));
}

protected override void Analyze(ToolComponent toolComponent, string toolComponentPointer)
{
AnalyzeUri(toolComponent.DownloadUri, toolComponentPointer.AtProperty(SarifPropertyName.DownloadUri));
}

protected override void Analyze(VersionControlDetails versionControlDetails, string versionControlDetailsPointer)
{
AnalyzeUri(versionControlDetails.RepositoryUri, versionControlDetailsPointer.AtProperty(SarifPropertyName.RepositoryUri));
}

private void AnalyzeUri(Uri uri, string pointer)
{
AnalyzeUri(uri?.OriginalString, pointer);
}

private void AnalyzeUri(string uriString, string pointer)
{
// If it's not a well-formed URI of _any_ kind, then don't bother triggering this rule.
// Rule SARIF1003, UrisMustBeValid, will catch it.
// Check for well-formedness first, before attempting to create a Uri object, to
// avoid having to do a try/catch. Unfortunately Uri.TryCreate will return true
// even for a malformed URI string.
if (uriString != null && Uri.IsWellFormedUriString(uriString, UriKind.RelativeOrAbsolute))
Copy link

@ghost ghost Jun 27, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can simplify this logic because if it's not absolute, SARIF1005.UriMustBeAbsolute will catch it. So:

        private void AnalyzeUri(string uriString, string pointer)
        {
            // If it's not a well-formed URI, or if it's not absolute, then don't bother triggering this rule.
            // If it's not well-formed, SARIF1003.UrisMustBeValid will catch it, and if it's not absolute,
            // SARIF1005.UriMustBeAbsolute will catch it.
            //
            // Check for well-formedness first, before attempting to create a Uri object, to
            // avoid having to do a try/catch. Unfortunately Uri.TryCreate will return true
            // even for a malformed URI string.
            if (uriString != null && Uri.IsWellFormedUriString(uriString, UriKind.Absolute))
            {
                // Ok, it's a well-formed absolute URI. If it's not reachable, _now_ we can report it.
                Uri uri = new Uri(uriString, UriKind.Absolute);
                if (!IsUriReachable(uri.AbsoluteUri))
                {
                    // Placeholder
                    LogResult(pointer, nameof(RuleResources.SARIF1005_UriMustBeAbsolute_Error_Default_Text), uriString);
                }
            }
        }
``` #Closed

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If i do that and the uri is not absolute, we will have a null exception. It's why I added. Unless I use the uriString


In reply to: 446547297 [](ancestors = 446547297)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As we discussed: I now use UriKind.Absolute in the call to IsWellFormedUriString so it should work.


In reply to: 446547509 [](ancestors = 446547509,446547297)

{
// Ok, it's a well-formed URI of some kind. If it's not absolute, _now_ we
// can report it.
Uri uri = new Uri(uriString, UriKind.RelativeOrAbsolute);
if (uri.IsAbsoluteUri && !IsUriReachable(uri.AbsoluteUri))
{
// Placeholder
LogResult(pointer, nameof(RuleResources.SARIF1005_UriMustBeAbsolute_Error_Default_Text), uriString);
}
}
}

private bool IsUriReachable(string uriString)
{
if (s_checkedUris.ContainsKey(uriString))
{
return s_checkedUris[uriString];
}

HttpResponseMessage httpResponseMessage = s_httpClient.GetAsync(uriString).GetAwaiter().GetResult();
s_checkedUris.Add(uriString, httpResponseMessage.IsSuccessStatusCode);
return httpResponseMessage.IsSuccessStatusCode;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,14 @@ public void SARIF2005_ProvideToolProperties_Valid()
public void SARIF2005_ProvideToolProperties_Invalid()
=> RunTest(MakeInvalidTestFileName(RuleId.ProvideHelpfulToolInformation, nameof(RuleId.ProvideHelpfulToolInformation)));

[Fact]
public void SARIF2006_UrisShouldBeReachable_Valid()
=> RunTest(MakeValidTestFileName(RuleId.UrisShouldBeReachable, nameof(RuleId.UrisShouldBeReachable)));

[Fact]
public void SARIF2006_UrisShouldBeReachable_Invalid()
=> RunTest(MakeInvalidTestFileName(RuleId.UrisShouldBeReachable, nameof(RuleId.UrisShouldBeReachable)));

[Fact]
public void SARIF2008_ProvideSchema_Valid()
=> RunTest(MakeValidTestFileName(RuleId.ProvideSchema, nameof(RuleId.ProvideSchema)));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,22 @@
}
},
"helpUri": "http://docs.oasis-open.org/sarif/sarif/v2.1.0/sarif-v2.1.0.html"
},
{
"id": "SARIF2006",
"name": "UrisShouldBeReachable",
"shortDescription": {
"text": "Placeholder."
},
"fullDescription": {
"text": "Placeholder"
},
"messageStrings": {
"Warning_Default": {
"text": "{0}: Placeholder {1}"
}
},
"helpUri": "http://docs.oasis-open.org/sarif/sarif/v2.1.0/sarif-v2.1.0.html"
}
]
}
Expand Down Expand Up @@ -358,6 +374,30 @@
}
}
]
},
{
"ruleId": "SARIF2006",
"ruleIndex": 2,
"message": {
"id": "Error_Default",
"arguments": [
"runs[0].results[1].workItemUris[0]",
"http://example.com/my-project/issues/42"
]
},
"locations": [
{
"physicalLocation": {
"artifactLocation": {
"index": 0
},
"region": {
"startLine": 71,
"startColumn": 53
}
}
}
]
}
],
"columnKind": "utf16CodeUnits"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,25 @@
{
"tool": {
"driver": {
"name": "SARIF Functional Testing"
"name": "SARIF Functional Testing",
"rules": [
{
"id": "SARIF2006",
"name": "UrisShouldBeReachable",
"shortDescription": {
"text": "Placeholder."
},
"fullDescription": {
"text": "Placeholder"
},
"messageStrings": {
"Warning_Default": {
"text": "{0}: Placeholder {1}"
}
},
"helpUri": "http://docs.oasis-open.org/sarif/sarif/v2.1.0/sarif-v2.1.0.html"
}
]
}
},
"invocations": [
Expand All @@ -21,7 +39,104 @@
}
}
],
"results": [],
"results": [
{
"ruleId": "SARIF2006",
"ruleIndex": 0,
"message": {
"id": "Error_Default",
"arguments": [
"runs[0].results[0].workItemUris[0]",
"https://example.com/my-project/issues/42"
]
},
"locations": [
{
"physicalLocation": {
"artifactLocation": {
"index": 0
},
"region": {
"startLine": 58,
"startColumn": 54
}
}
}
]
},
{
"ruleId": "SARIF2006",
"ruleIndex": 0,
"message": {
"id": "Error_Default",
"arguments": [
"runs[0].tool.driver.downloadUri",
"http://www.example.com/tools/codescanner/download.html"
]
},
"locations": [
{
"physicalLocation": {
"artifactLocation": {
"index": 0
},
"region": {
"startLine": 10,
"startColumn": 81
}
}
}
]
},
{
"ruleId": "SARIF2006",
"ruleIndex": 0,
"message": {
"id": "Error_Default",
"arguments": [
"runs[0].tool.driver.rules[0].helpUri",
"http://www.example.com/rules/tst0001.html"
]
},
"locations": [
{
"physicalLocation": {
"artifactLocation": {
"index": 0
},
"region": {
"startLine": 14,
"startColumn": 68
}
}
}
]
},
{
"ruleId": "SARIF2006",
"ruleIndex": 0,
"message": {
"id": "Error_Default",
"arguments": [
"runs[0].versionControlProvenance[0].repositoryUri",
"https://example.com/my-project"
]
},
"locations": [
{
"physicalLocation": {
"artifactLocation": {
"index": 0
},
"region": {
"startLine": 21,
"startColumn": 59
}
}
}
]
}
],
"columnKind": "utf16CodeUnits"
}
]
Expand Down
Loading