Conversation
|
This test with verbose logging enabled. Not much difference for latency, slight increase in allocations. Verbose logging is not expected to be enabled in production full time.
|
SummarySummary
CoverageMicrosoft.IdentityModel.JsonWebTokens - 80.3%
|
There was a problem hiding this comment.
Pull Request Overview
This PR implements log sanitization functionality to prevent log injection attacks by escaping potentially harmful characters in log messages. The changes ensure that control characters like newlines, carriage returns, and tabs are properly encoded when logging both PII and non-PII data.
- Adds a new
Sanitizemethod that encodes control characters to prevent log injection - Updates logging methods to apply sanitization to both PII and non-PII strings
- Enhances TestLogger with optional log saving capability for better test control
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/Microsoft.IdentityModel.Logging/LogHelper.cs | Implements core sanitization logic and integrates it into existing logging methods |
| test/Microsoft.IdentityModel.Logging.Tests/LogHelperTests.cs | Adds comprehensive tests for sanitization functionality covering PII and non-PII scenarios |
| test/Microsoft.IdentityModel.Logging.Tests/TestLogger.cs | Adds constructor parameter to control log saving behavior for testing |
|
With logging enabled:
With logging disabled:
With Sanitize method in code base (Sanitize is not called, so allocations are the same):
|
Added setup step for .NET 6.x in workflow.
SummarySummary
CoverageMicrosoft.IdentityModel.JsonWebTokens - 80.3%
|
|
#3341 optimizes performance for this change for NET8+ targets. |
No description provided.