Skip to content

Add BinaryFormatter auditing EventSource #39874

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
GrabYourPitchforks merged 7 commits into from
Jul 29, 2020
Merged

Add BinaryFormatter auditing EventSource #39874

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
1ffeffc
Add BinaryFormatter auditing EventSource
GrabYourPitchforks Jul 23, 2020
3fb9e02
PR feedback: rename Started -> Start, Ended -> Stop
GrabYourPitchforks Jul 29, 2020
a99f7cf
PR feedback: remove EventSource localization
GrabYourPitchforks Jul 29, 2020
8b8cc85
PR feedback: remove anti-recursion checks
GrabYourPitchforks Jul 29, 2020
6bb4f32
PR feedback: fix erroneous .csproj conditions
GrabYourPitchforks Jul 29, 2020
7d53f57
PR feedback: clarify listener dispose / log capture
GrabYourPitchforks Jul 29, 2020
d033375
PR feedback: mark Start events as recursive
GrabYourPitchforks Jul 29, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
PR feedback: remove anti-recursion checks
  • Loading branch information
@GrabYourPitchforks
GrabYourPitchforks committed Jul 29, 2020
commit 8b8cc85b52c6716192c587f8b682288a26c36b6d
79 changes: 12 additions & 67 deletions ...rmatters/src/System/Runtime/Serialization/Formatters/Binary/BinaryFormatterEventSource.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

using System.Diagnostics;
using System.Diagnostics.Tracing;
using System.Threading;

namespace System.Runtime.Serialization.Formatters.Binary
{
Expand All @@ -18,12 +17,6 @@ internal sealed class BinaryFormatterEventSource : EventSource
private const int EventId_DeserializationStop = 21;
private const int EventId_DeserializingObject = 22;

// Used to keep track of whether a write operation is in progress. It's
// possible the listener itself uses BinaryFormatter to write to a log,
// and if this is the case we suppress our own logging events so that we
// enter an infinite recursion scenario.
private static readonly AsyncLocal<bool> _writeInProgress = new AsyncLocal<bool>();

public static readonly BinaryFormatterEventSource Log = new BinaryFormatterEventSource();

private BinaryFormatterEventSource()
Expand All @@ -33,34 +26,18 @@ private BinaryFormatterEventSource()
[Event(EventId_SerializationStart, Opcode = EventOpcode.Start, Keywords = Keywords.Serialization, Level = EventLevel.Informational)]
public void SerializationStart()
{
if (IsEnabled(EventLevel.Informational, Keywords.Serialization) && !_writeInProgress.Value)
if (IsEnabled(EventLevel.Informational, Keywords.Serialization))
{
try
{
_writeInProgress.Value = true;
WriteEvent(EventId_SerializationStart);
}
finally
{
_writeInProgress.Value = false;
}
WriteEvent(EventId_SerializationStart);
}
}

[Event(EventId_SerializationStop, Opcode = EventOpcode.Stop, Keywords = Keywords.Serialization, Level = EventLevel.Informational)]
public void SerializationStop()
{
if (IsEnabled(EventLevel.Informational, Keywords.Serialization) && !_writeInProgress.Value)
if (IsEnabled(EventLevel.Informational, Keywords.Serialization))
{
try
{
_writeInProgress.Value = true;
WriteEvent(EventId_SerializationStop);
}
finally
{
_writeInProgress.Value = false;
}
WriteEvent(EventId_SerializationStop);
}
}

Expand All @@ -69,17 +46,9 @@ public void SerializingObject(Type type)
{
Debug.Assert(type != null);

if (IsEnabled(EventLevel.Informational, Keywords.Serialization) && !_writeInProgress.Value)
if (IsEnabled(EventLevel.Informational, Keywords.Serialization))
{
try
{
_writeInProgress.Value = true;
SerializingObject(type.AssemblyQualifiedName);
}
finally
{
_writeInProgress.Value = false;
}
SerializingObject(type.AssemblyQualifiedName);
}
}

Expand All @@ -92,34 +61,18 @@ private void SerializingObject(string? typeName)
[Event(EventId_DeserializationStart, Opcode = EventOpcode.Start, Keywords = Keywords.Deserialization, Level = EventLevel.Informational)]
public void DeserializationStart()
{
if (IsEnabled(EventLevel.Informational, Keywords.Deserialization) && !_writeInProgress.Value)
if (IsEnabled(EventLevel.Informational, Keywords.Deserialization))
{
try
{
_writeInProgress.Value = true;
WriteEvent(EventId_DeserializationStart);
}
finally
{
_writeInProgress.Value = false;
}
WriteEvent(EventId_DeserializationStart);
}
}

[Event(EventId_DeserializationStop, Opcode = EventOpcode.Stop, Keywords = Keywords.Deserialization, Level = EventLevel.Informational)]
public void DeserializationStop()
{
if (IsEnabled(EventLevel.Informational, Keywords.Deserialization) && !_writeInProgress.Value)
if (IsEnabled(EventLevel.Informational, Keywords.Deserialization))
{
try
{
_writeInProgress.Value = true;
WriteEvent(EventId_DeserializationStop);
}
finally
{
_writeInProgress.Value = false;
}
WriteEvent(EventId_DeserializationStop);
}
}

Expand All @@ -128,17 +81,9 @@ public void DeserializingObject(Type type)
{
Debug.Assert(type != null);

if (IsEnabled(EventLevel.Informational, Keywords.Deserialization) && !_writeInProgress.Value)
if (IsEnabled(EventLevel.Informational, Keywords.Deserialization))
{
try
{
_writeInProgress.Value = true;
DeserializingObject(type.AssemblyQualifiedName);
}
finally
{
_writeInProgress.Value = false;
}
DeserializingObject(type.AssemblyQualifiedName);
}
}

Expand Down
50 changes: 0 additions & 50 deletions ...ibraries/System.Runtime.Serialization.Formatters/tests/BinaryFormatterEventSourceTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,56 +103,6 @@ public static void RecordsNestedSerializationCalls()
Assert.Equal(expected, listener.Log);
}

[Fact]
public static void DoesNotRecordRecursiveSerializationPerformedByEventListener()
{
// First, serialization

LoggingEventListener listener = new LoggingEventListener();
listener.EventWritten += (source, args) => listener.Log.Add("Callback fired.");
listener.EventWritten += (source, args) => new BinaryFormatter().Serialize(Stream.Null, CreatePerson());

MemoryStream ms = new MemoryStream();
BinaryFormatter formatter = new BinaryFormatter();
formatter.Serialize(ms, CreatePerson());
ms.Position = 0;

string[] expected = new string[]
{
"SerializationStart [Start, 00000001]: <no payload>",
"Callback fired.",
"SerializingObject [Info, 00000001]: " + typeof(Person).AssemblyQualifiedName,
"Callback fired.",
"SerializingObject [Info, 00000001]: " + typeof(Address).AssemblyQualifiedName,
"Callback fired.",
"SerializationStop [Stop, 00000001]: <no payload>",
"Callback fired.",
};

Assert.Equal(expected, listener.Log);
listener.Log.Clear();

// Then, deserialization

ms.Position = 0;
formatter.Deserialize(ms);
listener.Dispose();

expected = new string[]
{
"DeserializationStart [Start, 00000002]: <no payload>",
"Callback fired.",
"DeserializingObject [Info, 00000002]: " + typeof(Person).AssemblyQualifiedName,
"Callback fired.",
"DeserializingObject [Info, 00000002]: " + typeof(Address).AssemblyQualifiedName,
"Callback fired.",
"DeserializationStop [Stop, 00000002]: <no payload>",
"Callback fired.",
};

Assert.Equal(expected, listener.Log);
}

private static Person CreatePerson()
{
return new Person()
Expand Down