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

restoring original functionality for sub rule: UriBaseIdRequiresRelat… #1967

Merged
Show file tree
Hide file tree
Changes from all commits
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
4 changes: 2 additions & 2 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.

4 changes: 2 additions & 2 deletions src/Sarif.Multitool/Rules/RuleResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -148,10 +148,10 @@ If the 'invocation' object specifies both 'startTimeUtc' and 'endTimeUtc', then
This is part of a set of authoring practices that make your rule messages more readable, understandable, and actionable. See also `SARIF2014.ProvideDynamicMessageContent` and `SARIF2015.EnquoteDynamicMessageContent`.</value>
</data>
<data name="SARIF1004_ExpressUriBaseIdsCorrectly_Error_UriBaseIdRequiresRelativeUri_Text" xml:space="preserve">
<value>{0}: The '{1}' element of 'originalUriBaseIds' has a 'uriBaseId' property '{2}', but its 'uri' property '{3}' is an absolute URI. Since the purpose of the 'uriBaseId' property is to help resolve a relative reference to an absolute URI, it is not allowed when the 'uri' property is already an absolute URI.</value>
<value>{0}: {1} Placeholder_SARIF1004_ExpressUriBaseIdsCorrectly_Error_UriBaseIdRequiresRelativeUri_Text</value>
</data>
<data name="SARIF1004_ExpressUriBaseIdsCorrectly_FullDescription_Text" xml:space="preserve">
<value>Every URI reference in 'originalUriBaseIds' must resolve to an absolute URI, in the manner described in the SARIF specification [3.14.14](https://docs.oasis-open.org/sarif/sarif/v2.1.0/os/sarif-v2.1.0-os.html#_Toc34317498). This is because the purpose of 'uriBaseIds' is to enable the resolution of relative references to absolute locations.</value>
<value>Placeholder_SARIF1004_ExpressUriBaseIdsCorrectly_FullDescription_Text</value>
</data>
<data name="SARIF1005_UriMustBeAbsolute_Error_Default_Text" xml:space="preserve">
<value>{0}: The value of this property is required to be an absolute URI, but '{1}' is a relative URI reference.</value>
Expand Down
13 changes: 0 additions & 13 deletions src/Sarif.Multitool/Rules/SARIF1002.UrisMustBeValid.cs
Original file line number Diff line number Diff line change
Expand Up @@ -62,19 +62,6 @@ protected override void Analyze(ReportingDescriptor reportingDescriptor, string
AnalyzeUri(reportingDescriptor.HelpUri, messageDescriptorPointer.AtProperty(SarifPropertyName.HelpUri));
}

protected override void Analyze(Run run, string runPointer)
{
if (run.OriginalUriBaseIds != null)
{
string originalUriBaseIdsPointer = runPointer.AtProperty(SarifPropertyName.OriginalUriBaseIds);

foreach (string key in run.OriginalUriBaseIds.Keys)
{
AnalyzeUri(run.OriginalUriBaseIds[key].Uri, originalUriBaseIdsPointer.AtProperty(key).AtProperty(SarifPropertyName.Uri));
}
}
}

protected override void Analyze(ToolComponent toolComponent, string toolComponentPointer)
{
AnalyzeUri(toolComponent.DownloadUri, toolComponentPointer.AtProperty(SarifPropertyName.DownloadUri));
Expand Down
33 changes: 14 additions & 19 deletions src/Sarif.Multitool/Rules/SARIF1004.ExpressUriBaseIdsCorrectly.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,7 @@ public class ExpressUriBaseIdsCorrectly : SarifValidationSkimmerBase
public override string Id => RuleId.ExpressUriBaseIdsCorrectly;

/// <summary>
/// Every URI reference in 'originalUriBaseIds' must resolve to an absolute URI,
/// in the manner described in the SARIF specification [3.14.14]
/// (https://docs.oasis-open.org/sarif/sarif/v2.1.0/os/sarif-v2.1.0-os.html#_Toc34317498).
/// This is because the purpose of 'uriBaseIds' is to enable the resolution of relative
/// references to absolute locations.
/// Placeholder_SARIF1004_ExpressUriBaseIdsCorrectly_FullDescription_Text
/// </summary>
public override MultiformatMessageString FullDescription => new MultiformatMessageString { Text = RuleResources.SARIF1004_ExpressUriBaseIdsCorrectly_FullDescription_Text };

Expand All @@ -35,6 +31,19 @@ public class ExpressUriBaseIdsCorrectly : SarifValidationSkimmerBase

public override FailureLevel DefaultLevel => FailureLevel.Error;

protected override void Analyze(ArtifactLocation fileLocation, string fileLocationPointer)
{
// UriBaseIdRequiresRelativeUri: The 'uri' property of 'fileLocation' must be a relative uri, since 'uriBaseId' is present.
if (fileLocation.UriBaseId != null && fileLocation.Uri.IsAbsoluteUri)
{
//{0}: {1} Placeholder_SARIF1004_ExpressUriBaseIdsCorrectly_Error_UriBaseIdRequiresRelativeUri_Text
LogResult(
fileLocationPointer.AtProperty(SarifPropertyName.Uri),
nameof(RuleResources.SARIF1004_ExpressUriBaseIdsCorrectly_Error_UriBaseIdRequiresRelativeUri_Text),
fileLocation.Uri.OriginalString);
}
}

protected override void Analyze(Run run, string runPointer)
{
if (run.OriginalUriBaseIds != null)
Expand All @@ -52,20 +61,6 @@ private void AnalyzeOriginalUriBaseIdsEntry(string uriBaseId, ArtifactLocation a
{
if (artifactLocation.Uri == null) { return; }

if (artifactLocation.UriBaseId != null && artifactLocation.Uri.IsAbsoluteUri)
{
// {0}: The '{1}' element of 'originalUriBaseIds' has a 'uriBaseId' property '{2}',
// but its 'uri' property '{3}' is an absolute URI. Since the purpose of the 'uriBaseId'
// property is to help resolve a relative reference to an absolute URI, it is not allowed
// when the 'uri' property is already an absolute URI.
LogResult(
pointer,
nameof(RuleResources.SARIF1004_ExpressUriBaseIdsCorrectly_Error_UriBaseIdRequiresRelativeUri_Text),
uriBaseId,
artifactLocation.UriBaseId,
artifactLocation.Uri.OriginalString);
}

// 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
Expand Down
10 changes: 10 additions & 0 deletions src/Sarif.Multitool/Rules/SarifValidationSkimmerBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1028,6 +1028,16 @@ private void Visit(Run run, string runPointer)
Visit(run.WebResponses[i], webResponsesPointer.AtIndex(i));
}
}

if (run.OriginalUriBaseIds != null)
{
string originalUriBaseIdsPointer = runPointer.AtProperty(SarifPropertyName.OriginalUriBaseIds);

foreach (string uriBaseId in run.OriginalUriBaseIds.Keys)
{
Visit(run.OriginalUriBaseIds[uriBaseId], originalUriBaseIdsPointer.AtProperty(uriBaseId));
}
}
}

private void Visit(Stack stack, string stackPointer)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,31 +110,6 @@
}
]
},
{
"ruleId": "SARIF1002",
Copy link
Contributor Author

@harleenkohli harleenkohli Jul 3, 2020

Choose a reason for hiding this comment

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

"ruleId": "SARIF1002", [](start = 10, length = 22)

due to skimmer fix, this node simply moved to the bottom, no change in functionality. #ByDesign

"ruleIndex": 0,
"level": "error",
"message": {
"id": "Error_UrisMustConformToRfc3986",
"arguments": [
"runs[0].originalUriBaseIds.SRCROOT.uri",
"fi%le:///c:/Code/sarif-sdk/src"
]
},
"locations": [
{
"physicalLocation": {
"artifactLocation": {
"index": 0
},
"region": {
"startLine": 36,
"startColumn": 49
}
}
}
]
},
{
"ruleId": "SARIF1002",
"ruleIndex": 0,
Expand Down Expand Up @@ -360,6 +335,31 @@
}
]
},
{
"ruleId": "SARIF1002",
"ruleIndex": 0,
"level": "error",
"message": {
"id": "Error_UrisMustConformToRfc3986",
"arguments": [
"runs[0].originalUriBaseIds.SRCROOT.uri",
"fi%le:///c:/Code/sarif-sdk/src"
]
},
"locations": [
{
"physicalLocation": {
"artifactLocation": {
"index": 0
},
"region": {
"startLine": 36,
"startColumn": 49
}
}
}
]
},
{
"ruleId": "SARIF1011",
"ruleIndex": 1,
Expand Down
Loading