diff --git a/src/Sarif.UnitTests/Visitors/MakeUriAbsoluteVisitorTest.cs b/src/Sarif.UnitTests/Visitors/MakeUriAbsoluteVisitorTest.cs index 21d90e9ec..27cc1974b 100644 --- a/src/Sarif.UnitTests/Visitors/MakeUriAbsoluteVisitorTest.cs +++ b/src/Sarif.UnitTests/Visitors/MakeUriAbsoluteVisitorTest.cs @@ -11,27 +11,22 @@ namespace Microsoft.CodeAnalysis.Sarif.Visitors { public class MakeUriAbsoluteVisitorTest { - private Run GenerateRunForTest(Dictionary uriBaseIdMapping) + private Run GenerateRunForTest(Dictionary originalUriBaseIds) { - Run run = new Run(); - run.Files = new List(new[] + return new Run { - new FileData() { FileLocation=new FileLocation{ Uri=new Uri("src/file1.cs", UriKind.Relative), UriBaseId="%TEST1%", FileIndex = 0 } }, - new FileData() { FileLocation=new FileLocation{ Uri=new Uri("src/file2.dll", UriKind.Relative), UriBaseId="%TEST2%", FileIndex = 1 } }, - new FileData() { FileLocation=new FileLocation{ Uri=new Uri("src/archive.zip", UriKind.Relative), UriBaseId="%TEST1%", FileIndex = 2 } }, - new FileData() { FileLocation=new FileLocation{ Uri=new Uri("src/archive.zip#file3.cs", UriKind.Relative), UriBaseId="%TEST1%", FileIndex = 3 } }, - new FileData() { FileLocation=new FileLocation{ Uri=new Uri("src/archive.zip#archive2.gz", UriKind.Relative), UriBaseId="%TEST1%", FileIndex = 4 } }, - new FileData() { FileLocation=new FileLocation{ Uri=new Uri("src/archive.zip#archive2.gz/file4.cs", UriKind.Relative), UriBaseId="%TEST1%", FileIndex = 5 }, }, - new FileData() { FileLocation=new FileLocation{ Uri=new Uri("src/archive.zip#file5.cs", UriKind.Relative), UriBaseId="%TEST1%", FileIndex = 6 }, } - }); - - if (uriBaseIdMapping != null) - { - run.Properties = new Dictionary(); - - run.Properties[RebaseUriVisitor.BaseUriDictionaryName] = RebaseUriVisitor.ReserializePropertyDictionary(uriBaseIdMapping); - } - return run; + Files = new List(new[] + { + new FileData { FileLocation=new FileLocation{ Uri=new Uri("src/file1.cs", UriKind.Relative), UriBaseId="%TEST1%", FileIndex = 0 } }, + new FileData { FileLocation=new FileLocation{ Uri=new Uri("src/file2.dll", UriKind.Relative), UriBaseId="%TEST2%", FileIndex = 1 } }, + new FileData { FileLocation=new FileLocation{ Uri=new Uri("src/archive.zip", UriKind.Relative), UriBaseId="%TEST1%", FileIndex = 2 } }, + new FileData { FileLocation=new FileLocation{ Uri=new Uri("file3.cs", UriKind.Relative), FileIndex = 3 }, ParentIndex = 2 }, + new FileData { FileLocation=new FileLocation{ Uri=new Uri("archive2.gz", UriKind.Relative), FileIndex = 4 }, ParentIndex = 2 }, + new FileData { FileLocation=new FileLocation{ Uri=new Uri("file4.cs", UriKind.Relative), FileIndex = 5 }, ParentIndex = 4 }, + }), + + OriginalUriBaseIds = originalUriBaseIds + }; } [Fact] @@ -44,7 +39,7 @@ public void MakeUriAbsoluteVisitor_VisitPhysicalLocation_SetsAbsoluteURI() ["%TEST%"] = new FileLocation { Uri = new Uri("C:/github/sarif/") } } }; - + // Initializes visitor with run in order to retrieve uri base id mappings MakeUrisAbsoluteVisitor visitor = new MakeUrisAbsoluteVisitor(); visitor.VisitRun(run); @@ -103,79 +98,117 @@ public void MakeUriAbsoluteVisitor_VisitPhysicalLocation_DoesNotSetUriIfBaseIsNo [Fact] public void MakeUriAbsoluteVisitor_VisitRun_SetsAbsoluteUriForAllApplicableFiles() { - Run run = GenerateRunForTest(new Dictionary() + Run run = GenerateRunForTest(new Dictionary() { - { "%TEST1%", new Uri(@"C:\srcroot\") }, - { "%TEST2%", new Uri(@"D:\bld\out\") } + ["%TEST1%"] = new FileLocation { Uri = new Uri(@"C:\srcroot\") }, + ["%TEST2%"] = new FileLocation { Uri = new Uri(@"D:\bld\out\") } }); - + MakeUrisAbsoluteVisitor visitor = new MakeUrisAbsoluteVisitor(); var newRun = visitor.VisitRun(run); + // Validate. - newRun.Files[0].FileLocation.Uri.ToString().Should().Be(@"file://C:/srcroot/src/file1.cs"); - newRun.Files[1].FileLocation.Uri.ToString().Should().Be(@"file://D:/bld/out/src/file2.dll"); - newRun.Files[2].FileLocation.Uri.ToString().Should().Be(@"file://C:/srcroot/src/archive.zip"); - newRun.Files[3].FileLocation.Uri.ToString().Should().Be(@"file://C:/srcroot/src/archive.zip#file3.cs"); - newRun.Files[4].FileLocation.Uri.ToString().Should().Be(@"file://C:/srcroot/src/archive.zip#archive2.gz"); - newRun.Files[5].FileLocation.Uri.ToString().Should().Be(@"file://C:/srcroot/src/archive.zip#archive2.gz/file4.cs"); - newRun.Files[6].FileLocation.Uri.ToString().Should().Be(@"file://C:/srcroot/src/archive.zip#file5.cs"); + newRun.Files[0].FileLocation.Uri.ToString().Should().Be(@"file:///C:/srcroot/src/file1.cs"); + newRun.Files[1].FileLocation.Uri.ToString().Should().Be(@"file:///D:/bld/out/src/file2.dll"); + newRun.Files[2].FileLocation.Uri.ToString().Should().Be(@"file:///C:/srcroot/src/archive.zip"); + newRun.Files[3].FileLocation.Uri.ToString().Should().Be(@"file3.cs"); + newRun.Files[4].FileLocation.Uri.ToString().Should().Be(@"archive2.gz"); + newRun.Files[5].FileLocation.Uri.ToString().Should().Be(@"file4.cs"); // Operation should zap all uri base ids - newRun.Files.Select(f => f.FileLocation.UriBaseId != null).Any().Should().BeFalse(); + newRun.Files.Where(f => f.FileLocation.UriBaseId != null).Any().Should().BeFalse(); } [Fact] public void MakeUriAbsoluteVisitor_VisitRun_DoesNotSetAbsoluteUriIfNotApplicable() { - Dictionary uriMapping = new Dictionary() + Dictionary uriMapping = new Dictionary() { - { "%TEST3%", new Uri(@"C:\srcroot\") }, - { "%TEST4%", new Uri(@"D:\bld\out\") }, + ["%TEST3%"] = new FileLocation { Uri = new Uri(@"C:\srcroot\") }, + ["%TEST4%"] = new FileLocation { Uri = new Uri(@"D:\bld\out\") } }; Run expectedRun = GenerateRunForTest(uriMapping); Run actualRun = expectedRun.DeepClone(); - MakeUrisAbsoluteVisitor visitor = new MakeUrisAbsoluteVisitor(); var newRun = visitor.VisitRun(actualRun); - expectedRun.Should().Be(actualRun); - } - - [Fact] - public void MakeUriAbsoluteVisitor_VisitRun_ThrowsIfPropertiesAreWrong() - { - Run run = GenerateRunForTest(null); - run.Properties = new Dictionary(); - run.Properties[RebaseUriVisitor.BaseUriDictionaryName] = new SerializedPropertyInfo("\"this is a string\"", true); - - MakeUrisAbsoluteVisitor visitor = new MakeUrisAbsoluteVisitor(); - Assert.Throws(()=> visitor.VisitRun(run)); + expectedRun.ValueEquals(actualRun).Should().BeTrue(); } [Fact] public void MakeUriAbsoluteVisitor_VisitSarifLog_MultipleRunsWithDifferentProperties_RebasesProperly() { - Run runA = GenerateRunForTest(new Dictionary() + Run runA = GenerateRunForTest(new Dictionary() { - { "%TEST1%", new Uri(@"C:\srcroot\") }, - { "%TEST2%", new Uri(@"D:\bld\out\") }, + ["%TEST1%"] = new FileLocation { Uri = new Uri(@"C:\srcroot\") }, + ["%TEST2%"] = new FileLocation { Uri = new Uri(@"D:\bld\out\") } }); - Run runB = GenerateRunForTest(new Dictionary() + Run runB = GenerateRunForTest(new Dictionary() { - { "%TEST1%", new Uri(@"C:\src\abc") }, - { "%TEST2%", new Uri(@"D:\bld\123\") }, + ["%TEST1%"] = new FileLocation { Uri = new Uri(@"C:\src\abc\") }, + ["%TEST2%"] = new FileLocation { Uri = new Uri(@"D:\bld\123\") } }); MakeUrisAbsoluteVisitor visitor = new MakeUrisAbsoluteVisitor(); - SarifLog log = new SarifLog() { Runs=new Run[] { runA, runB } }; + SarifLog log = new SarifLog() { Runs = new Run[] { runA, runB } }; SarifLog newLog = visitor.VisitSarifLog(log); // Validate newLog.Runs.Should().HaveCount(2); newLog.Runs[0].Files.Should().NotIntersectWith(newLog.Runs[1].Files); } + + [Fact] + public void MakeUriAbsoluteVisitor_CombineUriFunctionsProperly() + { + var testCases = new Tuple[] + { + new Tuple + (@"https://base/", @"relative/file.cpp", "https://base/relative/file.cpp") + }; + + foreach (Tuple testCase in testCases) + { + MakeUrisAbsoluteVisitor.CombineUris( + absoluteBaseUri: new Uri(testCase.Item1, UriKind.Absolute), + relativeUri: new Uri(testCase.Item2, UriKind.Relative)) + .Should().Be(testCase.Item3); + } + } + + [Fact] + public void MakeUriAbsoluteVisitor_CombineUriValidatesArgumentsProperly() + { + Uri absoluteUri = new Uri("https://absolute.example.com", UriKind.Absolute); + Uri relativeUri = new Uri("relative/someResource", UriKind.Relative); + + // First, ensure that our test data succeeds when used properly + MakeUrisAbsoluteVisitor.CombineUris( + absoluteBaseUri: absoluteUri, + relativeUri: relativeUri); + + // Pass relative URI where absolute expected. + Action action = () => + { + MakeUrisAbsoluteVisitor.CombineUris( + absoluteBaseUri: relativeUri, + relativeUri: relativeUri); + }; + + action.Should().Throw(); + + // Pass absolute URI where relative expected. + action = () => + { + MakeUrisAbsoluteVisitor.CombineUris( + absoluteBaseUri: absoluteUri, + relativeUri: absoluteUri); + }; + + action.Should().Throw(); + } } } diff --git a/src/Sarif/Core/Run.cs b/src/Sarif/Core/Run.cs index a56ac4ecd..118514ea1 100644 --- a/src/Sarif/Core/Run.cs +++ b/src/Sarif/Core/Run.cs @@ -17,9 +17,15 @@ public partial class Run private IDictionary _fileToIndexMap; - public Uri GetExpandedUriBaseIdValue(string key, string currentValue = null) + public Uri ExpandUrisWithUriBaseId(string key, string currentValue = null) { - throw new InvalidOperationException("Author this code along with tests"); + FileLocation fileLocation = this.OriginalUriBaseIds[key]; + + if (fileLocation.UriBaseId == null) + { + return fileLocation.Uri; + } + throw new InvalidOperationException("Author this code along with tests for originalUriBaseIds that are nested"); } public int GetFileIndex( diff --git a/src/Sarif/Processors/Log/SarifLogStageFactory.cs b/src/Sarif/Processors/Log/SarifLogStageFactory.cs index def0bd069..59844f3f3 100644 --- a/src/Sarif/Processors/Log/SarifLogStageFactory.cs +++ b/src/Sarif/Processors/Log/SarifLogStageFactory.cs @@ -31,11 +31,10 @@ public static IActionWrapper GetActionStage(SarifLogAction action, par { return new GenericMappingAction(log => { - bool rebaseRelativeUris = false; - bool castRelativeUrisArg = bool.TryParse(args[1], out rebaseRelativeUris); + bool castRelativeUrisArg = bool.TryParse(args[1], out bool rebaseRelativeUris); Debug.Assert(castRelativeUrisArg); - RebaseUriVisitor visitor = new RebaseUriVisitor(args[0], rebaseRelativeUris, new Uri(args[2])); + RebaseUriVisitor visitor = new RebaseUriVisitor(args[0], new Uri(args[2]), rebaseRelativeUris); return visitor.VisitSarifLog(log); }); } @@ -56,7 +55,7 @@ public static IActionWrapper GetActionStage(SarifLogAction action, par } } - private static Func mergeFunction = + private readonly static Func mergeFunction = (accumulator, nextLog) => { if (nextLog.Runs == null) diff --git a/src/Sarif/Visitors/MakeUrisAbsoluteVisitor.cs b/src/Sarif/Visitors/MakeUrisAbsoluteVisitor.cs index 845c08762..8713242a5 100644 --- a/src/Sarif/Visitors/MakeUrisAbsoluteVisitor.cs +++ b/src/Sarif/Visitors/MakeUrisAbsoluteVisitor.cs @@ -19,9 +19,10 @@ public override FileLocation VisitFileLocation(FileLocation node) { if ( _run.OriginalUriBaseIds!= null && !string.IsNullOrEmpty(node?.UriBaseId) && - _run.OriginalUriBaseIds.ContainsKey(node.UriBaseId)) + _run.OriginalUriBaseIds.ContainsKey(node.UriBaseId) && + !_run.OriginalUriBaseIds.Values.Contains(node)) { - Uri baseUri = _run.GetExpandedUriBaseIdValue(node.UriBaseId); + Uri baseUri = _run.ExpandUrisWithUriBaseId(node.UriBaseId); node.Uri = CombineUris(baseUri, node.Uri); node.UriBaseId = null; } @@ -29,9 +30,19 @@ public override FileLocation VisitFileLocation(FileLocation node) return node; } - private Uri CombineUris(Uri baseUri, Uri uri) + internal static Uri CombineUris(Uri absoluteBaseUri, Uri relativeUri) { - throw new NotImplementedException(); + if (!absoluteBaseUri.IsAbsoluteUri) + { + throw new ArgumentException($"{nameof(absoluteBaseUri)} is not an absolute URI", nameof(absoluteBaseUri)); + } + + if (relativeUri.IsAbsoluteUri) + { + throw new ArgumentException($"${nameof(relativeUri)} is not a relative URI", nameof(relativeUri)); + } + + return new Uri(absoluteBaseUri, relativeUri); } } } diff --git a/src/Sarif/Visitors/RebaseUriVisitor.cs b/src/Sarif/Visitors/RebaseUriVisitor.cs index aa2eaf663..49cb743dc 100644 --- a/src/Sarif/Visitors/RebaseUriVisitor.cs +++ b/src/Sarif/Visitors/RebaseUriVisitor.cs @@ -3,45 +3,31 @@ using System; using System.Collections.Generic; -using System.Diagnostics; -using Newtonsoft.Json; namespace Microsoft.CodeAnalysis.Sarif.Visitors { /// - /// A class that, given a variable name (e.x. "%SRCROOT%") and a value (e.x. "C:\src\root\"), rebases the URIs in a SARIF log - /// in order to make a SARIF log not depend on absolute paths/be machine independent. + /// A visitor that, given a URI base id (e.g., "%SRCROOT%") and its value (e.g., "C:\src\root\"), + /// rebases the URIs in a SARIF log to make the log independent of absolute paths (i.e., machine independent). /// public class RebaseUriVisitor : SarifRewritingVisitor { - internal const string BaseUriDictionaryName = "originalUriBaseIds"; - internal const string IncorrectlyFormattedDictionarySuffix = ".Old"; - - private Uri _baseUri; - private string _baseName; - private bool _rebaseRelativeUris; + private readonly Uri _baseUri; + private readonly string _uriBaseId; + private readonly bool _rebaseRelativeUris; /// - /// Create a RebaseUriVisitor, with a given name for the Base URI and a value for the base URI. + /// Create a new instance of the RebaseUriVisitor class with the specified URI base id and its value. /// - public RebaseUriVisitor(string baseName, Uri baseUri) + public RebaseUriVisitor(string uriBaseId, Uri baseUri, bool rebaseRelativeUris = false) { - _baseUri = baseUri; - _baseName = baseName; - _rebaseRelativeUris = false; - - Debug.Assert(_baseUri.IsAbsoluteUri); - } + if (!baseUri.IsAbsoluteUri) + { + throw new ArgumentException($"{nameof(baseUri)} must be an absolute URI.", nameof(baseUri)); + } - /// - /// Create a RebaseUriVisitor, with a given name for the Base URI and a value for the base URI. - /// - public RebaseUriVisitor(string baseName, bool rebaseRelativeUris, Uri baseUri) - { _baseUri = baseUri; - _baseName = baseName; - Debug.Assert(_baseUri.IsAbsoluteUri); - + _uriBaseId = uriBaseId; _rebaseRelativeUris = rebaseRelativeUris; } @@ -51,12 +37,12 @@ public override FileLocation VisitFileLocation(FileLocation node) if (newNode.Uri.IsAbsoluteUri && _baseUri.IsBaseOf(newNode.Uri)) { - newNode.UriBaseId = _baseName; + newNode.UriBaseId = _uriBaseId; newNode.Uri = _baseUri.MakeRelativeUri(node.Uri); } else if (_rebaseRelativeUris && !newNode.Uri.IsAbsoluteUri) { - newNode.UriBaseId = _baseName; + newNode.UriBaseId = _uriBaseId; } return newNode; @@ -66,55 +52,12 @@ public override Run VisitRun(Run node) { Run newRun = base.VisitRun(node); - // If the dictionary doesn't exist, we should add it to the properties. If it does, we should add/update the existing dictionary. - IDictionary baseUriDictionary = new Dictionary(); - if (node.OriginalUriBaseIds != null) - { - baseUriDictionary = node.OriginalUriBaseIds; - } - - // Note--this is an add or update, so if this is run twice with the same base variable, we'll replace the path. - baseUriDictionary[_baseName] = new FileLocation { Uri =_baseUri }; - newRun.OriginalUriBaseIds = baseUriDictionary; - - return newRun; - } + newRun.OriginalUriBaseIds = newRun.OriginalUriBaseIds ?? new Dictionary(); + // Add dictionary entry if it doesn't exist, or replace it if it does. + newRun.OriginalUriBaseIds[_uriBaseId] = new FileLocation { Uri =_baseUri }; - internal static bool TryDeserializePropertyDictionary(SerializedPropertyInfo serializedProperty, out Dictionary dictionary) - { - try - { - dictionary = JsonConvert.DeserializeObject>(serializedProperty.SerializedValue); - - return true; - } - // Didn't deserialize correctly - catch (Exception ex) - { - if(ex is JsonSerializationException || ex is ArgumentNullException) - { - dictionary = null; - return false; - } - throw; - } - } - - /// - /// Internal as used in testing as a helper. - /// - internal static Dictionary DeserializePropertyDictionary(SerializedPropertyInfo info) - { - return JsonConvert.DeserializeObject>(info.SerializedValue); - } - - /// - /// Internal as used in testing as a helper. - /// - internal static SerializedPropertyInfo ReserializePropertyDictionary(Dictionary dictionary) - { - return new SerializedPropertyInfo(JsonConvert.SerializeObject(dictionary), false); + return newRun; } } }