From 60bb4146ae20df5b7fa19c2b7c3b25b5e3822948 Mon Sep 17 00:00:00 2001 From: marmegh <30842915+marmegh@users.noreply.github.com> Date: Wed, 6 Jul 2022 09:34:54 -0700 Subject: [PATCH 1/7] Replace Uri.IsWellFormedUri with SarifValidationSkimmerBase.IsWellFormedUri, plus added example. --- src/ReleaseHistory.md | 4 ++++ .../Rules/SARIF1002.UrisMustBeValid.cs | 2 +- .../Rules/SARIF1004.ExpressUriBaseIdsCorrectly.cs | 2 +- .../Rules/SARIF1005.UriMustBeAbsolute.cs | 2 +- .../Rules/SARIF2006.UrisShouldBeReachable.cs | 2 +- .../Rules/SarifValidationSkimmerBase.cs | 12 ++++++++++++ .../Inputs/SARIF1002.UrisMustBeValid_Valid.sarif | 6 ++++++ 7 files changed, 26 insertions(+), 4 deletions(-) diff --git a/src/ReleaseHistory.md b/src/ReleaseHistory.md index d02a470f8..1d59c106c 100644 --- a/src/ReleaseHistory.md +++ b/src/ReleaseHistory.md @@ -1,5 +1,9 @@ # SARIF Package Release History (SDK, Driver, Converters, and Multitool) +## Unreleased + +* BUGFIX: Fix false positive for `SARIF1002.UrisMustBeValid` for URIs with the format `file:/c:/location/filename.txt`. [#]() + ## **v2.4.15** [Sdk](https://www.nuget.org/packages/Sarif.Sdk/2.4.15) | [Driver](https://www.nuget.org/packages/Sarif.Driver/2.4.15) | [Converters](https://www.nuget.org/packages/Sarif.Converters/2.4.15) | [Multitool](https://www.nuget.org/packages/Sarif.Multitool/2.4.15) | [Multitool Library](https://www.nuget.org/packages/Sarif.Multitool.Library/2.4.15) * BUGFIX: Fix `ArgumentNullException` when `PropertiesDictionary` is instantiated with a null comparer. [#2482](https://github.com/microsoft/sarif-sdk/pull/2482) diff --git a/src/Sarif.Multitool.Library/Rules/SARIF1002.UrisMustBeValid.cs b/src/Sarif.Multitool.Library/Rules/SARIF1002.UrisMustBeValid.cs index 52f8f2cf4..c76d5acbc 100644 --- a/src/Sarif.Multitool.Library/Rules/SARIF1002.UrisMustBeValid.cs +++ b/src/Sarif.Multitool.Library/Rules/SARIF1002.UrisMustBeValid.cs @@ -80,7 +80,7 @@ private void AnalyzeUri(Uri uri, string pointer) string uriString = uri?.OriginalString; if (uriString != null) { - if (!Uri.IsWellFormedUriString(uriString, UriKind.RelativeOrAbsolute)) + if (!IsWellFormedUri(uriString, UriKind.RelativeOrAbsolute)) { // {0}: The string '{1}' is not a valid URI reference. URIs must conform to // [RFC 3986](https://tools.ietf.org/html/rfc3986). diff --git a/src/Sarif.Multitool.Library/Rules/SARIF1004.ExpressUriBaseIdsCorrectly.cs b/src/Sarif.Multitool.Library/Rules/SARIF1004.ExpressUriBaseIdsCorrectly.cs index 26da20b12..2d917212a 100644 --- a/src/Sarif.Multitool.Library/Rules/SARIF1004.ExpressUriBaseIdsCorrectly.cs +++ b/src/Sarif.Multitool.Library/Rules/SARIF1004.ExpressUriBaseIdsCorrectly.cs @@ -100,7 +100,7 @@ private void AnalyzeOriginalUriBaseIdsEntry(string uriBaseId, ArtifactLocation a // avoid having to do a try/catch. Unfortunately Uri.TryCreate will return true // even for a malformed URI string. string uriString = artifactLocation.Uri.OriginalString; - if (uriString != null && Uri.IsWellFormedUriString(uriString, UriKind.RelativeOrAbsolute)) + if (uriString != null && IsWellFormedUri(uriString, UriKind.RelativeOrAbsolute)) { var uri = new Uri(uriString, UriKind.RelativeOrAbsolute); diff --git a/src/Sarif.Multitool.Library/Rules/SARIF1005.UriMustBeAbsolute.cs b/src/Sarif.Multitool.Library/Rules/SARIF1005.UriMustBeAbsolute.cs index b43b3cf54..89b386490 100644 --- a/src/Sarif.Multitool.Library/Rules/SARIF1005.UriMustBeAbsolute.cs +++ b/src/Sarif.Multitool.Library/Rules/SARIF1005.UriMustBeAbsolute.cs @@ -77,7 +77,7 @@ private void AnalyzeUri(string uriString, string pointer) // 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)) + if (uriString != null && IsWellFormedUri(uriString, UriKind.RelativeOrAbsolute)) { // Ok, it's a well-formed URI of some kind. If it's not absolute, _now_ we // can report it. diff --git a/src/Sarif.Multitool.Library/Rules/SARIF2006.UrisShouldBeReachable.cs b/src/Sarif.Multitool.Library/Rules/SARIF2006.UrisShouldBeReachable.cs index 5f9ad2696..402aabba0 100644 --- a/src/Sarif.Multitool.Library/Rules/SARIF2006.UrisShouldBeReachable.cs +++ b/src/Sarif.Multitool.Library/Rules/SARIF2006.UrisShouldBeReachable.cs @@ -87,7 +87,7 @@ private void AnalyzeUri(string uriString, string pointer) // 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)) + if (uriString != null && IsWellFormedUri(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); diff --git a/src/Sarif.Multitool.Library/Rules/SarifValidationSkimmerBase.cs b/src/Sarif.Multitool.Library/Rules/SarifValidationSkimmerBase.cs index 4da930a22..1d6689945 100644 --- a/src/Sarif.Multitool.Library/Rules/SarifValidationSkimmerBase.cs +++ b/src/Sarif.Multitool.Library/Rules/SarifValidationSkimmerBase.cs @@ -246,6 +246,18 @@ internal static string JsonPointerToJavaScript(string pointerString) return sb.ToString(); } + internal static bool IsWellFormedUri(string uriString, UriKind uriKind) + { + bool isWellFormed = Uri.IsWellFormedUriString(uriString, UriKind.RelativeOrAbsolute); + bool csBug = (uriString.StartsWith("file:/") && Uri.TryCreate(uriString, uriKind, out Uri result)); + + return isWellFormed || csBug; + } + //=> + //Uri.IsWellFormedUriString(uriString, UriKind.RelativeOrAbsolute) || + // (uriString.StartsWith("file:/") && + // Uri.TryCreate(uriString, uriKind, out Uri result)); + private static readonly string s_javaScriptIdentifierPattern = @"^[$_\p{L}][$_\p{L}0-9]*$"; private static readonly Regex s_javaScriptIdentifierRegex = new Regex(s_javaScriptIdentifierPattern, RegexOptions.Compiled); diff --git a/src/Test.FunctionalTests.Sarif/TestData/Multitool/ValidateCommand/Inputs/SARIF1002.UrisMustBeValid_Valid.sarif b/src/Test.FunctionalTests.Sarif/TestData/Multitool/ValidateCommand/Inputs/SARIF1002.UrisMustBeValid_Valid.sarif index 301df35e2..14b9bb0fb 100644 --- a/src/Test.FunctionalTests.Sarif/TestData/Multitool/ValidateCommand/Inputs/SARIF1002.UrisMustBeValid_Valid.sarif +++ b/src/Test.FunctionalTests.Sarif/TestData/Multitool/ValidateCommand/Inputs/SARIF1002.UrisMustBeValid_Valid.sarif @@ -40,6 +40,12 @@ "uriBaseId": "SRCROOT", "index": 1 } + }, + { + "location": { + "uri": "file:/c:/src/file.c", + "index": 2 + } } ], "results": [ From 4bb946177574b6ed3d79d1d015fc4d2214db28d5 Mon Sep 17 00:00:00 2001 From: marmegh <30842915+marmegh@users.noreply.github.com> Date: Mon, 11 Jul 2022 10:04:37 -0700 Subject: [PATCH 2/7] Updated method name to standard --- src/Sarif.Multitool.Library/Rules/SARIF1002.UrisMustBeValid.cs | 2 +- .../Rules/SARIF1004.ExpressUriBaseIdsCorrectly.cs | 2 +- .../Rules/SARIF1005.UriMustBeAbsolute.cs | 2 +- .../Rules/SARIF2006.UrisShouldBeReachable.cs | 2 +- src/Sarif.Multitool.Library/Rules/SarifValidationSkimmerBase.cs | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Sarif.Multitool.Library/Rules/SARIF1002.UrisMustBeValid.cs b/src/Sarif.Multitool.Library/Rules/SARIF1002.UrisMustBeValid.cs index c76d5acbc..ef5400b5f 100644 --- a/src/Sarif.Multitool.Library/Rules/SARIF1002.UrisMustBeValid.cs +++ b/src/Sarif.Multitool.Library/Rules/SARIF1002.UrisMustBeValid.cs @@ -80,7 +80,7 @@ private void AnalyzeUri(Uri uri, string pointer) string uriString = uri?.OriginalString; if (uriString != null) { - if (!IsWellFormedUri(uriString, UriKind.RelativeOrAbsolute)) + if (!UriIsWellFormedUriString(uriString, UriKind.RelativeOrAbsolute)) { // {0}: The string '{1}' is not a valid URI reference. URIs must conform to // [RFC 3986](https://tools.ietf.org/html/rfc3986). diff --git a/src/Sarif.Multitool.Library/Rules/SARIF1004.ExpressUriBaseIdsCorrectly.cs b/src/Sarif.Multitool.Library/Rules/SARIF1004.ExpressUriBaseIdsCorrectly.cs index 2d917212a..aa409b4da 100644 --- a/src/Sarif.Multitool.Library/Rules/SARIF1004.ExpressUriBaseIdsCorrectly.cs +++ b/src/Sarif.Multitool.Library/Rules/SARIF1004.ExpressUriBaseIdsCorrectly.cs @@ -100,7 +100,7 @@ private void AnalyzeOriginalUriBaseIdsEntry(string uriBaseId, ArtifactLocation a // avoid having to do a try/catch. Unfortunately Uri.TryCreate will return true // even for a malformed URI string. string uriString = artifactLocation.Uri.OriginalString; - if (uriString != null && IsWellFormedUri(uriString, UriKind.RelativeOrAbsolute)) + if (uriString != null && UriIsWellFormedUriString(uriString, UriKind.RelativeOrAbsolute)) { var uri = new Uri(uriString, UriKind.RelativeOrAbsolute); diff --git a/src/Sarif.Multitool.Library/Rules/SARIF1005.UriMustBeAbsolute.cs b/src/Sarif.Multitool.Library/Rules/SARIF1005.UriMustBeAbsolute.cs index 89b386490..67fcc9c2d 100644 --- a/src/Sarif.Multitool.Library/Rules/SARIF1005.UriMustBeAbsolute.cs +++ b/src/Sarif.Multitool.Library/Rules/SARIF1005.UriMustBeAbsolute.cs @@ -77,7 +77,7 @@ private void AnalyzeUri(string uriString, string pointer) // 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 && IsWellFormedUri(uriString, UriKind.RelativeOrAbsolute)) + if (uriString != null && UriIsWellFormedUriString(uriString, UriKind.RelativeOrAbsolute)) { // Ok, it's a well-formed URI of some kind. If it's not absolute, _now_ we // can report it. diff --git a/src/Sarif.Multitool.Library/Rules/SARIF2006.UrisShouldBeReachable.cs b/src/Sarif.Multitool.Library/Rules/SARIF2006.UrisShouldBeReachable.cs index 402aabba0..0a17d5a2d 100644 --- a/src/Sarif.Multitool.Library/Rules/SARIF2006.UrisShouldBeReachable.cs +++ b/src/Sarif.Multitool.Library/Rules/SARIF2006.UrisShouldBeReachable.cs @@ -87,7 +87,7 @@ private void AnalyzeUri(string uriString, string pointer) // 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 && IsWellFormedUri(uriString, UriKind.Absolute)) + if (uriString != null && UriIsWellFormedUriString(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); diff --git a/src/Sarif.Multitool.Library/Rules/SarifValidationSkimmerBase.cs b/src/Sarif.Multitool.Library/Rules/SarifValidationSkimmerBase.cs index 1d6689945..e4eca398b 100644 --- a/src/Sarif.Multitool.Library/Rules/SarifValidationSkimmerBase.cs +++ b/src/Sarif.Multitool.Library/Rules/SarifValidationSkimmerBase.cs @@ -246,7 +246,7 @@ internal static string JsonPointerToJavaScript(string pointerString) return sb.ToString(); } - internal static bool IsWellFormedUri(string uriString, UriKind uriKind) + internal static bool UriIsWellFormedUriString(string uriString, UriKind uriKind) { bool isWellFormed = Uri.IsWellFormedUriString(uriString, UriKind.RelativeOrAbsolute); bool csBug = (uriString.StartsWith("file:/") && Uri.TryCreate(uriString, uriKind, out Uri result)); From ebc734c7e3f95c86a71340c6645565331247003f Mon Sep 17 00:00:00 2001 From: marmegh <30842915+marmegh@users.noreply.github.com> Date: Mon, 11 Jul 2022 11:13:23 -0700 Subject: [PATCH 3/7] Fixed unhandled exception. --- .../Rules/SarifValidationSkimmerBase.cs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/Sarif.Multitool.Library/Rules/SarifValidationSkimmerBase.cs b/src/Sarif.Multitool.Library/Rules/SarifValidationSkimmerBase.cs index e4eca398b..0c6bfdd0c 100644 --- a/src/Sarif.Multitool.Library/Rules/SarifValidationSkimmerBase.cs +++ b/src/Sarif.Multitool.Library/Rules/SarifValidationSkimmerBase.cs @@ -248,15 +248,11 @@ internal static string JsonPointerToJavaScript(string pointerString) internal static bool UriIsWellFormedUriString(string uriString, UriKind uriKind) { - bool isWellFormed = Uri.IsWellFormedUriString(uriString, UriKind.RelativeOrAbsolute); + bool isWellFormed = Uri.IsWellFormedUriString(uriString, uriKind); bool csBug = (uriString.StartsWith("file:/") && Uri.TryCreate(uriString, uriKind, out Uri result)); return isWellFormed || csBug; } - //=> - //Uri.IsWellFormedUriString(uriString, UriKind.RelativeOrAbsolute) || - // (uriString.StartsWith("file:/") && - // Uri.TryCreate(uriString, uriKind, out Uri result)); private static readonly string s_javaScriptIdentifierPattern = @"^[$_\p{L}][$_\p{L}0-9]*$"; private static readonly Regex s_javaScriptIdentifierRegex = new Regex(s_javaScriptIdentifierPattern, RegexOptions.Compiled); From f288136e7e891d09676f476a3db3fa88c7558bbb Mon Sep 17 00:00:00 2001 From: marmegh <30842915+marmegh@users.noreply.github.com> Date: Mon, 11 Jul 2022 12:13:04 -0700 Subject: [PATCH 4/7] Added unit test for UriIsWellFormedUriString --- .../Rules/SarifValidationSkimmerBaseTests.cs | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/src/Test.UnitTests.Sarif.Multitool.Library/Rules/SarifValidationSkimmerBaseTests.cs b/src/Test.UnitTests.Sarif.Multitool.Library/Rules/SarifValidationSkimmerBaseTests.cs index 448e8dd71..311e68276 100644 --- a/src/Test.UnitTests.Sarif.Multitool.Library/Rules/SarifValidationSkimmerBaseTests.cs +++ b/src/Test.UnitTests.Sarif.Multitool.Library/Rules/SarifValidationSkimmerBaseTests.cs @@ -1,6 +1,7 @@ // 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.Text; using FluentAssertions; @@ -76,5 +77,66 @@ public void SarifValidationSkimmerBase_JsonPointerToJavaScript_ProducesExpectedR string errorMessage = sb.ToString(); errorMessage.Should().BeEmpty(); } + + [Fact] + public void SarifValidationSkimmerBase_UriIsWellFormedUriString_ProducesExpectedResults() + { + var testCases = new[] + { + new + { + expectedIsWellFormedUri = true, + uriKind = UriKind.Absolute, + uriString = "file:///c:/Code/sarif-sdk/src/" + }, + new + { + expectedIsWellFormedUri = true, + uriKind = UriKind.Absolute, + uriString = "https://example.com/my-project" + }, + new + { + expectedIsWellFormedUri = true, + uriKind = UriKind.Absolute, + uriString = "file:/c:/src/file.c" + }, + new + { + expectedIsWellFormedUri = false, + uriKind = UriKind.Absolute, + uriString = "ht%tp://www.example.com/rules/tst0001.html" + }, + new + { + expectedIsWellFormedUri = true, + uriKind = UriKind.RelativeOrAbsolute, + uriString = "file:///c:/Code/sarif-sdk/src/" + }, + new + { + expectedIsWellFormedUri = true, + uriKind = UriKind.RelativeOrAbsolute, + uriString = "https://example.com/my-project" + }, + new + { + expectedIsWellFormedUri = true, + uriKind = UriKind.RelativeOrAbsolute, + uriString = "file:/c:/src/file.c" + }, + new + { + expectedIsWellFormedUri = false, + uriKind = UriKind.RelativeOrAbsolute, + uriString = "ht%tp://www.example.com/rules/tst0001.html" + }, + }; + + foreach(var testCase in testCases) + { + SarifValidationSkimmerBase.UriIsWellFormedUriString(testCase.uriString, testCase.uriKind).Should().Be(testCase.expectedIsWellFormedUri); + } + } } } From 04fe163c909ded6f1ccb00b3c5693d90372d1060 Mon Sep 17 00:00:00 2001 From: marmegh <30842915+marmegh@users.noreply.github.com> Date: Tue, 12 Jul 2022 08:23:40 -0700 Subject: [PATCH 5/7] Applied PR feedback. --- src/ReleaseHistory.md | 2 +- .../Rules/SARIF1002.UrisMustBeValid.cs | 2 +- .../SARIF1004.ExpressUriBaseIdsCorrectly.cs | 2 +- .../Rules/SARIF1005.UriMustBeAbsolute.cs | 2 +- .../Rules/SARIF2006.UrisShouldBeReachable.cs | 2 +- .../Rules/SarifValidationSkimmerBase.cs | 14 ++++-- .../Rules/SARIF1002.UrisMustBeValidTests.cs | 23 ++++++++++ .../Rules/SarifValidationSkimmerBaseTests.cs | 44 ++++++++++++++----- 8 files changed, 71 insertions(+), 20 deletions(-) create mode 100644 src/Test.UnitTests.Sarif.Multitool.Library/Rules/SARIF1002.UrisMustBeValidTests.cs diff --git a/src/ReleaseHistory.md b/src/ReleaseHistory.md index 760ad1a00..18b0aec73 100644 --- a/src/ReleaseHistory.md +++ b/src/ReleaseHistory.md @@ -3,7 +3,7 @@ ## Unreleased * FEATURE: Add `max-file-size-in-kb` argument that allows filtering scan targets by file size. [#2494](https://github.com/microsoft/sarif-sdk/pull/2494) -* BUGFIX: Fix false positive for `SARIF1002.UrisMustBeValid` for URIs with the format `file:/c:/location/filename.txt`. [#]() +* BUGFIX: Fix false positive for `SARIF1002.UrisMustBeValid` for file URIs that omit the `authority`. [#2501](https://github.com/microsoft/sarif-sdk/pull/2501) ## **v2.4.15** [Sdk](https://www.nuget.org/packages/Sarif.Sdk/2.4.15) | [Driver](https://www.nuget.org/packages/Sarif.Driver/2.4.15) | [Converters](https://www.nuget.org/packages/Sarif.Converters/2.4.15) | [Multitool](https://www.nuget.org/packages/Sarif.Multitool/2.4.15) | [Multitool Library](https://www.nuget.org/packages/Sarif.Multitool.Library/2.4.15) diff --git a/src/Sarif.Multitool.Library/Rules/SARIF1002.UrisMustBeValid.cs b/src/Sarif.Multitool.Library/Rules/SARIF1002.UrisMustBeValid.cs index ef5400b5f..7848c5a88 100644 --- a/src/Sarif.Multitool.Library/Rules/SARIF1002.UrisMustBeValid.cs +++ b/src/Sarif.Multitool.Library/Rules/SARIF1002.UrisMustBeValid.cs @@ -80,7 +80,7 @@ private void AnalyzeUri(Uri uri, string pointer) string uriString = uri?.OriginalString; if (uriString != null) { - if (!UriIsWellFormedUriString(uriString, UriKind.RelativeOrAbsolute)) + if (!IsWellFormedUriString(uriString, UriKind.RelativeOrAbsolute)) { // {0}: The string '{1}' is not a valid URI reference. URIs must conform to // [RFC 3986](https://tools.ietf.org/html/rfc3986). diff --git a/src/Sarif.Multitool.Library/Rules/SARIF1004.ExpressUriBaseIdsCorrectly.cs b/src/Sarif.Multitool.Library/Rules/SARIF1004.ExpressUriBaseIdsCorrectly.cs index aa409b4da..3a92d3904 100644 --- a/src/Sarif.Multitool.Library/Rules/SARIF1004.ExpressUriBaseIdsCorrectly.cs +++ b/src/Sarif.Multitool.Library/Rules/SARIF1004.ExpressUriBaseIdsCorrectly.cs @@ -100,7 +100,7 @@ private void AnalyzeOriginalUriBaseIdsEntry(string uriBaseId, ArtifactLocation a // avoid having to do a try/catch. Unfortunately Uri.TryCreate will return true // even for a malformed URI string. string uriString = artifactLocation.Uri.OriginalString; - if (uriString != null && UriIsWellFormedUriString(uriString, UriKind.RelativeOrAbsolute)) + if (uriString != null && IsWellFormedUriString(uriString, UriKind.RelativeOrAbsolute)) { var uri = new Uri(uriString, UriKind.RelativeOrAbsolute); diff --git a/src/Sarif.Multitool.Library/Rules/SARIF1005.UriMustBeAbsolute.cs b/src/Sarif.Multitool.Library/Rules/SARIF1005.UriMustBeAbsolute.cs index 67fcc9c2d..541b1b1ba 100644 --- a/src/Sarif.Multitool.Library/Rules/SARIF1005.UriMustBeAbsolute.cs +++ b/src/Sarif.Multitool.Library/Rules/SARIF1005.UriMustBeAbsolute.cs @@ -77,7 +77,7 @@ private void AnalyzeUri(string uriString, string pointer) // 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 && UriIsWellFormedUriString(uriString, UriKind.RelativeOrAbsolute)) + if (uriString != null && IsWellFormedUriString(uriString, UriKind.RelativeOrAbsolute)) { // Ok, it's a well-formed URI of some kind. If it's not absolute, _now_ we // can report it. diff --git a/src/Sarif.Multitool.Library/Rules/SARIF2006.UrisShouldBeReachable.cs b/src/Sarif.Multitool.Library/Rules/SARIF2006.UrisShouldBeReachable.cs index 0a17d5a2d..1bda2bd03 100644 --- a/src/Sarif.Multitool.Library/Rules/SARIF2006.UrisShouldBeReachable.cs +++ b/src/Sarif.Multitool.Library/Rules/SARIF2006.UrisShouldBeReachable.cs @@ -87,7 +87,7 @@ private void AnalyzeUri(string uriString, string pointer) // 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 && UriIsWellFormedUriString(uriString, UriKind.Absolute)) + if (uriString != null && 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); diff --git a/src/Sarif.Multitool.Library/Rules/SarifValidationSkimmerBase.cs b/src/Sarif.Multitool.Library/Rules/SarifValidationSkimmerBase.cs index 0c6bfdd0c..485fea970 100644 --- a/src/Sarif.Multitool.Library/Rules/SarifValidationSkimmerBase.cs +++ b/src/Sarif.Multitool.Library/Rules/SarifValidationSkimmerBase.cs @@ -246,12 +246,20 @@ internal static string JsonPointerToJavaScript(string pointerString) return sb.ToString(); } - internal static bool UriIsWellFormedUriString(string uriString, UriKind uriKind) + /// + /// Validate URIs conform to [RFC 3986](https://tools.ietf.org/html/rfc3986). + /// + /// The string used to attempt to construct a `Uri`. + /// The type of the `Uri` in `uriString`. + /// + internal static bool IsWellFormedUriString(string uriString, UriKind uriKind) { bool isWellFormed = Uri.IsWellFormedUriString(uriString, uriKind); - bool csBug = (uriString.StartsWith("file:/") && Uri.TryCreate(uriString, uriKind, out Uri result)); - return isWellFormed || csBug; + // `System.Uri.IsWellFormedUriString` incorrectly returns false if the `authority` is missing from the Uri string. + bool subjectToWellKnownClrBug = (uriString.StartsWith("file:/", StringComparison.OrdinalIgnoreCase) && Uri.TryCreate(uriString, uriKind, out Uri result)); + + return isWellFormed || subjectToWellKnownClrBug; } private static readonly string s_javaScriptIdentifierPattern = @"^[$_\p{L}][$_\p{L}0-9]*$"; diff --git a/src/Test.UnitTests.Sarif.Multitool.Library/Rules/SARIF1002.UrisMustBeValidTests.cs b/src/Test.UnitTests.Sarif.Multitool.Library/Rules/SARIF1002.UrisMustBeValidTests.cs new file mode 100644 index 000000000..c7bee48a3 --- /dev/null +++ b/src/Test.UnitTests.Sarif.Multitool.Library/Rules/SARIF1002.UrisMustBeValidTests.cs @@ -0,0 +1,23 @@ +// Copyright (c) Microsoft. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System.Text; + +using FluentAssertions; + +using Microsoft.CodeAnalysis.Sarif.Multitool.Rules; + +using Xunit; + +namespace Microsoft.CodeAnalysis.Sarif.Multitool +{ + public class UrisMustBeValidTests + { + [Fact] + public void UrisMustBeValid_ShouldProduceExpectedResults() + { + var testSkimmer = new UrisMustBeValid(); + + } + } +} diff --git a/src/Test.UnitTests.Sarif.Multitool.Library/Rules/SarifValidationSkimmerBaseTests.cs b/src/Test.UnitTests.Sarif.Multitool.Library/Rules/SarifValidationSkimmerBaseTests.cs index 311e68276..aa22aa15a 100644 --- a/src/Test.UnitTests.Sarif.Multitool.Library/Rules/SarifValidationSkimmerBaseTests.cs +++ b/src/Test.UnitTests.Sarif.Multitool.Library/Rules/SarifValidationSkimmerBaseTests.cs @@ -79,63 +79,83 @@ public void SarifValidationSkimmerBase_JsonPointerToJavaScript_ProducesExpectedR } [Fact] - public void SarifValidationSkimmerBase_UriIsWellFormedUriString_ProducesExpectedResults() + public void SarifValidationSkimmerBase_UriIsWellFormedUriString_ShouldReturnTrueIfWellFormedUri() { var testCases = new[] { new { - expectedIsWellFormedUri = true, uriKind = UriKind.Absolute, uriString = "file:///c:/Code/sarif-sdk/src/" }, new { - expectedIsWellFormedUri = true, uriKind = UriKind.Absolute, uriString = "https://example.com/my-project" }, new { - expectedIsWellFormedUri = true, uriKind = UriKind.Absolute, uriString = "file:/c:/src/file.c" }, new { - expectedIsWellFormedUri = false, uriKind = UriKind.Absolute, - uriString = "ht%tp://www.example.com/rules/tst0001.html" + uriString = "file:/C:/src/file.c" + }, + new + { + uriKind = UriKind.Absolute, + uriString = "FILE:/C:/SRC/FILE.c" }, new { - expectedIsWellFormedUri = true, uriKind = UriKind.RelativeOrAbsolute, uriString = "file:///c:/Code/sarif-sdk/src/" }, new { - expectedIsWellFormedUri = true, uriKind = UriKind.RelativeOrAbsolute, uriString = "https://example.com/my-project" }, new { - expectedIsWellFormedUri = true, uriKind = UriKind.RelativeOrAbsolute, uriString = "file:/c:/src/file.c" }, new { - expectedIsWellFormedUri = false, uriKind = UriKind.RelativeOrAbsolute, - uriString = "ht%tp://www.example.com/rules/tst0001.html" + uriString = "file://networkLocation/folder/someDoc.txt" }, }; foreach(var testCase in testCases) { - SarifValidationSkimmerBase.UriIsWellFormedUriString(testCase.uriString, testCase.uriKind).Should().Be(testCase.expectedIsWellFormedUri); + SarifValidationSkimmerBase.IsWellFormedUriString(testCase.uriString, testCase.uriKind).Should().BeTrue(); + } + } + + [Fact] + public void SarifValidationSkimmerBase_UriIsWellFormedUriString_ShouldReturnFalseIfNotWellFormedUri() + { + var testCases = new[] + { + new + { + uriKind = UriKind.Absolute, + uriString = "ht%tp://www.example.com/rules/tst0001.html" + }, + new + { + uriKind = UriKind.RelativeOrAbsolute, + uriString = "ht%tp://www.example.com/rules/tst0001.html" + }, + }; + + foreach (var testCase in testCases) + { + SarifValidationSkimmerBase.IsWellFormedUriString(testCase.uriString, testCase.uriKind).Should().BeFalse(); } } } From 6c908c39a0845615c616211c18b97ab51b0b931c Mon Sep 17 00:00:00 2001 From: marmegh <30842915+marmegh@users.noreply.github.com> Date: Tue, 12 Jul 2022 11:55:35 -0700 Subject: [PATCH 6/7] Fixed missing whitespace in test. --- .../Rules/SarifValidationSkimmerBaseTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Test.UnitTests.Sarif.Multitool.Library/Rules/SarifValidationSkimmerBaseTests.cs b/src/Test.UnitTests.Sarif.Multitool.Library/Rules/SarifValidationSkimmerBaseTests.cs index aa22aa15a..3a4c10a9c 100644 --- a/src/Test.UnitTests.Sarif.Multitool.Library/Rules/SarifValidationSkimmerBaseTests.cs +++ b/src/Test.UnitTests.Sarif.Multitool.Library/Rules/SarifValidationSkimmerBaseTests.cs @@ -130,7 +130,7 @@ public void SarifValidationSkimmerBase_UriIsWellFormedUriString_ShouldReturnTrue }, }; - foreach(var testCase in testCases) + foreach (var testCase in testCases) { SarifValidationSkimmerBase.IsWellFormedUriString(testCase.uriString, testCase.uriKind).Should().BeTrue(); } From 976f073bd3a08d553cc5be09b2b24ee0da825df2 Mon Sep 17 00:00:00 2001 From: marmegh <30842915+marmegh@users.noreply.github.com> Date: Tue, 12 Jul 2022 22:26:44 -0700 Subject: [PATCH 7/7] Remove unneeded file and automatically generated code update from running BuildAndTest --- src/Sarif/Autogenerated/AddressComparer.cs | 6 +++++ .../Rules/SARIF1002.UrisMustBeValidTests.cs | 23 ------------------- 2 files changed, 6 insertions(+), 23 deletions(-) delete mode 100644 src/Test.UnitTests.Sarif.Multitool.Library/Rules/SARIF1002.UrisMustBeValidTests.cs diff --git a/src/Sarif/Autogenerated/AddressComparer.cs b/src/Sarif/Autogenerated/AddressComparer.cs index fbe3185bb..dc192c71c 100644 --- a/src/Sarif/Autogenerated/AddressComparer.cs +++ b/src/Sarif/Autogenerated/AddressComparer.cs @@ -32,6 +32,8 @@ public int Compare(Address left, Address right) return compareResult; } + // TryReferenceCompares is an autogenerated extension method + // that will properly handle the case when 'left' is null. if (left.RelativeAddress.TryReferenceCompares(right.RelativeAddress, out compareResult)) { return compareResult; @@ -43,6 +45,8 @@ public int Compare(Address left, Address right) return compareResult; } + // TryReferenceCompares is an autogenerated extension method + // that will properly handle the case when 'left' is null. if (left.Length.TryReferenceCompares(right.Length, out compareResult)) { return compareResult; @@ -72,6 +76,8 @@ public int Compare(Address left, Address right) return compareResult; } + // TryReferenceCompares is an autogenerated extension method + // that will properly handle the case when 'left' is null. if (left.OffsetFromParent.TryReferenceCompares(right.OffsetFromParent, out compareResult)) { return compareResult; diff --git a/src/Test.UnitTests.Sarif.Multitool.Library/Rules/SARIF1002.UrisMustBeValidTests.cs b/src/Test.UnitTests.Sarif.Multitool.Library/Rules/SARIF1002.UrisMustBeValidTests.cs deleted file mode 100644 index c7bee48a3..000000000 --- a/src/Test.UnitTests.Sarif.Multitool.Library/Rules/SARIF1002.UrisMustBeValidTests.cs +++ /dev/null @@ -1,23 +0,0 @@ -// Copyright (c) Microsoft. All rights reserved. -// Licensed under the MIT license. See LICENSE file in the project root for full license information. - -using System.Text; - -using FluentAssertions; - -using Microsoft.CodeAnalysis.Sarif.Multitool.Rules; - -using Xunit; - -namespace Microsoft.CodeAnalysis.Sarif.Multitool -{ - public class UrisMustBeValidTests - { - [Fact] - public void UrisMustBeValid_ShouldProduceExpectedResults() - { - var testSkimmer = new UrisMustBeValid(); - - } - } -}