From 7bd9cb88dabbd7d999e0e5db91e8db0cd128461f Mon Sep 17 00:00:00 2001 From: "Michael C. Fanning" Date: Tue, 31 Jan 2023 15:47:09 -0800 Subject: [PATCH 01/15] Add rolling has optional data enum value. Remove SarifLogger tool parameter. --- src/ReleaseHistory.md | 1 + src/Sarif/OptionallyEmittedData.cs | 11 +++++++---- src/Sarif/Writers/SarifLogger.cs | 3 --- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/ReleaseHistory.md b/src/ReleaseHistory.md index a21b4f30f..ff3c31078 100644 --- a/src/ReleaseHistory.md +++ b/src/ReleaseHistory.md @@ -1,6 +1,7 @@ # SARIF Package Release History (SDK, Driver, Converters, and Multitool) ## **v3.2.0** (UNRELEASED) +* BRK: `SarifLogger` no longer allows providing a `Tool` instance. Use the `run` parameter instead (and populate it with any custom `Tool` object). [#2661](https://github.com/microsoft/sarif-sdk/pull/2611) * BRK: `SarifLogger` updates version details differently. [#2661](https://github.com/microsoft/sarif-sdk/pull/2611) * BRK: Add `ToolComponent` argument to `IAnalysisLogger.Log(ReportingDescriptor, Result)` method. [#2661](https://github.com/microsoft/sarif-sdk/pull/2611) * BRK: Rename `--normalize-for-github` argument to `--normalize-for-ghas` for `convert` command and mark `--normalize-for-github` as obsolete. [#2581](https://github.com/microsoft/sarif-sdk/pull/2581) diff --git a/src/Sarif/OptionallyEmittedData.cs b/src/Sarif/OptionallyEmittedData.cs index 04d2e9b6f..d534b50b7 100644 --- a/src/Sarif/OptionallyEmittedData.cs +++ b/src/Sarif/OptionallyEmittedData.cs @@ -66,7 +66,7 @@ public enum OptionallyEmittedData : int // both versions exist FlattenedMessages = 0x100, - // SARIF Results may each have a GUID assigned to uniquely identify them. + // Assign a unique GUID to every SARIF result. Guids = 0x200, // Specifies the repository, branch, and other information describing the source controlled @@ -76,20 +76,23 @@ public enum OptionallyEmittedData : int // A set of threadFlows which together describe a pattern of code execution relevant // to detecting a result. CodeFlows = 0x800, - + // Enrich SARIF log with git blame information GitBlameInformation = 0x1000, - // Enrich SARIF log with partial fingerprint based on the hash value of the context region snippet (sha256) + // Enrich SARIF log with partial fingerprint based on the hash value of the context region snippet (sha256). ContextRegionSnippetPartialFingerprints = 0x2000, + // Enrich SARIF log with partial fingerprint based on the CodeQL rolling hash algorithm. + RollingHashPartialFingerprints = 0x4000, + // A special enum value that indicates that insertion should overwrite any existing // information in the SARIF log file. In the absence of this setting, any existing // data that would otherwise have been overwritten by the insert operation will // be preserved. OverwriteExistingData = 0x40000000, - // Insert Everything - should include every flag except the overwrite and git blame information ones + // Insert Everything - should include every flag except the overwrite and git blame information ones. All = ~OverwriteExistingData & ~GitBlameInformation } } diff --git a/src/Sarif/Writers/SarifLogger.cs b/src/Sarif/Writers/SarifLogger.cs index 822dfb6fb..846fc68c9 100644 --- a/src/Sarif/Writers/SarifLogger.cs +++ b/src/Sarif/Writers/SarifLogger.cs @@ -34,7 +34,6 @@ public SarifLogger(string outputFilePath, LogFilePersistenceOptions logFilePersistenceOptions = DefaultLogFilePersistenceOptions, OptionallyEmittedData dataToInsert = OptionallyEmittedData.None, OptionallyEmittedData dataToRemove = OptionallyEmittedData.None, - Tool tool = null, Run run = null, IEnumerable analysisTargets = null, IEnumerable invocationTokensToRedact = null, @@ -50,7 +49,6 @@ public SarifLogger(string outputFilePath, logFilePersistenceOptions, dataToInsert, dataToRemove, - tool, run, analysisTargets, invocationTokensToRedact, @@ -69,7 +67,6 @@ public SarifLogger(TextWriter textWriter, LogFilePersistenceOptions logFilePersistenceOptions = DefaultLogFilePersistenceOptions, OptionallyEmittedData dataToInsert = OptionallyEmittedData.None, OptionallyEmittedData dataToRemove = OptionallyEmittedData.None, - Tool tool = null, Run run = null, IEnumerable analysisTargets = null, IEnumerable invocationTokensToRedact = null, From 9346d42da4f75cb89fdbdba2d859f86d251af01a Mon Sep 17 00:00:00 2001 From: "Michael C. Fanning" Date: Tue, 31 Jan 2023 16:27:30 -0800 Subject: [PATCH 02/15] Remove unnecessary tool argument from all log construction. --- src/ReleaseHistory.md | 6 +++--- src/Samples/Sarif.Sdk.Sample/Program.cs | 1 - src/Samples/SarifTrim/Program.cs | 2 +- src/Sarif.Driver/Sdk/AnalyzeCommandBase.cs | 5 ++--- src/Sarif.Driver/Sdk/MultithreadedAnalyzeCommandBase.cs | 5 ++--- src/Sarif/Writers/SarifLogger.cs | 4 +--- src/Sarif/Writers/SarifOneZeroZeroLogger.cs | 2 -- 7 files changed, 9 insertions(+), 16 deletions(-) diff --git a/src/ReleaseHistory.md b/src/ReleaseHistory.md index ff3c31078..89940c423 100644 --- a/src/ReleaseHistory.md +++ b/src/ReleaseHistory.md @@ -1,9 +1,9 @@ # SARIF Package Release History (SDK, Driver, Converters, and Multitool) ## **v3.2.0** (UNRELEASED) -* BRK: `SarifLogger` no longer allows providing a `Tool` instance. Use the `run` parameter instead (and populate it with any custom `Tool` object). [#2661](https://github.com/microsoft/sarif-sdk/pull/2611) -* BRK: `SarifLogger` updates version details differently. [#2661](https://github.com/microsoft/sarif-sdk/pull/2611) -* BRK: Add `ToolComponent` argument to `IAnalysisLogger.Log(ReportingDescriptor, Result)` method. [#2661](https://github.com/microsoft/sarif-sdk/pull/2611) +* BRK: `SarifLogger` no longer allows providing a `Tool` instance. Use the `run` parameter instead (and populate it with any custom `Tool` object). [#2614](https://github.com/microsoft/sarif-sdk/pull/2614) +* BRK: `SarifLogger` updates version details differently. [#2611](https://github.com/microsoft/sarif-sdk/pull/2611) +* BRK: Add `ToolComponent` argument to `IAnalysisLogger.Log(ReportingDescriptor, Result)` method. [#2611](https://github.com/microsoft/sarif-sdk/pull/2611) * BRK: Rename `--normalize-for-github` argument to `--normalize-for-ghas` for `convert` command and mark `--normalize-for-github` as obsolete. [#2581](https://github.com/microsoft/sarif-sdk/pull/2581) * BRK: Update `IAnalysisContext.LogToolNotification` method to add `ReportingDescriptor` parameter. This is required in order to populated `AssociatedRule` data in `Notification` instances. The new method has an option value of null for the `associatedRule` parameter to maximize build compatibility. [#2604](https://github.com/microsoft/sarif-sdk/pull/2604) * BRK: Correct casing of `LogMissingreportingConfiguration` helper to `LogMissingReportingConfiguration`. [#2599](https://github.com/microsoft/sarif-sdk/pull/2599) diff --git a/src/Samples/Sarif.Sdk.Sample/Program.cs b/src/Samples/Sarif.Sdk.Sample/Program.cs index 3f20b8f7f..1280cb02f 100644 --- a/src/Samples/Sarif.Sdk.Sample/Program.cs +++ b/src/Samples/Sarif.Sdk.Sample/Program.cs @@ -328,7 +328,6 @@ private static void SerializeSarifResult(string filePath, int numResult, bool us OptionallyEmittedData.TextFiles | // Embed source file content directly in the log file -- great for portability of the log! OptionallyEmittedData.Hashes | OptionallyEmittedData.RegionSnippets, - tool: null, run: run, kinds: new ResultKind[] { ResultKind.Fail, ResultKind.Pass }, levels: new FailureLevel[] { FailureLevel.Error, FailureLevel.Warning, FailureLevel.Note }, diff --git a/src/Samples/SarifTrim/Program.cs b/src/Samples/SarifTrim/Program.cs index 7c56ef038..2b5dc09a2 100644 --- a/src/Samples/SarifTrim/Program.cs +++ b/src/Samples/SarifTrim/Program.cs @@ -53,7 +53,7 @@ private static void Main(string[] args) consolidator.RemoveWebResponses = (removeParts.Contains("WebResponses")); // Consolidate the SarifLog per settings - using (SarifLogger logger = new SarifLogger(outputFilePath, LogFilePersistenceOptions.OverwriteExistingOutputFile, tool: run.Tool, run: run)) + using (SarifLogger logger = new SarifLogger(outputFilePath, LogFilePersistenceOptions.OverwriteExistingOutputFile, run: run)) { foreach (Result result in run.Results) { diff --git a/src/Sarif.Driver/Sdk/AnalyzeCommandBase.cs b/src/Sarif.Driver/Sdk/AnalyzeCommandBase.cs index 85712f776..44fb2ffa7 100644 --- a/src/Sarif.Driver/Sdk/AnalyzeCommandBase.cs +++ b/src/Sarif.Driver/Sdk/AnalyzeCommandBase.cs @@ -411,7 +411,8 @@ private void InitializeOutputFile(TOptions analyzeOptions, TContext context) { Id = analyzeOptions.AutomationId, Guid = analyzeOptions.AutomationGuid - } + }, + Tool = _tool, }; if (analyzeOptions.SarifOutputVersion != SarifVersion.OneZeroZero) @@ -421,7 +422,6 @@ private void InitializeOutputFile(TOptions analyzeOptions, TContext context) logFilePersistenceOptions, dataToInsert, dataToRemove, - tool: _tool, run: _run, analysisTargets: null, quiet: analyzeOptions.Quiet, @@ -437,7 +437,6 @@ private void InitializeOutputFile(TOptions analyzeOptions, TContext context) logFilePersistenceOptions, dataToInsert, dataToRemove, - tool: _tool, run: _run, analysisTargets: null, invocationTokensToRedact: GenerateSensitiveTokensList(), diff --git a/src/Sarif.Driver/Sdk/MultithreadedAnalyzeCommandBase.cs b/src/Sarif.Driver/Sdk/MultithreadedAnalyzeCommandBase.cs index efdb5e64d..f68116ef5 100644 --- a/src/Sarif.Driver/Sdk/MultithreadedAnalyzeCommandBase.cs +++ b/src/Sarif.Driver/Sdk/MultithreadedAnalyzeCommandBase.cs @@ -770,7 +770,8 @@ public virtual void InitializeOutputFile(TOptions analyzeOptions, TContext conte { Id = analyzeOptions.AutomationId, Guid = analyzeOptions.AutomationGuid - } + }, + Tool = Tool, }; if (analyzeOptions.SarifOutputVersion != SarifVersion.OneZeroZero) @@ -779,7 +780,6 @@ public virtual void InitializeOutputFile(TOptions analyzeOptions, TContext conte logFilePersistenceOptions, dataToInsert, dataToRemove, - tool: Tool, run: _run, analysisTargets: null, quiet: analyzeOptions.Quiet, @@ -795,7 +795,6 @@ public virtual void InitializeOutputFile(TOptions analyzeOptions, TContext conte logFilePersistenceOptions, dataToInsert, dataToRemove, - tool: Tool, run: _run, analysisTargets: null, invocationTokensToRedact: GenerateSensitiveTokensList(), diff --git a/src/Sarif/Writers/SarifLogger.cs b/src/Sarif/Writers/SarifLogger.cs index 846fc68c9..d276b86ff 100644 --- a/src/Sarif/Writers/SarifLogger.cs +++ b/src/Sarif/Writers/SarifLogger.cs @@ -123,9 +123,7 @@ public SarifLogger(TextWriter textWriter, defaultFileEncoding, AnalysisTargetToHashDataMap); - tool ??= Tool.CreateFromAssemblyData(); - - _run.Tool = tool; + _run.Tool ??= Tool.CreateFromAssemblyData(); _dataToInsert = dataToInsert; _dataToRemove = dataToRemove; _issueLogJsonWriter.Initialize(_run); diff --git a/src/Sarif/Writers/SarifOneZeroZeroLogger.cs b/src/Sarif/Writers/SarifOneZeroZeroLogger.cs index 19eb8a7b5..d797318c2 100644 --- a/src/Sarif/Writers/SarifOneZeroZeroLogger.cs +++ b/src/Sarif/Writers/SarifOneZeroZeroLogger.cs @@ -25,7 +25,6 @@ public SarifOneZeroZeroLogger( LogFilePersistenceOptions logFilePersistenceOptions = SarifLogger.DefaultLogFilePersistenceOptions, OptionallyEmittedData dataToInsert = OptionallyEmittedData.None, OptionallyEmittedData dataToRemove = OptionallyEmittedData.None, - Tool tool = null, Run run = null, IEnumerable analysisTargets = null, IEnumerable invocationTokensToRedact = null, @@ -39,7 +38,6 @@ public SarifOneZeroZeroLogger( dataToInsert: dataToInsert, dataToRemove: dataToRemove, defaultFileEncoding: defaultFileEncoding, - tool: tool, run: run, analysisTargets: analysisTargets, invocationTokensToRedact: invocationTokensToRedact, From e596d394c5aaf48c4289c0846001f222a7ec8236 Mon Sep 17 00:00:00 2001 From: "Michael C. Fanning" Date: Tue, 31 Jan 2023 16:30:57 -0800 Subject: [PATCH 03/15] Formatting updates. --- src/Sarif.Driver/Sdk/AnalyzeCommandBase.cs | 2 +- src/Sarif/OptionallyEmittedData.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Sarif.Driver/Sdk/AnalyzeCommandBase.cs b/src/Sarif.Driver/Sdk/AnalyzeCommandBase.cs index 44fb2ffa7..d6945f962 100644 --- a/src/Sarif.Driver/Sdk/AnalyzeCommandBase.cs +++ b/src/Sarif.Driver/Sdk/AnalyzeCommandBase.cs @@ -412,7 +412,7 @@ private void InitializeOutputFile(TOptions analyzeOptions, TContext context) Id = analyzeOptions.AutomationId, Guid = analyzeOptions.AutomationGuid }, - Tool = _tool, + Tool = _tool, }; if (analyzeOptions.SarifOutputVersion != SarifVersion.OneZeroZero) diff --git a/src/Sarif/OptionallyEmittedData.cs b/src/Sarif/OptionallyEmittedData.cs index d534b50b7..f13d8f29a 100644 --- a/src/Sarif/OptionallyEmittedData.cs +++ b/src/Sarif/OptionallyEmittedData.cs @@ -76,7 +76,7 @@ public enum OptionallyEmittedData : int // A set of threadFlows which together describe a pattern of code execution relevant // to detecting a result. CodeFlows = 0x800, - + // Enrich SARIF log with git blame information GitBlameInformation = 0x1000, From 0edd2d4adccf8d4b423cc902a76ea9f6f1cb687d Mon Sep 17 00:00:00 2001 From: "Michael C. Fanning" Date: Tue, 31 Jan 2023 17:19:41 -0800 Subject: [PATCH 04/15] Fine-tune extensions serialization. --- src/Sarif/Core/Tool.cs | 11 ++++++++--- src/Sarif/Core/ToolComponent.cs | 5 +++++ 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/Sarif/Core/Tool.cs b/src/Sarif/Core/Tool.cs index 940c0bf1f..0919972ef 100644 --- a/src/Sarif/Core/Tool.cs +++ b/src/Sarif/Core/Tool.cs @@ -42,9 +42,9 @@ public static Tool CreateFromAssemblyData(Assembly assembly = null, Driver = new ToolComponent { Name = name, - FullName = name + " " + version.ToString(), - Version = fileVersion.FileVersion, - DottedQuadFileVersion = dottedQuadFileVersion, + FullName = name + " " + (omitSemanticVersion ? null : version.ToString()), + Version = omitSemanticVersion ? null :fileVersion.FileVersion, + DottedQuadFileVersion = omitSemanticVersion ? null : dottedQuadFileVersion, SemanticVersion = omitSemanticVersion ? null : fileVersion.ProductVersion, Organization = string.IsNullOrEmpty(fileVersion.CompanyName) ? null : fileVersion.CompanyName, Product = string.IsNullOrEmpty(fileVersion.ProductName) ? null : fileVersion.ProductName, @@ -107,5 +107,10 @@ public ToolComponent GetToolComponentFromReference(ToolComponentReference refere // Neither specified? Driver. return this.Driver; } + + public bool ShouldSerializeExtensions() + { + return this.Extensions?.HasAtLeastOneNonNullValue() == true; + } } } diff --git a/src/Sarif/Core/ToolComponent.cs b/src/Sarif/Core/ToolComponent.cs index 3f2084deb..12dad1c4f 100644 --- a/src/Sarif/Core/ToolComponent.cs +++ b/src/Sarif/Core/ToolComponent.cs @@ -55,6 +55,11 @@ public ReportingDescriptor GetRuleByGuid(Guid ruleGuid) return rule; } + public bool ShouldSerializeName() + { + return !string.IsNullOrWhiteSpace(this.Name); + } + public bool ShouldSerializeRules() { return this.Rules.HasAtLeastOneNonNullValue(); From d4efb0dfded04b124ce344bd592808c278049bb7 Mon Sep 17 00:00:00 2001 From: "Michael C. Fanning" Date: Wed, 1 Feb 2023 07:32:09 -0800 Subject: [PATCH 05/15] Don't serialize empty guid values. --- src/Sarif/Core/ToolComponent.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/Sarif/Core/ToolComponent.cs b/src/Sarif/Core/ToolComponent.cs index 12dad1c4f..15d70b1ba 100644 --- a/src/Sarif/Core/ToolComponent.cs +++ b/src/Sarif/Core/ToolComponent.cs @@ -55,6 +55,11 @@ public ReportingDescriptor GetRuleByGuid(Guid ruleGuid) return rule; } + public bool ShouldSerializeGuid() + { + return this.Guid != default; + } + public bool ShouldSerializeName() { return !string.IsNullOrWhiteSpace(this.Name); From fe3d1385631941ed23bfd82c0f1ee4ccf3863978 Mon Sep 17 00:00:00 2001 From: "Michael C. Fanning" Date: Wed, 1 Feb 2023 15:01:13 -0800 Subject: [PATCH 06/15] Add hash computation helpers. --- src/Sarif/Core/Artifact.cs | 2 +- src/Sarif/Core/Tool.cs | 2 +- src/Sarif/HashUtilities.cs | 67 +++++++++++++++++++------------- src/Sarif/Writers/SarifLogger.cs | 10 ++++- 4 files changed, 50 insertions(+), 31 deletions(-) diff --git a/src/Sarif/Core/Artifact.cs b/src/Sarif/Core/Artifact.cs index 5f729a6bc..2f1962cee 100644 --- a/src/Sarif/Core/Artifact.cs +++ b/src/Sarif/Core/Artifact.cs @@ -72,7 +72,7 @@ public static Artifact Create( { HashData hashes = hashData ?? HashUtilities.ComputeHashes(filePath); - // The hash utilities will return null data in some text contexts. + // The hash utilities will return null data in some test contexts. if (hashes != null) { artifact.Hashes = new Dictionary diff --git a/src/Sarif/Core/Tool.cs b/src/Sarif/Core/Tool.cs index 0919972ef..38138c58b 100644 --- a/src/Sarif/Core/Tool.cs +++ b/src/Sarif/Core/Tool.cs @@ -64,7 +64,7 @@ internal static string ParseFileVersion(string fileVersion) private static void SetDriverPropertiesFromFileVersionInfo(ToolComponent driver, FileVersionInfo fileVersion) { - if (!string.IsNullOrEmpty(fileVersion.Comments)) { driver.SetProperty("Comments", fileVersion.Comments); } + if (!string.IsNullOrEmpty(fileVersion.Comments)) { driver.SetProperty("comments", fileVersion.Comments); } } /// diff --git a/src/Sarif/HashUtilities.cs b/src/Sarif/HashUtilities.cs index d3fcda95b..ea1440ecd 100644 --- a/src/Sarif/HashUtilities.cs +++ b/src/Sarif/HashUtilities.cs @@ -100,42 +100,53 @@ public static HashData ComputeHashes(string fileName, IFileSystem fileSystem = n // and record exception details. if (stream == null) { return null; } - using (var bufferedStream = new BufferedStream(stream, 1024 * 32)) - { - string md5, sha1, sha256; - byte[] checksum; + return ComputeHashes(stream); + } + } + catch (IOException) { } + catch (UnauthorizedAccessException) { } + return null; + } - using (var md5Cng = MD5.Create()) - { - checksum = md5Cng.ComputeHash(bufferedStream); - md5 = BitConverter.ToString(checksum).Replace("-", string.Empty); - } + public static HashData ComputeHashes(Stream stream) + { + using (var bufferedStream = new BufferedStream(stream, 1024 * 32)) + { + string md5, sha1, sha256; + byte[] checksum; - stream.Seek(0, SeekOrigin.Begin); - bufferedStream.Seek(0, SeekOrigin.Begin); + using (var md5Cng = MD5.Create()) + { + checksum = md5Cng.ComputeHash(bufferedStream); + md5 = BitConverter.ToString(checksum).Replace("-", string.Empty); + } - using (var sha1Cng = SHA1.Create()) - { - checksum = sha1Cng.ComputeHash(bufferedStream); - sha1 = BitConverter.ToString(checksum).Replace("-", string.Empty); - } + stream.Seek(0, SeekOrigin.Begin); + bufferedStream.Seek(0, SeekOrigin.Begin); - stream.Seek(0, SeekOrigin.Begin); - bufferedStream.Seek(0, SeekOrigin.Begin); + using (var sha1Cng = SHA1.Create()) + { + checksum = sha1Cng.ComputeHash(bufferedStream); + sha1 = BitConverter.ToString(checksum).Replace("-", string.Empty); + } - using (var sha256Cng = SHA256.Create()) - { - checksum = sha256Cng.ComputeHash(bufferedStream); - sha256 = BitConverter.ToString(checksum).Replace("-", string.Empty); - } + stream.Seek(0, SeekOrigin.Begin); + bufferedStream.Seek(0, SeekOrigin.Begin); - return new HashData(md5, sha1, sha256); - } + using (var sha256Cng = SHA256.Create()) + { + checksum = sha256Cng.ComputeHash(bufferedStream); + sha256 = BitConverter.ToString(checksum).Replace("-", string.Empty); } + + return new HashData(md5, sha1, sha256); } - catch (IOException) { } - catch (UnauthorizedAccessException) { } - return null; + } + + public static HashData ComputeHashesForText(string text) + { + var stream = new MemoryStream(Encoding.UTF8.GetBytes(text)); + return ComputeHashes(stream); } public static string ComputeSha256Hash(string fileName) diff --git a/src/Sarif/Writers/SarifLogger.cs b/src/Sarif/Writers/SarifLogger.cs index d276b86ff..daef855be 100644 --- a/src/Sarif/Writers/SarifLogger.cs +++ b/src/Sarif/Writers/SarifLogger.cs @@ -259,6 +259,8 @@ private void EnhanceRun(IEnumerable analysisTargets, _run.Invocations.Add(invocation); } + public Func ComputeHashData { get; set; } + public IDictionary AnalysisTargetToHashDataMap { get; } public IDictionary RuleToReportingDescriptorReferenceMap { get; } @@ -505,7 +507,13 @@ private void CaptureArtifact(ArtifactLocation fileLocation) } HashData hashData = null; - AnalysisTargetToHashDataMap?.TryGetValue(fileLocation.Uri.OriginalString, out hashData); + if (AnalysisTargetToHashDataMap == null || + !AnalysisTargetToHashDataMap.TryGetValue(fileLocation.Uri.OriginalString, out hashData) && + ComputeFileHashes && + ComputeHashData != null) + { + hashData = ComputeHashData(fileLocation.Uri); + } // Ensure Artifact is in Run.Artifacts and ArtifactLocation.Index is set to point to it int index = _run.GetFileIndex(fileLocation, From bae5a37f9ae37e563d46ad1e5ab49e01a48c50c6 Mon Sep 17 00:00:00 2001 From: "Michael C. Fanning" Date: Wed, 1 Feb 2023 15:08:28 -0800 Subject: [PATCH 07/15] Update release notes. --- src/ReleaseHistory.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/ReleaseHistory.md b/src/ReleaseHistory.md index 89940c423..c1a65b3da 100644 --- a/src/ReleaseHistory.md +++ b/src/ReleaseHistory.md @@ -34,6 +34,8 @@ * NEW: Add `--normalize-for-ghas` argument to the `rewrite` command to ensure rewritten SARIF is compatible with GitHub Advanced Security (GHAS) ingestion requirements. [#2581](https://github.com/microsoft/sarif-sdk/pull/2581) * NEW: Allow per-line rolling (partial) hash computation for a file. [#2605](https://github.com/microsoft/sarif-sdk/pull/2605) * NEW: `SarifLogger` now supports extensions rules data when logging (by providing a `ToolComponent` instance to the result logging method). [#2661](https://github.com/microsoft/sarif-sdk/pull/2611) +* NEW: `SarifLogger` provides a `ComputeHashData` callback to provide hash data for in-memory scan targets. [#2614](https://github.com/microsoft/sarif-sdk/pull/2614) +* NEW: Provide `HashUtilities.ComputeHashes(Stream)` and `ComputeHashesForText(string) helpers. [#2614](https://github.com/microsoft/sarif-sdk/pull/2614) ## **v3.1.0** [Sdk](https://www.nuget.org/packages/Sarif.Sdk/3.1.0) | [Driver](https://www.nuget.org/packages/Sarif.Driver/3.1.0) | [Converters](https://www.nuget.org/packages/Sarif.Converters/3.1.0) | [Multitool](https://www.nuget.org/packages/Sarif.Multitool/3.1.0) | [Multitool Library](https://www.nuget.org/packages/Sarif.Multitool.Library/3.1.0) * BUG: Loosen `System.Collections.Immutable` minimum version requirement to 1.5.0. [#2504](https://github.com/microsoft/sarif-sdk/pull/2533) From 4369a209d1ac32351efbeca4554716375c96dc58 Mon Sep 17 00:00:00 2001 From: "Michael C. Fanning" Date: Wed, 1 Feb 2023 15:13:20 -0800 Subject: [PATCH 08/15] Fix compute hash function callback logic. --- src/Sarif/Writers/SarifLogger.cs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/Sarif/Writers/SarifLogger.cs b/src/Sarif/Writers/SarifLogger.cs index daef855be..b64c69711 100644 --- a/src/Sarif/Writers/SarifLogger.cs +++ b/src/Sarif/Writers/SarifLogger.cs @@ -507,10 +507,9 @@ private void CaptureArtifact(ArtifactLocation fileLocation) } HashData hashData = null; - if (AnalysisTargetToHashDataMap == null || - !AnalysisTargetToHashDataMap.TryGetValue(fileLocation.Uri.OriginalString, out hashData) && - ComputeFileHashes && - ComputeHashData != null) + if ((AnalysisTargetToHashDataMap == null || + !AnalysisTargetToHashDataMap.TryGetValue(fileLocation.Uri.OriginalString, out hashData)) && + ComputeFileHashes && ComputeHashData != null) { hashData = ComputeHashData(fileLocation.Uri); } From fdb2545d71b0841f253017c6d25b030b78c0f9c9 Mon Sep 17 00:00:00 2001 From: "Michael C. Fanning" Date: Wed, 1 Feb 2023 15:14:39 -0800 Subject: [PATCH 09/15] Formatting changes. --- src/Sarif/Core/Tool.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Sarif/Core/Tool.cs b/src/Sarif/Core/Tool.cs index 38138c58b..4d7d2b22b 100644 --- a/src/Sarif/Core/Tool.cs +++ b/src/Sarif/Core/Tool.cs @@ -43,7 +43,7 @@ public static Tool CreateFromAssemblyData(Assembly assembly = null, { Name = name, FullName = name + " " + (omitSemanticVersion ? null : version.ToString()), - Version = omitSemanticVersion ? null :fileVersion.FileVersion, + Version = omitSemanticVersion ? null : fileVersion.FileVersion, DottedQuadFileVersion = omitSemanticVersion ? null : dottedQuadFileVersion, SemanticVersion = omitSemanticVersion ? null : fileVersion.ProductVersion, Organization = string.IsNullOrEmpty(fileVersion.CompanyName) ? null : fileVersion.CompanyName, From f47084999a0d93710bd18c2a8547b7fa0621e3d3 Mon Sep 17 00:00:00 2001 From: Michael Fanning Date: Sun, 12 Feb 2023 13:47:45 -0800 Subject: [PATCH 10/15] Fix condition where context regions are required for single-line files <512 characters. --- src/Sarif/FileRegionsCache.cs | 26 +++-- .../FileRegionsCacheTests.cs | 101 +++++++++++++++++- 2 files changed, 110 insertions(+), 17 deletions(-) diff --git a/src/Sarif/FileRegionsCache.cs b/src/Sarif/FileRegionsCache.cs index 838eae321..2e206014d 100644 --- a/src/Sarif/FileRegionsCache.cs +++ b/src/Sarif/FileRegionsCache.cs @@ -142,6 +142,9 @@ private Region PopulateTextRegionProperties(NewLineIndex lineIndex, Region input return region; } + internal const int BIGSNIPPETLENGTH = 512; + internal const int SMALLSNIPPETLENGTH = 128; + public Region ConstructMultilineContextSnippet(Region inputRegion, Uri uri, string fileText = null) { if (inputRegion?.IsBinaryRegion != false) @@ -156,11 +159,8 @@ public Region ConstructMultilineContextSnippet(Region inputRegion, Uri uri, stri return null; } - const int bigSnippetLength = 512; - const int smallSnippetLength = 128; - // Generating full inputRegion to prevent issues. - Region originalRegion = this.PopulateTextRegionProperties(inputRegion, uri, populateSnippet: true); + Region originalRegion = this.PopulateTextRegionProperties(inputRegion, uri, populateSnippet: true, fileText); int maxLineNumber = newLineIndex.MaximumLineNumber; @@ -171,10 +171,10 @@ public Region ConstructMultilineContextSnippet(Region inputRegion, Uri uri, stri }; // Generating multilineRegion with one line before and after. - Region multilineContextSnippet = this.PopulateTextRegionProperties(region, uri, populateSnippet: true); + Region multilineContextSnippet = this.PopulateTextRegionProperties(region, uri, populateSnippet: true, fileText); if (originalRegion.CharLength <= multilineContextSnippet.CharLength && - multilineContextSnippet.CharLength <= bigSnippetLength) + multilineContextSnippet.CharLength <= BIGSNIPPETLENGTH) { return multilineContextSnippet; } @@ -184,17 +184,15 @@ public Region ConstructMultilineContextSnippet(Region inputRegion, Uri uri, stri region.EndColumn = 0; region.StartLine = 0; region.EndLine = 0; - region.CharOffset = originalRegion.CharOffset < smallSnippetLength - ? 0 - : originalRegion.CharOffset - smallSnippetLength; + region.CharOffset = Math.Max(0, originalRegion.CharOffset - SMALLSNIPPETLENGTH); - region.CharLength = originalRegion.CharLength + region.CharOffset + smallSnippetLength < newLineIndex.Text.Length - ? originalRegion.CharLength + smallSnippetLength + Math.Abs(region.CharOffset - originalRegion.CharOffset) - : newLineIndex.Text.Length - region.CharOffset; + region.CharLength = + Math.Min(originalRegion.CharLength + region.CharOffset + SMALLSNIPPETLENGTH, + fileText.Length - region.CharOffset); - // Generating multineRegion with 128 characters to the left and right from the + // Generating multiline region with 128 characters to the left and right from the // originalRegion if possible. - multilineContextSnippet = this.PopulateTextRegionProperties(region, uri, populateSnippet: true); + multilineContextSnippet = this.PopulateTextRegionProperties(region, uri, populateSnippet: true, fileText); // We can't generate a contextRegion which is smaller than the original region. Debug.Assert(originalRegion.CharLength <= multilineContextSnippet.CharLength); diff --git a/src/Test.UnitTests.Sarif/FileRegionsCacheTests.cs b/src/Test.UnitTests.Sarif/FileRegionsCacheTests.cs index 744b5ab7a..1c5492d6e 100644 --- a/src/Test.UnitTests.Sarif/FileRegionsCacheTests.cs +++ b/src/Test.UnitTests.Sarif/FileRegionsCacheTests.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.Collections.ObjectModel; using System.Diagnostics; +using System.Runtime.Intrinsics.X86; using System.Text; using System.Threading.Tasks; @@ -411,11 +412,8 @@ public void FileRegionsCache_PopulatesFromMissingFile() [Fact] public void FileRegionsCache_PopulatesUsingProvidedText() { - var run = new Run(); var fileRegionsCache = new FileRegionsCache(); - Uri uri = new Uri(@"c:\temp\DoesNotExist\" + Guid.NewGuid().ToString() + ".cpp"); - string fileText = "12345\n56790\n"; int charOffset = 6; int charLength = 1; @@ -447,6 +445,103 @@ public void FileRegionsCache_PopulatesUsingProvidedText() actual.ValueEquals(expected).Should().BeTrue(); } + [Fact] + public void FileRegionsCache_PopulatesContextRegions() + { + string sentinel = "baz"; + char padding = 'a'; + + // The context region populating API has two special values, 128 characters + // (which are used to generate leading and trailing text in single-line + // text files) and a value of 512', which is intended to be a limit + // on the overall size of the snippet that's returned. + + int bsl = FileRegionsCache.BIGSNIPPETLENGTH; + int ssl = FileRegionsCache.SMALLSNIPPETLENGTH; + + string[] tests = + { + $"{new string(padding, 0)}{sentinel}{new string(padding, 0)}", + $"{new string(padding, 0)}{sentinel}{new string(padding, 3)}", + $"{new string(padding, 3)}{sentinel}{new string(padding, 0)}", + $"{new string(padding, 3)}{sentinel}{new string(padding, 3)}", + $"{new string(padding, ssl)}{sentinel}{new string(padding, ssl)}", + $"{new string(padding, 1)}{sentinel}{new string(padding, ssl)}", + $"{new string(padding, ssl)}{sentinel}{new string(padding, 1)}", + $"{new string(padding, ssl)}{sentinel}{new string(padding, 0)}", + $"{new string(padding, 0)}{sentinel}{new string(padding, ssl)}", + $"{new string(padding, 0)}{sentinel}{new string(padding, 0)}", + $"{new string(padding, bsl)}{sentinel}{new string(padding, bsl)}", + $"{new string(padding, 10)}{sentinel}{new string(padding, bsl)}", + $"{new string(padding, bsl)}{sentinel}{new string(padding, 10)}", + $"{new string(padding, bsl)}{sentinel}{new string(padding, 0)}", + $"{new string(padding, 0)}{sentinel}{new string(padding, bsl)}", + $"{new string(padding, 0)}{sentinel}{new string(padding, 0)}", + }; + + var context = new StringBuilder(); + int iteration = 0; + + // DEBUGGING THESE TESTS: these tests do not accumulate all outputs and report + // them, instead they break on the first failure. A failure will report a + // message like so: + // + // Expected contextRegion.Snippet not to be because 'baz' snippet exists + // (iteration 12, while processing char-based region type for value 'abaza'). + // + // Observe the iteration value (in this case 12). Set a conditional breakpoint + // below when the iteration variable equals this value and you can debug the + // relevant failure. + + foreach (string test in tests) + { + var cache = new FileRegionsCache(); + var uri = new Uri(@"c:\temp\DoesNotExist\" + Guid.NewGuid().ToString() + ".cpp"); + + int index = test.IndexOf(sentinel); + + // The FileRegions code takes two discrete code paths depending on whether + // the input variable is char-offset based or uses the start line convention. + var regions = new Region[] + { + new Region + { + CharOffset = index, + CharLength = sentinel.Length, + }, + new Region + { + StartLine = 1, + StartColumn = index + 1, + EndColumn = index + sentinel.Length + 1, + } + }; + + string charBased = "char-based"; + string lineBased = "line-based"; + + foreach (Region region in regions) + { + context.Clear(); + context.Append($"(iteration {iteration}, while processing {(region.StartLine == 1 ? lineBased : charBased)} region type for value '{test}')"); + + // First, we populate the region and text snippet for the actual test finding. + Region actual = cache.PopulateTextRegionProperties(region, uri, populateSnippet: true, test); + + actual.Snippet.Should().NotBeNull($"'{sentinel}' snippet exists {context}"); + actual.Snippet.Text?.Should().Be($"{sentinel}", $"region snippet did not match {context}"); + + // Now, we attempt to produce a context region. + Region contextRegion = cache.ConstructMultilineContextSnippet(actual, uri, test); + contextRegion.Snippet.Should().NotBeNull($"'{sentinel}' snippet exists {context}"); + contextRegion.Snippet.Text.Contains(sentinel).Should().BeTrue($"context region should encapsulate finding {context}"); + } + + iteration++; + } + } + + [Fact] public void FileRegionsCache_PopulatesSpecExampleRegions() { From 09a417c6fe913850372542ea027af03f7ffa991d Mon Sep 17 00:00:00 2001 From: Michael Fanning Date: Sun, 12 Feb 2023 14:15:08 -0800 Subject: [PATCH 11/15] All tests pass. --- src/Sarif/FileRegionsCache.cs | 2 + .../FileRegionsCacheTests.cs | 154 ++++++++++-------- 2 files changed, 91 insertions(+), 65 deletions(-) diff --git a/src/Sarif/FileRegionsCache.cs b/src/Sarif/FileRegionsCache.cs index 2e206014d..243197220 100644 --- a/src/Sarif/FileRegionsCache.cs +++ b/src/Sarif/FileRegionsCache.cs @@ -159,6 +159,8 @@ public Region ConstructMultilineContextSnippet(Region inputRegion, Uri uri, stri return null; } + fileText ??= newLineIndex.Text; + // Generating full inputRegion to prevent issues. Region originalRegion = this.PopulateTextRegionProperties(inputRegion, uri, populateSnippet: true, fileText); diff --git a/src/Test.UnitTests.Sarif/FileRegionsCacheTests.cs b/src/Test.UnitTests.Sarif/FileRegionsCacheTests.cs index 1c5492d6e..092e7d6e8 100644 --- a/src/Test.UnitTests.Sarif/FileRegionsCacheTests.cs +++ b/src/Test.UnitTests.Sarif/FileRegionsCacheTests.cs @@ -459,51 +459,73 @@ public void FileRegionsCache_PopulatesContextRegions() int bsl = FileRegionsCache.BIGSNIPPETLENGTH; int ssl = FileRegionsCache.SMALLSNIPPETLENGTH; - string[] tests = - { - $"{new string(padding, 0)}{sentinel}{new string(padding, 0)}", - $"{new string(padding, 0)}{sentinel}{new string(padding, 3)}", - $"{new string(padding, 3)}{sentinel}{new string(padding, 0)}", - $"{new string(padding, 3)}{sentinel}{new string(padding, 3)}", - $"{new string(padding, ssl)}{sentinel}{new string(padding, ssl)}", - $"{new string(padding, 1)}{sentinel}{new string(padding, ssl)}", - $"{new string(padding, ssl)}{sentinel}{new string(padding, 1)}", - $"{new string(padding, ssl)}{sentinel}{new string(padding, 0)}", - $"{new string(padding, 0)}{sentinel}{new string(padding, ssl)}", - $"{new string(padding, 0)}{sentinel}{new string(padding, 0)}", - $"{new string(padding, bsl)}{sentinel}{new string(padding, bsl)}", - $"{new string(padding, 10)}{sentinel}{new string(padding, bsl)}", - $"{new string(padding, bsl)}{sentinel}{new string(padding, 10)}", - $"{new string(padding, bsl)}{sentinel}{new string(padding, 0)}", - $"{new string(padding, 0)}{sentinel}{new string(padding, bsl)}", - $"{new string(padding, 0)}{sentinel}{new string(padding, 0)}", - }; - - var context = new StringBuilder(); - int iteration = 0; - - // DEBUGGING THESE TESTS: these tests do not accumulate all outputs and report - // them, instead they break on the first failure. A failure will report a - // message like so: - // - // Expected contextRegion.Snippet not to be because 'baz' snippet exists - // (iteration 12, while processing char-based region type for value 'abaza'). - // - // Observe the iteration value (in this case 12). Set a conditional breakpoint - // below when the iteration variable equals this value and you can debug the - // relevant failure. - - foreach (string test in tests) + // Prepend a newline (or not!) in front of every sentinel region. + foreach (string pr in new string[] { null, Environment.NewLine }) { - var cache = new FileRegionsCache(); - var uri = new Uri(@"c:\temp\DoesNotExist\" + Guid.NewGuid().ToString() + ".cpp"); + // Post-fix a newline (or not!) in front of every sentinel region. + foreach (string po in new string[] { null, Environment.NewLine }) + { + string[] tests = + { + $"{new string(padding, 0)}{pr}{sentinel}{po}{new string(padding, 0)}", + $"{new string(padding, 0)}{pr}{sentinel}{po}{new string(padding, 3)}", + $"{new string(padding, 3)}{pr}{sentinel}{po}{new string(padding, 0)}", + $"{new string(padding, 3)}{pr}{sentinel}{po}{new string(padding, 3)}", + $"{new string(padding, ssl)}{pr}{sentinel}{po}{new string(padding, ssl)}", + $"{new string(padding, 1)}{pr}{sentinel}{po}{new string(padding, ssl)}", + $"{new string(padding, ssl)}{pr}{sentinel}{po}{new string(padding, 1)}", + $"{new string(padding, ssl)}{pr}{sentinel}{po}{new string(padding, 0)}", + $"{new string(padding, 0)}{pr}{sentinel}{po}{new string(padding, ssl)}", + $"{new string(padding, 0)}{pr}{sentinel}{po}{new string(padding, 0)}", + $"{new string(padding, bsl)}{pr}{sentinel}{po}{new string(padding, bsl)}", + $"{new string(padding, 10)}{pr}{sentinel}{po}{new string(padding, bsl)}", + $"{new string(padding, bsl)}{pr}{sentinel}{po}{new string(padding, 10)}", + $"{new string(padding, bsl)}{pr}{sentinel}{po}{new string(padding, 0)}", + $"{new string(padding, 0)}{pr}{sentinel}{po}{new string(padding, bsl)}", + $"{new string(padding, 0)}{pr}{sentinel}{po}{new string(padding, 0)}", + $"{new string(padding, 0)}{pr}{sentinel}{po}{new string(padding, 0)}", + $"{new string(padding, 0)}{pr}{sentinel}{po}{new string(padding, 3)}", + $"{new string(padding, 3)}{pr}{sentinel}{po}{new string(padding, 0)}", + $"{new string(padding, 3)}{pr}{sentinel}{po}{new string(padding, 3)}", + $"{new string(padding, ssl)}{pr}{sentinel}{po}{new string(padding, ssl)}", + $"{new string(padding, 1)}{pr}{sentinel}{po}{new string(padding, ssl)}", + $"{new string(padding, ssl)}{pr}{sentinel}{po}{new string(padding, 1)}", + $"{new string(padding, ssl)}{pr}{sentinel}{po}{new string(padding, 0)}", + $"{new string(padding, 0)}{pr}{sentinel}{po}{new string(padding, ssl)}", + $"{new string(padding, 0)}{pr}{sentinel}{po}{new string(padding, 0)}", + $"{new string(padding, bsl)}{pr}{sentinel}{po}{new string(padding, bsl)}", + $"{new string(padding, 10)}{pr}{sentinel}{po}{new string(padding, bsl)}", + $"{new string(padding, bsl)}{pr}{sentinel}{po}{new string(padding, 10)}", + $"{new string(padding, bsl)}{pr}{sentinel}{po}{new string(padding, 0)}", + $"{new string(padding, 0)}{pr}{sentinel}{po}{new string(padding, bsl)}", + $"{new string(padding, 0)}{pr}{sentinel}{po}{new string(padding, 0)}", + }; + + var context = new StringBuilder(); + int iteration = 0; + + // DEBUGGING THESE TESTS: these tests do not accumulate all outputs and report + // them, instead they break on the first failure. A failure will report a + // message like so: + // + // Expected contextRegion.Snippet not to be because 'baz' snippet exists + // (iteration 12, while processing char-based region type for value 'abaza'). + // + // Observe the iteration value (in this case 12). Set a conditional breakpoint + // below when the iteration variable equals this value and you can debug the + // relevant failure. + + foreach (string test in tests) + { + var cache = new FileRegionsCache(); + var uri = new Uri(@"c:\temp\DoesNotExist\" + Guid.NewGuid().ToString() + ".cpp"); - int index = test.IndexOf(sentinel); + int index = test.IndexOf(sentinel); - // The FileRegions code takes two discrete code paths depending on whether - // the input variable is char-offset based or uses the start line convention. - var regions = new Region[] - { + // The FileRegions code takes two discrete code paths depending on whether + // the input variable is char-offset based or uses the start line convention. + var regions = new Region[] + { new Region { CharOffset = index, @@ -515,29 +537,31 @@ public void FileRegionsCache_PopulatesContextRegions() StartColumn = index + 1, EndColumn = index + sentinel.Length + 1, } - }; + }; - string charBased = "char-based"; - string lineBased = "line-based"; + string charBased = "char-based"; + string lineBased = "line-based"; - foreach (Region region in regions) - { - context.Clear(); - context.Append($"(iteration {iteration}, while processing {(region.StartLine == 1 ? lineBased : charBased)} region type for value '{test}')"); - - // First, we populate the region and text snippet for the actual test finding. - Region actual = cache.PopulateTextRegionProperties(region, uri, populateSnippet: true, test); - - actual.Snippet.Should().NotBeNull($"'{sentinel}' snippet exists {context}"); - actual.Snippet.Text?.Should().Be($"{sentinel}", $"region snippet did not match {context}"); - - // Now, we attempt to produce a context region. - Region contextRegion = cache.ConstructMultilineContextSnippet(actual, uri, test); - contextRegion.Snippet.Should().NotBeNull($"'{sentinel}' snippet exists {context}"); - contextRegion.Snippet.Text.Contains(sentinel).Should().BeTrue($"context region should encapsulate finding {context}"); - } + foreach (Region region in regions) + { + context.Clear(); + context.Append($"(iteration {iteration}, while processing {(region.StartLine == 1 ? lineBased : charBased)} region type for value '{test}')"); + + // First, we populate the region and text snippet for the actual test finding. + Region actual = cache.PopulateTextRegionProperties(region, uri, populateSnippet: true, test); + + actual.Snippet.Should().NotBeNull($"'{sentinel}' snippet exists {context}"); + actual.Snippet.Text?.Should().Be($"{sentinel}", $"region snippet did not match {context}"); - iteration++; + // Now, we attempt to produce a context region. + Region contextRegion = cache.ConstructMultilineContextSnippet(actual, uri, test); + contextRegion.Snippet.Should().NotBeNull($"'{sentinel}' snippet exists {context}"); + contextRegion.Snippet.Text.Contains(sentinel).Should().BeTrue($"context region should encapsulate finding {context}"); + } + + iteration++; + } + } } } @@ -690,14 +714,14 @@ public void FileRegionsCache_IncreasingToLeftAndRight() CharOffset = 114, CharLength = 600, }; - + var fileRegionsCache = new FileRegionsCache(); region = fileRegionsCache.PopulateTextRegionProperties(region, uri, true, fileContent); Region multilineRegion = fileRegionsCache.ConstructMultilineContextSnippet(region, uri); - // 114 (charoffset) + 600 (charlength) + 128 (grabbing right content) - multilineRegion.CharLength.Should().Be(114 + 600 + 128); + // 114 (charoffset) + 600 (charlength) + left-side + remainder of 128 chars. + multilineRegion.CharLength.Should().Be(114 + 600 + (128 - 114)); } [Fact] From 520eb131776fa427eb69453bebbcc7088bde10af Mon Sep 17 00:00:00 2001 From: "Michael C. Fanning" Date: Mon, 13 Feb 2023 09:23:38 -0800 Subject: [PATCH 12/15] Update test comment. --- src/Test.UnitTests.Sarif/FileRegionsCacheTests.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Test.UnitTests.Sarif/FileRegionsCacheTests.cs b/src/Test.UnitTests.Sarif/FileRegionsCacheTests.cs index 092e7d6e8..19480e435 100644 --- a/src/Test.UnitTests.Sarif/FileRegionsCacheTests.cs +++ b/src/Test.UnitTests.Sarif/FileRegionsCacheTests.cs @@ -714,14 +714,14 @@ public void FileRegionsCache_IncreasingToLeftAndRight() CharOffset = 114, CharLength = 600, }; - + var fileRegionsCache = new FileRegionsCache(); region = fileRegionsCache.PopulateTextRegionProperties(region, uri, true, fileContent); Region multilineRegion = fileRegionsCache.ConstructMultilineContextSnippet(region, uri); - // 114 (charoffset) + 600 (charlength) + left-side + remainder of 128 chars. - multilineRegion.CharLength.Should().Be(114 + 600 + (128 - 114)); + // 114 (charoffset) + 600 (charlength) + left-side + remainder of file. + multilineRegion.CharLength.Should().Be(fileContent.Length); } [Fact] From c70851b9e176a2ba9841e71bc79ce84f13b51e96 Mon Sep 17 00:00:00 2001 From: "Michael C. Fanning" Date: Mon, 13 Feb 2023 09:30:57 -0800 Subject: [PATCH 13/15] CodeQL advice taken. --- src/Test.UnitTests.Sarif/FileRegionsCacheTests.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Test.UnitTests.Sarif/FileRegionsCacheTests.cs b/src/Test.UnitTests.Sarif/FileRegionsCacheTests.cs index 19480e435..8d2700c19 100644 --- a/src/Test.UnitTests.Sarif/FileRegionsCacheTests.cs +++ b/src/Test.UnitTests.Sarif/FileRegionsCacheTests.cs @@ -459,6 +459,8 @@ public void FileRegionsCache_PopulatesContextRegions() int bsl = FileRegionsCache.BIGSNIPPETLENGTH; int ssl = FileRegionsCache.SMALLSNIPPETLENGTH; + var context = new StringBuilder(); + // Prepend a newline (or not!) in front of every sentinel region. foreach (string pr in new string[] { null, Environment.NewLine }) { @@ -501,7 +503,7 @@ public void FileRegionsCache_PopulatesContextRegions() $"{new string(padding, 0)}{pr}{sentinel}{po}{new string(padding, 0)}", }; - var context = new StringBuilder(); + context.Clear(); int iteration = 0; // DEBUGGING THESE TESTS: these tests do not accumulate all outputs and report From 44f8e58bd7def9f7e942e9406f77c6a8f6e31365 Mon Sep 17 00:00:00 2001 From: "Michael C. Fanning" Date: Mon, 13 Feb 2023 10:45:50 -0800 Subject: [PATCH 14/15] Fix CodeQL alert. --- src/Test.UnitTests.Sarif/FileRegionsCacheTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Test.UnitTests.Sarif/FileRegionsCacheTests.cs b/src/Test.UnitTests.Sarif/FileRegionsCacheTests.cs index 8d2700c19..d69c77ae2 100644 --- a/src/Test.UnitTests.Sarif/FileRegionsCacheTests.cs +++ b/src/Test.UnitTests.Sarif/FileRegionsCacheTests.cs @@ -520,7 +520,7 @@ public void FileRegionsCache_PopulatesContextRegions() foreach (string test in tests) { var cache = new FileRegionsCache(); - var uri = new Uri(@"c:\temp\DoesNotExist\" + Guid.NewGuid().ToString() + ".cpp"); + var uri = new Uri(@$"c:\temp\DoesNotExist\{Guid.NewGuid()}.cpp"); int index = test.IndexOf(sentinel); From 2d48491c6eb14c8a349adeaf99072cbc45f53928 Mon Sep 17 00:00:00 2001 From: "Michael C. Fanning" Date: Mon, 13 Feb 2023 10:52:21 -0800 Subject: [PATCH 15/15] Restore test and correct comment. --- src/Test.UnitTests.Sarif/FileRegionsCacheTests.cs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/Test.UnitTests.Sarif/FileRegionsCacheTests.cs b/src/Test.UnitTests.Sarif/FileRegionsCacheTests.cs index d69c77ae2..f7acdf3ec 100644 --- a/src/Test.UnitTests.Sarif/FileRegionsCacheTests.cs +++ b/src/Test.UnitTests.Sarif/FileRegionsCacheTests.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Collections.ObjectModel; +using System.ComponentModel.DataAnnotations; using System.Diagnostics; using System.Runtime.Intrinsics.X86; using System.Text; @@ -722,8 +723,11 @@ public void FileRegionsCache_IncreasingToLeftAndRight() Region multilineRegion = fileRegionsCache.ConstructMultilineContextSnippet(region, uri); - // 114 (charoffset) + 600 (charlength) + left-side + remainder of file. - multilineRegion.CharLength.Should().Be(fileContent.Length); + // 114 (charoffset) + 600 (charlength) + (128 - prefixed length). + + // The length of our prepended data; + int prefixed = Math.Max(multilineRegion.CharOffset - 128, 0); + multilineRegion.CharLength.Should().Be(prefixed + region.CharLength + (128 - prefixed)); } [Fact]