Skip to content

Commit

Permalink
Fix missing command line id from compiler events (#652)
Browse files Browse the repository at this point in the history 
* Adjusted event order and updated test to prevent regression.

* Updated release history.

* Updated comment.

* Adding string constants for testing.

* Addressed additional PR feedback.

* Removed extra whitespace, added suggested validations, and cleaned up variable usage.

* Fix whitespace.
  • Loading branch information
@marmegh
marmegh authored Jun 15, 2022
1 parent dcafdde commit c4a8b86
Show file tree
Hide file tree
Showing 3 changed files with 181 additions and 10 deletions.
20 changes: 14 additions & 6 deletions src/BinSkim.Sdk/CompilerDataLogger.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,19 +48,25 @@ public class CompilerDataLogger : IDisposable
internal const string ProjectId = "projectId";
internal const string SymbolPath = "symbolPath";
internal const string BuildRunId = "buildRunId";
internal const string CommandLine = "commandLine";
internal const string ProjectName = "projectName";
internal const string ToolVersion = "toolVersion";
internal const string TimeConsumed = "timeConsumed";
internal const string RepositoryId = "repositoryId";
internal const string CommandLineId = "commandLineId";
internal const string RepositoryName = "repositoryName";
internal const string OrganizationId = "organizationId";
internal const string NormalizedPath = "normalizedPath";
internal const string AnalysisEndTime = "analysisEndTime";
internal const string OrganizationName = "organizationName";
internal const string AnalysisStartTime = "analysisStartTime";
internal const string BuildDefinitionId = "buildDefinitionId";
internal const string AssemblyReferences = "assemblyReferences";
internal const string ChunkedCommandLine = "chunkedcommandLine";
internal const string BuildDefinitionName = "buildDefinitionName";
internal const string AssemblyReferencesId = "assemblyReferencesId";
internal const string NumberOfBinaryAnalyzed = "numberOfBinaryAnalyzed";
internal const string ChunkedAssemblyReferences = "chunkedassemblyReferences";

// This object is required to synchronize multi-threaded writes
// to the CSV writer only. The AppInsights client is already
Expand Down Expand Up @@ -260,29 +266,31 @@ public void Write(BinaryAnalyzerContext context, CompilerData compilerData)
{ "error", string.Empty }
};

this.telemetryClient.TrackEvent(CompilerEventName, properties: properties);

if (!string.IsNullOrWhiteSpace(compilerData.CommandLine))
{
string commandLineId = Guid.NewGuid().ToString();
properties.Add("commandLineId", commandLineId);
properties.Add(CommandLineId, commandLineId);

SendChunkedContent(CommandLineEventName,
commandLineId,
"commandLine",
CommandLine,
compilerData.CommandLine);
}

if (!string.IsNullOrWhiteSpace(compilerData.AssemblyReferences))
{
string assemblyReferencesId = Guid.NewGuid().ToString();
properties.Add("assemblyReferencesId", assemblyReferencesId);
properties.Add(AssemblyReferencesId, assemblyReferencesId);

SendChunkedContent(AssemblyReferencesEventName,
assemblyReferencesId,
"assemblyReferences",
AssemblyReferences,
compilerData.AssemblyReferences);
}

// To prevent loss of relationship between compiler and assembly/commandline data
// this event cannot be sent before AssemblyReferences or CommandLine.
this.telemetryClient.TrackEvent(CompilerEventName, properties: properties);
}

public void WriteException(BinaryAnalyzerContext context, string errorMessage)
Expand Down
1 change: 1 addition & 0 deletions src/ReleaseHistory.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
* FEATURE: Enable BinSkim for MacOS. [#576](https://github.com/microsoft/binskim/pull/576)
* Bump Sarif.Sdk by updating submodule from [4e9f606 to fc9a9df](https://github.com/microsoft/sarif-sdk/compare/4e9f606bb0e88428866e253352cdc70dc68f98cb...fc9a9dfb865096b5aaa9fa3651854670940f7459). [#638](https://github.com/microsoft/binskim/pull/638)
* FALSE POSITIVE FIX: Skip `BA2025.EnableShadowStack` rule for ARM Binaries which cannot use `/CETCOMPAT`. [#650](https://github.com/microsoft/binskim/pull/650)
* BUGFIX: Fix missing `commandLineId` from `CommandLineInformation` event. [#652](https://github.com/microsoft/binskim/pull/652)

## **v1.9.4** [NuGet Package](https://www.nuget.org/packages/Microsoft.CodeAnalysis.BinSkim/1.9.4)

Expand Down
170 changes: 166 additions & 4 deletions src/Test.UnitTests.BinSkim.Driver/CompilerDataLoggerTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,16 @@ public void CompilerDataLogger_Write_ShouldSendAssemblyReferencesInChunks_WhenTe

string assemblies = "Microsoft.DiaSymReader (1.3.0);Newtonsoft.Json (13.0.1)";
CompilerDataLogger.s_chunkSize = SmallChunkSize;
int chunkNumber = logger.CalculateChunkedContentSize(assemblies.Length);
int expectedChunkCount = logger.CalculateChunkedContentSize(assemblies.Length);
logger.Write(context, new CompilerData { CompilerName = ".NET Compiler", AssemblyReferences = assemblies });
telemetryEventOutput.Count.Should().Be(chunkNumber + 1);

StringBuilder sb = ValidateChunkedEvents(
expectedEventName: CompilerDataLogger.AssemblyReferencesEventName,
telemetryGeneratedEvents: telemetryEventOutput,
expectedChunkedContent: assemblies,
chunkCount: expectedChunkCount);

Assert.Equal(string.Empty, sb.ToString());
}

[Fact]
Expand All @@ -58,11 +65,13 @@ public void CompilerDataLogger_Write_ShouldSendCommandLineDataInChunks_WhenTelem

string commandLine = "TestCommandLine";
CompilerDataLogger.s_chunkSize = SmallChunkSize;
int chunksize = CompilerDataLogger.s_chunkSize;
int chunkNumber = logger.CalculateChunkedContentSize(commandLine.Length);
CompilerData compilerData = new CompilerData { CompilerName = ".NET Compiler", CommandLine = commandLine };
logger.Write(context, compilerData);
telemetryEventOutput.Count.Should().Be(chunkNumber + 1);

StringBuilder sb = ValidateChunkedEvents(CompilerDataLogger.CommandLineEventName, telemetryEventOutput, commandLine, chunkNumber);

Assert.Equal(string.Empty, sb.ToString());
}

[Fact]
Expand Down Expand Up @@ -622,6 +631,154 @@ private StringBuilder ValidateExceptionEvents(string csvOutputPath,
return sb;
}

private StringBuilder ValidateChunkedEvents(string expectedEventName,
List<ITelemetry> telemetryGeneratedEvents,
string expectedChunkedContent,
int chunkCount)
{
var sb = new StringBuilder();
string assemblyEventsId = string.Empty;
string commandLineEventsId = string.Empty;

var compilerEvents = new List<EventTelemetry>();
var assemblyReferencesEvents = new List<EventTelemetry>();
var commandLineEvents = new List<EventTelemetry>();
var summaryEvents = new List<EventTelemetry>();

foreach (EventTelemetry telemetryEvent in telemetryGeneratedEvents)
{
IDictionary<string, string> properties = telemetryEvent.Properties;

if (string.IsNullOrWhiteSpace(commandLineEventsId)
&& properties.TryGetValue(CompilerDataLogger.CommandLineId, out string commandLineEventId))
{
commandLineEventsId = commandLineEventId;
}

if (string.IsNullOrWhiteSpace(assemblyEventsId)
&& properties.TryGetValue(CompilerDataLogger.AssemblyReferencesId, out string assemblyEventId))
{
assemblyEventsId = assemblyEventId;
}

switch (telemetryEvent.Name)
{
case CompilerDataLogger.CompilerEventName:
{
compilerEvents.Add(telemetryEvent);

if (expectedEventName == CompilerDataLogger.AssemblyReferencesEventName)
{
if (!properties.TryGetValue(CompilerDataLogger.AssemblyReferencesId, out string compilerAssemblyReferencesId))
{
sb.AppendLine("Compiler Event is missing the `assemblyReferencesId`");
}
else if (assemblyEventsId != compilerAssemblyReferencesId)
{
sb.AppendLine(
$"`{CompilerDataLogger.CompilerEventName}` event detected with unexpected Id. " +
$"Expected `{assemblyEventsId}` but found `{compilerAssemblyReferencesId}`.");
}
}
else if (expectedEventName == CompilerDataLogger.CommandLineEventName)
{
if (!properties.TryGetValue(CompilerDataLogger.CommandLineId, out string compilerCommandLineEventId))
{
sb.AppendLine("Compiler Event is missing the `commandLineId`");
}
else if (commandLineEventsId != compilerCommandLineEventId)
{
sb.AppendLine(
$"`{CompilerDataLogger.CommandLineEventName}` event detected with unexpected Id. " +
$"Expected `{commandLineEventsId}` but found `{compilerCommandLineEventId}`.");
}
}
break;
}
case CompilerDataLogger.AssemblyReferencesEventName:
{
assemblyReferencesEvents.Add(telemetryEvent);

if (expectedEventName == CompilerDataLogger.AssemblyReferencesEventName
&& !expectedChunkedContent.Contains(properties[CompilerDataLogger.ChunkedAssemblyReferences]))
{
sb.AppendLine(
$"Unexpected `{CompilerDataLogger.AssemblyReferencesEventName}` chunked content: " +
$"`{properties[CompilerDataLogger.ChunkedAssemblyReferences]}` " +
$"expected: `{expectedChunkedContent}`");
}

string currentAssemblyEventId = properties[CompilerDataLogger.AssemblyReferencesId];

if (assemblyEventsId != currentAssemblyEventId)
{
sb.AppendLine(
$"`{CompilerDataLogger.AssemblyReferencesEventName}` event detected with unexpected Id. " +
$"Expected `{assemblyEventsId}` but found `{currentAssemblyEventId}`.");
}

break;
}
case CompilerDataLogger.CommandLineEventName:
{
commandLineEvents.Add(telemetryEvent);

if (expectedEventName == CompilerDataLogger.CommandLineEventName
&& !expectedChunkedContent.Contains(properties["chunkedcommandLine"]))
{
sb.AppendLine(
$"Unexpected `{CompilerDataLogger.CommandLineEventName}` chunked content: " +
$"`{properties[CompilerDataLogger.ChunkedCommandLine]}` " +
$"expected: `{expectedChunkedContent}`");
}

string currentCommandLineEventId = properties["commandLineId"];

if (commandLineEventsId != currentCommandLineEventId)
{
sb.AppendLine(
$"`{CompilerDataLogger.CommandLineEventName}` event detected with unexpected Id. " +
$"Expected `{commandLineEventsId}` but found `{currentCommandLineEventId}`.");
}
break;
}
case CompilerDataLogger.SummaryEventName:
{
summaryEvents.Add(telemetryEvent);
break;
}
}
}

if ((expectedEventName == CompilerDataLogger.AssemblyReferencesEventName
&& assemblyReferencesEvents.Count != chunkCount) ||
(expectedEventName == CompilerDataLogger.CommandLineEventName
&& commandLineEvents.Count != chunkCount))
{
sb.AppendLine(
$"Expected `{chunkCount}` `{expectedEventName}` events, " +
$"but `{assemblyReferencesEvents.Count}` were found");
}

return sb;
}

private StringBuilder ValidateChunkedContent(StringBuilder sb,
int expectedChunkSize,
List<EventTelemetry> chunkedEvents)
{
if (chunkedEvents.Count != expectedChunkSize)
{
sb.AppendLine(
string.Format(
"Incorrect number of chunkedEvents de tected. Expected {0}, but found {1}",
expectedChunkSize,
chunkedEvents.Count));
}

return sb;
}

public static string GetExampleSarifPath(Sarif.SarifVersion sarifVersion)
{
return sarifVersion == Sarif.SarifVersion.Current
Expand All @@ -638,5 +795,10 @@ internal static string GetTestDirectory(string relativeDirectory)
dirPath = Path.GetFullPath(dirPath);
return Path.Combine(dirPath, relativeDirectory);
}

internal static int CalculateChunkedContentSize(int contentLength)
{
return (int)Math.Ceiling(1.0 * contentLength / CompilerDataLogger.s_chunkSize);
}
}
}

0 comments on commit c4a8b86

Please sign in to comment.