Skip to content

Commit 561edc8

Browse files
mattjohnsonpintmattjohnsonpint
authored
Don't allow newlines in diagnostic logger messages (#1756)
1 parent b434388 commit 561edc8

15 files changed

+251
-121
lines changed

‎CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,10 @@
1010
- Support IntPtr and UIntPtr serialization ([#1746](https://github.com/getsentry/sentry-dotnet/pull/1746))
1111
- Catch permission exceptions on Android ([#1750](https://github.com/getsentry/sentry-dotnet/pull/1750))
1212

13+
### Fixes
14+
15+
- Don't allow newlines in diagnostic logger messages ([#1756](https://github.com/getsentry/sentry-dotnet/pull/1756))
16+
1317
## Sentry.Maui 3.18.0-preview.1
1418

1519
### Features

‎src/Sentry/Infrastructure/ConsoleDiagnosticLogger.cs

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
using System;
2-
using Sentry.Extensibility;
32

43
namespace Sentry.Infrastructure
54
{
@@ -9,25 +8,16 @@ namespace Sentry.Infrastructure
98
/// <remarks>
109
/// The default logger, usually replaced by a higher level logging adapter like Microsoft.Extensions.Logging.
1110
/// </remarks>
12-
public class ConsoleDiagnosticLogger : IDiagnosticLogger
11+
public class ConsoleDiagnosticLogger : DiagnosticLogger
1312
{
14-
private readonly SentryLevel _minimalLevel;
15-
1613
/// <summary>
1714
/// Creates a new instance of <see cref="ConsoleDiagnosticLogger"/>.
1815
/// </summary>
19-
public ConsoleDiagnosticLogger(SentryLevel minimalLevel) => _minimalLevel = minimalLevel;
20-
21-
/// <summary>
22-
/// Whether the logger is enabled to the defined level.
23-
/// </summary>
24-
public bool IsEnabled(SentryLevel level) => level >= _minimalLevel;
16+
public ConsoleDiagnosticLogger(SentryLevel minimalLevel) : base(minimalLevel)
17+
{
18+
}
2519

26-
/// <summary>
27-
/// Log message with level, exception and parameters.
28-
/// </summary>
29-
public void Log(SentryLevel logLevel, string message, Exception? exception = null, params object?[] args)
30-
=> Console.Write($@"{logLevel,7}: {string.Format(message, args)}
31-
{exception}");
20+
/// <inheritdoc />
21+
protected override void LogMessage(string message) => Console.WriteLine(message);
3222
}
3323
}

‎src/Sentry/Infrastructure/DebugDiagnosticLogger.cs

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
using System;
22
using System.Diagnostics;
3-
using Sentry.Extensibility;
43

54
namespace Sentry.Infrastructure
65
{
@@ -11,25 +10,16 @@ namespace Sentry.Infrastructure
1110
/// Logger available when compiled in Debug mode. It's useful when debugging apps running under IIS which have no output to Console logger.
1211
/// </remarks>
1312
[Obsolete("Logger doesn't work outside of Sentry SDK. Please use TraceDiagnosticLogger instead")]
14-
public class DebugDiagnosticLogger : IDiagnosticLogger
13+
public class DebugDiagnosticLogger : DiagnosticLogger
1514
{
16-
private readonly SentryLevel _minimalLevel;
17-
1815
/// <summary>
1916
/// Creates a new instance of <see cref="DebugDiagnosticLogger"/>.
2017
/// </summary>
21-
public DebugDiagnosticLogger(SentryLevel minimalLevel) => _minimalLevel = minimalLevel;
22-
23-
/// <summary>
24-
/// Whether the logger is enabled to the defined level.
25-
/// </summary>
26-
public bool IsEnabled(SentryLevel level) => level >= _minimalLevel;
18+
public DebugDiagnosticLogger(SentryLevel minimalLevel) : base(minimalLevel)
19+
{
20+
}
2721

28-
/// <summary>
29-
/// Log message with level, exception and parameters.
30-
/// </summary>
31-
public void Log(SentryLevel logLevel, string message, Exception? exception = null, params object?[] args)
32-
=> Debug.Write($@"{logLevel,7}: {string.Format(message, args)}
33-
{exception}");
22+
/// <inheritdoc />
23+
protected override void LogMessage(string message) => Debug.WriteLine(message);
3424
}
3525
}

‎src/Sentry/Infrastructure/DiagnosticLogger.cs

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
using System;
2+
using System.Text;
3+
using Sentry.Extensibility;
4+
5+
namespace Sentry.Infrastructure
6+
{
7+
/// <summary>
8+
/// Base class for diagnostic loggers.
9+
/// </summary>
10+
public abstract class DiagnosticLogger : IDiagnosticLogger
11+
{
12+
private readonly SentryLevel _minimalLevel;
13+
14+
/// <summary>
15+
/// Creates a new instance of <see cref="DiagnosticLogger"/>.
16+
/// </summary>
17+
protected DiagnosticLogger(SentryLevel minimalLevel) => _minimalLevel = minimalLevel;
18+
19+
/// <summary>
20+
/// Whether the logger is enabled to the defined level.
21+
/// </summary>
22+
public bool IsEnabled(SentryLevel level) => level >= _minimalLevel;
23+
24+
/// <summary>
25+
/// Log message with level, exception and parameters.
26+
/// </summary>
27+
public void Log(SentryLevel logLevel, string message, Exception? exception = null, params object?[] args)
28+
{
29+
// Note, linefeed and newline chars are removed to guard against log injection attacks.
30+
// See https://github.com/getsentry/sentry-dotnet/security/code-scanning/5
31+
32+
var formattedMessage = ScrubNewlines(string.Format(message, args));
33+
34+
var completeMessage = exception == null
35+
? $"{logLevel,7}: {formattedMessage}"
36+
: $"{logLevel,7}: {formattedMessage}{Environment.NewLine}{exception}";
37+
38+
LogMessage(completeMessage);
39+
}
40+
41+
/// <summary>
42+
/// Writes a formatted message to the log.
43+
/// </summary>
44+
/// <param name="message">The complete message, ready to be logged.</param>
45+
protected abstract void LogMessage(string message);
46+
47+
private static string ScrubNewlines(string s)
48+
{
49+
// Replaces "\r", "\n", or "\r\n" with a single space in one pass (and trims the end result)
50+
51+
var sb = new StringBuilder(s.Length);
52+
53+
for (var i = 0; i < s.Length; i++)
54+
{
55+
var c = s[i];
56+
switch (c)
57+
{
58+
case '\r':
59+
sb.Append(' ');
60+
if (i < s.Length - 1 && s[i + 1] == '\n')
61+
{
62+
i++; // to prevent two consecutive spaces from "\r\n"
63+
}
64+
break;
65+
case '\n':
66+
sb.Append(' ');
67+
break;
68+
default:
69+
sb.Append(c);
70+
break;
71+
}
72+
}
73+
74+
// trim end and return
75+
var len = sb.Length;
76+
while (sb[len - 1] == ' ')
77+
{
78+
len--;
79+
}
80+
81+
return sb.ToString(0, len);
82+
}
83+
}
84+
}

‎src/Sentry/Infrastructure/TraceDiagnosticLogger.cs

Lines changed: 5 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
#define TRACE
2-
using System;
32
using System.Diagnostics;
4-
using Sentry.Extensibility;
53

64
namespace Sentry.Infrastructure
75
{
@@ -11,35 +9,16 @@ namespace Sentry.Infrastructure
119
/// <remarks>
1210
/// Logger available when hooked to an IDE. It's useful when debugging apps running under IIS which have no output to Console logger.
1311
/// </remarks>
14-
public class TraceDiagnosticLogger : IDiagnosticLogger
12+
public class TraceDiagnosticLogger : DiagnosticLogger
1513
{
16-
private readonly SentryLevel _minimalLevel;
17-
1814
/// <summary>
1915
/// Creates a new instance of <see cref="TraceDiagnosticLogger"/>.
2016
/// </summary>
21-
public TraceDiagnosticLogger(SentryLevel minimalLevel) => _minimalLevel = minimalLevel;
22-
23-
/// <summary>
24-
/// Whether the logger is enabled to the defined level.
25-
/// </summary>
26-
public bool IsEnabled(SentryLevel level) => level >= _minimalLevel;
27-
28-
/// <summary>
29-
/// Log message with level, exception and parameters.
30-
/// </summary>
31-
public void Log(SentryLevel logLevel, string message, Exception? exception = null, params object?[] args)
17+
public TraceDiagnosticLogger(SentryLevel minimalLevel) : base(minimalLevel)
3218
{
33-
var formattedMessage = string.Format(message, args);
34-
if (exception == null)
35-
{
36-
Trace.WriteLine($"{logLevel,7}: {formattedMessage}");
37-
}
38-
else
39-
{
40-
Trace.WriteLine($@"{logLevel,7}: {formattedMessage}
41-
{exception}");
42-
}
4319
}
20+
21+
/// <inheritdoc />
22+
protected override void LogMessage(string message) => Trace.WriteLine(message);
4423
}
4524
}

‎test/Sentry.DiagnosticSource.Tests/ApiApprovalTests.Run.Core3_1.verified.txt

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1094,19 +1094,24 @@ namespace Sentry.Http
10941094
}
10951095
namespace Sentry.Infrastructure
10961096
{
1097-
public class ConsoleDiagnosticLogger : Sentry.Extensibility.IDiagnosticLogger
1097+
public class ConsoleDiagnosticLogger : Sentry.Infrastructure.DiagnosticLogger
10981098
{
10991099
public ConsoleDiagnosticLogger(Sentry.SentryLevel minimalLevel) { }
1100-
public bool IsEnabled(Sentry.SentryLevel level) { }
1101-
public void Log(Sentry.SentryLevel logLevel, string message, System.Exception? exception = null, params object?[] args) { }
1100+
protected override void LogMessage(string message) { }
11021101
}
11031102
[System.Obsolete("Logger doesn\'t work outside of Sentry SDK. Please use TraceDiagnosticLogger inste" +
11041103
"ad")]
1105-
public class DebugDiagnosticLogger : Sentry.Extensibility.IDiagnosticLogger
1104+
public class DebugDiagnosticLogger : Sentry.Infrastructure.DiagnosticLogger
11061105
{
11071106
public DebugDiagnosticLogger(Sentry.SentryLevel minimalLevel) { }
1107+
protected override void LogMessage(string message) { }
1108+
}
1109+
public abstract class DiagnosticLogger : Sentry.Extensibility.IDiagnosticLogger
1110+
{
1111+
protected DiagnosticLogger(Sentry.SentryLevel minimalLevel) { }
11081112
public bool IsEnabled(Sentry.SentryLevel level) { }
11091113
public void Log(Sentry.SentryLevel logLevel, string message, System.Exception? exception = null, params object?[] args) { }
1114+
protected abstract void LogMessage(string message);
11101115
}
11111116
public interface ISystemClock
11121117
{
@@ -1118,11 +1123,10 @@ namespace Sentry.Infrastructure
11181123
public SystemClock() { }
11191124
public System.DateTimeOffset GetUtcNow() { }
11201125
}
1121-
public class TraceDiagnosticLogger : Sentry.Extensibility.IDiagnosticLogger
1126+
public class TraceDiagnosticLogger : Sentry.Infrastructure.DiagnosticLogger
11221127
{
11231128
public TraceDiagnosticLogger(Sentry.SentryLevel minimalLevel) { }
1124-
public bool IsEnabled(Sentry.SentryLevel level) { }
1125-
public void Log(Sentry.SentryLevel logLevel, string message, System.Exception? exception = null, params object?[] args) { }
1129+
protected override void LogMessage(string message) { }
11261130
}
11271131
}
11281132
namespace Sentry.Integrations

‎test/Sentry.DiagnosticSource.Tests/ApiApprovalTests.Run.DotNet5_0.verified.txt

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1094,19 +1094,24 @@ namespace Sentry.Http
10941094
}
10951095
namespace Sentry.Infrastructure
10961096
{
1097-
public class ConsoleDiagnosticLogger : Sentry.Extensibility.IDiagnosticLogger
1097+
public class ConsoleDiagnosticLogger : Sentry.Infrastructure.DiagnosticLogger
10981098
{
10991099
public ConsoleDiagnosticLogger(Sentry.SentryLevel minimalLevel) { }
1100-
public bool IsEnabled(Sentry.SentryLevel level) { }
1101-
public void Log(Sentry.SentryLevel logLevel, string message, System.Exception? exception = null, params object?[] args) { }
1100+
protected override void LogMessage(string message) { }
11021101
}
11031102
[System.Obsolete("Logger doesn\'t work outside of Sentry SDK. Please use TraceDiagnosticLogger inste" +
11041103
"ad")]
1105-
public class DebugDiagnosticLogger : Sentry.Extensibility.IDiagnosticLogger
1104+
public class DebugDiagnosticLogger : Sentry.Infrastructure.DiagnosticLogger
11061105
{
11071106
public DebugDiagnosticLogger(Sentry.SentryLevel minimalLevel) { }
1107+
protected override void LogMessage(string message) { }
1108+
}
1109+
public abstract class DiagnosticLogger : Sentry.Extensibility.IDiagnosticLogger
1110+
{
1111+
protected DiagnosticLogger(Sentry.SentryLevel minimalLevel) { }
11081112
public bool IsEnabled(Sentry.SentryLevel level) { }
11091113
public void Log(Sentry.SentryLevel logLevel, string message, System.Exception? exception = null, params object?[] args) { }
1114+
protected abstract void LogMessage(string message);
11101115
}
11111116
public interface ISystemClock
11121117
{
@@ -1118,11 +1123,10 @@ namespace Sentry.Infrastructure
11181123
public SystemClock() { }
11191124
public System.DateTimeOffset GetUtcNow() { }
11201125
}
1121-
public class TraceDiagnosticLogger : Sentry.Extensibility.IDiagnosticLogger
1126+
public class TraceDiagnosticLogger : Sentry.Infrastructure.DiagnosticLogger
11221127
{
11231128
public TraceDiagnosticLogger(Sentry.SentryLevel minimalLevel) { }
1124-
public bool IsEnabled(Sentry.SentryLevel level) { }
1125-
public void Log(Sentry.SentryLevel logLevel, string message, System.Exception? exception = null, params object?[] args) { }
1129+
protected override void LogMessage(string message) { }
11261130
}
11271131
}
11281132
namespace Sentry.Integrations

‎test/Sentry.DiagnosticSource.Tests/ApiApprovalTests.Run.DotNet6_0.verified.txt

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1094,19 +1094,24 @@ namespace Sentry.Http
10941094
}
10951095
namespace Sentry.Infrastructure
10961096
{
1097-
public class ConsoleDiagnosticLogger : Sentry.Extensibility.IDiagnosticLogger
1097+
public class ConsoleDiagnosticLogger : Sentry.Infrastructure.DiagnosticLogger
10981098
{
10991099
public ConsoleDiagnosticLogger(Sentry.SentryLevel minimalLevel) { }
1100-
public bool IsEnabled(Sentry.SentryLevel level) { }
1101-
public void Log(Sentry.SentryLevel logLevel, string message, System.Exception? exception = null, params object?[] args) { }
1100+
protected override void LogMessage(string message) { }
11021101
}
11031102
[System.Obsolete("Logger doesn\'t work outside of Sentry SDK. Please use TraceDiagnosticLogger inste" +
11041103
"ad")]
1105-
public class DebugDiagnosticLogger : Sentry.Extensibility.IDiagnosticLogger
1104+
public class DebugDiagnosticLogger : Sentry.Infrastructure.DiagnosticLogger
11061105
{
11071106
public DebugDiagnosticLogger(Sentry.SentryLevel minimalLevel) { }
1107+
protected override void LogMessage(string message) { }
1108+
}
1109+
public abstract class DiagnosticLogger : Sentry.Extensibility.IDiagnosticLogger
1110+
{
1111+
protected DiagnosticLogger(Sentry.SentryLevel minimalLevel) { }
11081112
public bool IsEnabled(Sentry.SentryLevel level) { }
11091113
public void Log(Sentry.SentryLevel logLevel, string message, System.Exception? exception = null, params object?[] args) { }
1114+
protected abstract void LogMessage(string message);
11101115
}
11111116
public interface ISystemClock
11121117
{
@@ -1118,11 +1123,10 @@ namespace Sentry.Infrastructure
11181123
public SystemClock() { }
11191124
public System.DateTimeOffset GetUtcNow() { }
11201125
}
1121-
public class TraceDiagnosticLogger : Sentry.Extensibility.IDiagnosticLogger
1126+
public class TraceDiagnosticLogger : Sentry.Infrastructure.DiagnosticLogger
11221127
{
11231128
public TraceDiagnosticLogger(Sentry.SentryLevel minimalLevel) { }
1124-
public bool IsEnabled(Sentry.SentryLevel level) { }
1125-
public void Log(Sentry.SentryLevel logLevel, string message, System.Exception? exception = null, params object?[] args) { }
1129+
protected override void LogMessage(string message) { }
11261130
}
11271131
}
11281132
namespace Sentry.Integrations

‎test/Sentry.Tests/ApiApprovalTests.Run.Core2_1.verified.txt

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1093,19 +1093,24 @@ namespace Sentry.Http
10931093
}
10941094
namespace Sentry.Infrastructure
10951095
{
1096-
public class ConsoleDiagnosticLogger : Sentry.Extensibility.IDiagnosticLogger
1096+
public class ConsoleDiagnosticLogger : Sentry.Infrastructure.DiagnosticLogger
10971097
{
10981098
public ConsoleDiagnosticLogger(Sentry.SentryLevel minimalLevel) { }
1099-
public bool IsEnabled(Sentry.SentryLevel level) { }
1100-
public void Log(Sentry.SentryLevel logLevel, string message, System.Exception? exception = null, params object?[] args) { }
1099+
protected override void LogMessage(string message) { }
11011100
}
11021101
[System.Obsolete("Logger doesn\'t work outside of Sentry SDK. Please use TraceDiagnosticLogger inste" +
11031102
"ad")]
1104-
public class DebugDiagnosticLogger : Sentry.Extensibility.IDiagnosticLogger
1103+
public class DebugDiagnosticLogger : Sentry.Infrastructure.DiagnosticLogger
11051104
{
11061105
public DebugDiagnosticLogger(Sentry.SentryLevel minimalLevel) { }
1106+
protected override void LogMessage(string message) { }
1107+
}
1108+
public abstract class DiagnosticLogger : Sentry.Extensibility.IDiagnosticLogger
1109+
{
1110+
protected DiagnosticLogger(Sentry.SentryLevel minimalLevel) { }
11071111
public bool IsEnabled(Sentry.SentryLevel level) { }
11081112
public void Log(Sentry.SentryLevel logLevel, string message, System.Exception? exception = null, params object?[] args) { }
1113+
protected abstract void LogMessage(string message);
11091114
}
11101115
public interface ISystemClock
11111116
{
@@ -1117,11 +1122,10 @@ namespace Sentry.Infrastructure
11171122
public SystemClock() { }
11181123
public System.DateTimeOffset GetUtcNow() { }
11191124
}
1120-
public class TraceDiagnosticLogger : Sentry.Extensibility.IDiagnosticLogger
1125+
public class TraceDiagnosticLogger : Sentry.Infrastructure.DiagnosticLogger
11211126
{
11221127
public TraceDiagnosticLogger(Sentry.SentryLevel minimalLevel) { }
1123-
public bool IsEnabled(Sentry.SentryLevel level) { }
1124-
public void Log(Sentry.SentryLevel logLevel, string message, System.Exception? exception = null, params object?[] args) { }
1128+
protected override void LogMessage(string message) { }
11251129
}
11261130
}
11271131
namespace Sentry.Integrations

0 commit comments

Comments
 (0)