Skip to content

Commit

Permalink
Merge pull request #2957 from sywhang/dev/suwhang/fix-logging-event-s…
Browse files Browse the repository at this point in the history 
…ource

Fix LoggingEventSource to handle null strings
  • Loading branch information
@sywhang
sywhang authored Mar 10, 2020
2 parents 59d48e0 + 6171c14 commit 8d0fa05
Show file tree
Hide file tree
Showing 3 changed files with 112 additions and 10 deletions.
29 changes: 19 additions & 10 deletions src/Logging/Logging.EventSource/src/LoggingEventSource.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,10 @@ internal unsafe void FormattedMessage(LogLevel Level, int FactoryID, string Logg
{
if (IsEnabled())
{
LoggerName ??= "";
EventName ??= "";
FormattedMessage ??= "";

fixed (char* loggerName = LoggerName)
fixed (char* eventName = EventName)
fixed (char* formattedMessage = FormattedMessage)
Expand Down Expand Up @@ -175,6 +179,8 @@ internal unsafe void ActivityStop(int ID, int FactoryID, string LoggerName)
{
if (IsEnabled())
{
LoggerName ??= "";

fixed (char* loggerName = LoggerName)
{
const int eventDataCount = 3;
Expand All @@ -194,6 +200,11 @@ internal unsafe void MessageJson(LogLevel Level, int FactoryID, string LoggerNam
{
if (IsEnabled())
{
LoggerName ??= "";
EventName ??= "";
ExceptionJson ??= "";
ArgumentsJson ??= "";

fixed (char* loggerName = LoggerName)
fixed (char* eventName = EventName)
fixed (char* exceptionJson = ExceptionJson)
Expand All @@ -220,6 +231,9 @@ internal unsafe void ActivityJsonStart(int ID, int FactoryID, string LoggerName,
{
if (IsEnabled())
{
LoggerName ??= "";
ArgumentsJson ??= "";

fixed (char* loggerName = LoggerName)
fixed (char* argumentsJson = ArgumentsJson)
{
Expand All @@ -241,6 +255,8 @@ internal unsafe void ActivityJsonStop(int ID, int FactoryID, string LoggerName)
{
if (IsEnabled())
{
LoggerName ??= "";

fixed (char* loggerName = LoggerName)
{
const int eventDataCount = 3;
Expand Down Expand Up @@ -458,16 +474,9 @@ private static unsafe void SetEventData<T>(ref EventData eventData, ref T value,
}
#endif

if (pinnedString != null)
{
eventData.DataPointer = (IntPtr)pinnedString;
eventData.Size = checked((str.Length + 1) * sizeof(char)); // size is specified in bytes, including null wide char
}
else
{
eventData.DataPointer = IntPtr.Zero;
eventData.Size = 0;
}
eventData.DataPointer = (IntPtr)pinnedString;
eventData.Size = checked((str.Length + 1) * sizeof(char)); // size is specified in bytes, including null wide char

}
else
{
Expand Down
4 changes: 4 additions & 0 deletions src/Logging/Logging.EventSource/src/Microsoft.Extensions.Logging.EventSource.csproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,8 @@
<Reference Include="System.Threading.Tasks.Extensions" />
</ItemGroup>

<ItemGroup>
<InternalsVisibleTo Include="Microsoft.Extensions.Logging.EventSource.Tests" />
</ItemGroup>

</Project>
89 changes: 89 additions & 0 deletions src/Logging/Logging.EventSource/test/EventSourceLoggerTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,95 @@ public void Logs_Nothing_AfterDispose()
}
}

[Fact]
public void Logs_AsExpected_FormattedMessage_WithNullString()
{
using (var testListener = new TestEventListener())
{
var factory = CreateLoggerFactory();

var listenerSettings = new TestEventListener.ListenerSettings();
listenerSettings.Keywords = LoggingEventSource.Keywords.FormattedMessage;
listenerSettings.FilterSpec = null;
listenerSettings.Level = EventLevel.Verbose;
testListener.EnableEvents(listenerSettings);

LogStuff(factory);

var containsNullEventName = false;

foreach (var eventJson in testListener.Events)
{
if (eventJson.Contains(@"""__EVENT_NAME"":""FormattedMessage""") && eventJson.Contains(@"""EventName"":"""","))
{
containsNullEventName = true;
}
}

Assert.True(containsNullEventName, "EventName is supposed to be null but it isn't.");
}
}

[Fact]
public void Logs_AsExpected_MessageJson_WithNullString()
{
using (var testListener = new TestEventListener())
{
var listenerSettings = new TestEventListener.ListenerSettings();
listenerSettings.Keywords = LoggingEventSource.Keywords.JsonMessage;
listenerSettings.FilterSpec = null;
listenerSettings.Level = EventLevel.Verbose;
testListener.EnableEvents(listenerSettings);

// Write some MessageJson events with null string.
for (var i = 0; i < 100; i++)
{
LoggingEventSource.Instance.MessageJson(LogLevel.Trace, 1, "MyLogger", 5, null, null, "testJson");
}

bool containsNullEventName = false;
foreach (var eventJson in testListener.Events)
{
if (eventJson.Contains(@"""__EVENT_NAME"":""MessageJson""") && eventJson.Contains(@"""EventName"":"""","))
{
containsNullEventName = true;
}
}

Assert.True(containsNullEventName, "EventName and ExceptionJson is supposed to be null but it isn't.");
}
}

[Fact]
public void Logs_AsExpected_ActivityJson_WithNullString()
{
using (var testListener = new TestEventListener())
{
var listenerSettings = new TestEventListener.ListenerSettings();
listenerSettings.Keywords = LoggingEventSource.Keywords.JsonMessage;
listenerSettings.FilterSpec = null;
listenerSettings.Level = EventLevel.Verbose;
testListener.EnableEvents(listenerSettings);

// Write some MessageJson events with null string.
for (var i = 0; i < 100; i++)
{
LoggingEventSource.Instance.ActivityJsonStart(6, 1, null, "someJson");
}

bool containsNullLoggerName = false;
foreach (var eventJson in testListener.Events)
{
if (eventJson.Contains(@"""__EVENT_NAME"":""ActivityJsonStart""") && eventJson.Contains(@"""LoggerName"":"""","))
{
containsNullLoggerName = true;
}
}

Assert.True(containsNullLoggerName, "LoggerName is supposed to be null but it isn't.");
}
}

private void LogStuff(ILoggerFactory factory)
{
var logger1 = factory.CreateLogger("Logger1");
Expand Down

0 comments on commit 8d0fa05

Please sign in to comment.