Skip to content

Commit 47af310

Browse files
CarnaViireMihaZupan
CarnaViire
and
MihaZupan
authored
[HttpClientFactory] Don't log header values by default (#106271)
* Don't log header values by default * Apply suggestions from code review Co-authored-by: Miha Zupan <[email protected]> * Update RedactedLogValueIntegrationTest.cs --------- Co-authored-by: Miha Zupan <[email protected]>
1 parent b080dc2 commit 47af310

File tree

5 files changed

+102
-15
lines changed

5 files changed

+102
-15
lines changed

‎src/libraries/Microsoft.Extensions.Http/src/HttpClientFactoryOptions.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
using System.Net.Http;
77
using System.Threading;
88
using Microsoft.Extensions.DependencyInjection;
9+
using Microsoft.Extensions.Http.Logging;
910

1011
namespace Microsoft.Extensions.Http
1112
{
@@ -72,7 +73,7 @@ public TimeSpan HandlerLifetime
7273
/// <summary>
7374
/// The <see cref="Func{T, R}"/> which determines whether to redact the HTTP header value before logging.
7475
/// </summary>
75-
public Func<string, bool> ShouldRedactHeaderValue { get; set; } = (header) => false;
76+
public Func<string, bool> ShouldRedactHeaderValue { get; set; } = LogHelper.ShouldRedactHeaderValue;
7677

7778
/// <summary>
7879
/// <para>

‎src/libraries/Microsoft.Extensions.Http/src/Logging/LogHelper.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ private static class EventIds
3232
public static readonly EventId RequestPipelineResponseHeader = new EventId(103, "RequestPipelineResponseHeader");
3333
}
3434

35+
public static readonly Func<string, bool> ShouldRedactHeaderValue = (header) => true;
36+
3537
private static readonly Action<ILogger, HttpMethod, string?, Exception?> _requestStart = LoggerMessage.Define<HttpMethod, string?>(
3638
LogLevel.Information,
3739
EventIds.RequestStart,

‎src/libraries/Microsoft.Extensions.Http/src/Logging/LoggingHttpMessageHandler.cs‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,6 @@ public class LoggingHttpMessageHandler : DelegatingHandler
1919
private readonly ILogger _logger;
2020
private readonly HttpClientFactoryOptions? _options;
2121

22-
private static readonly Func<string, bool> _shouldNotRedactHeaderValue = (header) => false;
23-
2422
/// <summary>
2523
/// Initializes a new instance of the <see cref="LoggingHttpMessageHandler"/> class with a specified logger.
2624
/// </summary>
@@ -55,7 +53,7 @@ private Task<HttpResponseMessage> SendCoreAsync(HttpRequestMessage request, bool
5553

5654
async Task<HttpResponseMessage> Core(HttpRequestMessage request, bool useAsync, CancellationToken cancellationToken)
5755
{
58-
Func<string, bool> shouldRedactHeaderValue = _options?.ShouldRedactHeaderValue ?? _shouldNotRedactHeaderValue;
56+
Func<string, bool> shouldRedactHeaderValue = _options?.ShouldRedactHeaderValue ?? LogHelper.ShouldRedactHeaderValue;
5957

6058
// Not using a scope here because we always expect this to be at the end of the pipeline, thus there's
6159
// not really anything to surround.

‎src/libraries/Microsoft.Extensions.Http/src/Logging/LoggingScopeHttpMessageHandler.cs‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,6 @@ public class LoggingScopeHttpMessageHandler : DelegatingHandler
1818
private readonly ILogger _logger;
1919
private readonly HttpClientFactoryOptions? _options;
2020

21-
private static readonly Func<string, bool> _shouldNotRedactHeaderValue = (header) => false;
22-
2321
/// <summary>
2422
/// Initializes a new instance of the <see cref="LoggingScopeHttpMessageHandler"/> class with a specified logger.
2523
/// </summary>
@@ -56,7 +54,7 @@ async Task<HttpResponseMessage> Core(HttpRequestMessage request, bool useAsync,
5654
{
5755
var stopwatch = ValueStopwatch.StartNew();
5856

59-
Func<string, bool> shouldRedactHeaderValue = _options?.ShouldRedactHeaderValue ?? _shouldNotRedactHeaderValue;
57+
Func<string, bool> shouldRedactHeaderValue = _options?.ShouldRedactHeaderValue ?? LogHelper.ShouldRedactHeaderValue;
6058

6159
using (_logger.BeginRequestPipelineScope(request, out string? formattedUri))
6260
{

‎src/libraries/Microsoft.Extensions.Http/tests/Microsoft.Extensions.Http.Tests/Logging/RedactedLogValueIntegrationTest.cs‎

Lines changed: 96 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@ namespace Microsoft.Extensions.Http.Logging
1616
{
1717
public class RedactedLogValueIntegrationTest
1818
{
19+
private const string OuterLoggerName = "System.Net.Http.HttpClient.test.LogicalHandler";
20+
private const string InnerLoggerName = "System.Net.Http.HttpClient.test.ClientHandler";
21+
1922
private static class EventIds
2023
{
2124
public static readonly EventId RequestHeader = new EventId(102, "RequestHeader");
@@ -25,6 +28,91 @@ private static class EventIds
2528
public static readonly EventId RequestPipelineResponseHeader = new EventId(103, "RequestPipelineResponseHeader");
2629
}
2730

31+
[Fact]
32+
public async Task RedactLoggedHeadersNotCalled_AllValuesAreRedactedBeforeLogging()
33+
{
34+
// Arrange
35+
var sink = new TestSink();
36+
37+
var serviceCollection = new ServiceCollection();
38+
serviceCollection.AddLogging();
39+
serviceCollection.AddSingleton<ILoggerFactory>(new TestLoggerFactory(sink, enabled: true));
40+
41+
// Act
42+
serviceCollection
43+
.AddHttpClient("test")
44+
.ConfigurePrimaryHttpMessageHandler(() => new TestMessageHandler());
45+
46+
// Assert
47+
var services = serviceCollection.BuildServiceProvider();
48+
49+
var client = services.GetRequiredService<IHttpClientFactory>().CreateClient("test");
50+
51+
var request = new HttpRequestMessage(HttpMethod.Get, "http://example.com");
52+
request.Headers.Authorization = new AuthenticationHeaderValue("fake", "secret value");
53+
request.Headers.CacheControl = new CacheControlHeaderValue() { NoCache = true, };
54+
55+
await client.SendAsync(request);
56+
57+
var messages = sink.Writes.ToArray();
58+
59+
var message = Assert.Single(messages.Where(m =>
60+
{
61+
return
62+
m.EventId == EventIds.RequestPipelineRequestHeader &&
63+
m.LoggerName == OuterLoggerName;
64+
}));
65+
Assert.StartsWith(LineEndingsHelper.Normalize(
66+
"""
67+
Request Headers:
68+
Authorization: *
69+
Cache-Control: *
70+
"""),
71+
message.Message);
72+
73+
message = Assert.Single(messages.Where(m =>
74+
{
75+
return
76+
m.EventId == EventIds.RequestHeader &&
77+
m.LoggerName == InnerLoggerName;
78+
}));
79+
Assert.StartsWith(LineEndingsHelper.Normalize(
80+
"""
81+
Request Headers:
82+
Authorization: *
83+
Cache-Control: *
84+
"""),
85+
message.Message);
86+
87+
message = Assert.Single(messages.Where(m =>
88+
{
89+
return
90+
m.EventId == EventIds.ResponseHeader &&
91+
m.LoggerName == InnerLoggerName;
92+
}));
93+
Assert.StartsWith(LineEndingsHelper.Normalize(
94+
"""
95+
Response Headers:
96+
X-Sensitive: *
97+
Y-Non-Sensitive: *
98+
"""),
99+
message.Message);
100+
101+
message = Assert.Single(messages.Where(m =>
102+
{
103+
return
104+
m.EventId == EventIds.RequestPipelineResponseHeader &&
105+
m.LoggerName == OuterLoggerName;
106+
}));
107+
Assert.StartsWith(LineEndingsHelper.Normalize(
108+
"""
109+
Response Headers:
110+
X-Sensitive: *
111+
Y-Non-Sensitive: *
112+
"""),
113+
message.Message);
114+
}
115+
28116
[Fact]
29117
public async Task RedactHeaderValueWithHeaderList_ValueIsRedactedBeforeLogging()
30118
{
@@ -58,7 +146,7 @@ public async Task RedactHeaderValueWithHeaderList_ValueIsRedactedBeforeLogging()
58146
{
59147
return
60148
m.EventId == EventIds.RequestPipelineRequestHeader &&
61-
m.LoggerName == "System.Net.Http.HttpClient.test.LogicalHandler";
149+
m.LoggerName == OuterLoggerName;
62150
}));
63151
Assert.StartsWith(LineEndingsHelper.Normalize(
64152
@"Request Headers:
@@ -70,7 +158,7 @@ public async Task RedactHeaderValueWithHeaderList_ValueIsRedactedBeforeLogging()
70158
{
71159
return
72160
m.EventId == EventIds.RequestHeader &&
73-
m.LoggerName == "System.Net.Http.HttpClient.test.ClientHandler";
161+
m.LoggerName == InnerLoggerName;
74162
}));
75163
Assert.StartsWith(LineEndingsHelper.Normalize(
76164
@"Request Headers:
@@ -82,7 +170,7 @@ public async Task RedactHeaderValueWithHeaderList_ValueIsRedactedBeforeLogging()
82170
{
83171
return
84172
m.EventId == EventIds.ResponseHeader &&
85-
m.LoggerName == "System.Net.Http.HttpClient.test.ClientHandler";
173+
m.LoggerName == InnerLoggerName;
86174
}));
87175
Assert.StartsWith(LineEndingsHelper.Normalize(
88176
@"Response Headers:
@@ -94,7 +182,7 @@ public async Task RedactHeaderValueWithHeaderList_ValueIsRedactedBeforeLogging()
94182
{
95183
return
96184
m.EventId == EventIds.RequestPipelineResponseHeader &&
97-
m.LoggerName == "System.Net.Http.HttpClient.test.LogicalHandler";
185+
m.LoggerName == OuterLoggerName;
98186
}));
99187
Assert.StartsWith(LineEndingsHelper.Normalize(
100188
@"Response Headers:
@@ -139,7 +227,7 @@ public async Task RedactHeaderValueWithPredicate_ValueIsRedactedBeforeLogging()
139227
{
140228
return
141229
m.EventId == EventIds.RequestPipelineRequestHeader &&
142-
m.LoggerName == "System.Net.Http.HttpClient.test.LogicalHandler";
230+
m.LoggerName == OuterLoggerName;
143231
}));
144232
Assert.StartsWith(LineEndingsHelper.Normalize(
145233
@"Request Headers:
@@ -151,7 +239,7 @@ public async Task RedactHeaderValueWithPredicate_ValueIsRedactedBeforeLogging()
151239
{
152240
return
153241
m.EventId == EventIds.RequestHeader &&
154-
m.LoggerName == "System.Net.Http.HttpClient.test.ClientHandler";
242+
m.LoggerName == InnerLoggerName;
155243
}));
156244
Assert.StartsWith(LineEndingsHelper.Normalize(
157245
@"Request Headers:
@@ -163,7 +251,7 @@ public async Task RedactHeaderValueWithPredicate_ValueIsRedactedBeforeLogging()
163251
{
164252
return
165253
m.EventId == EventIds.ResponseHeader &&
166-
m.LoggerName == "System.Net.Http.HttpClient.test.ClientHandler";
254+
m.LoggerName == InnerLoggerName;
167255
}));
168256
Assert.StartsWith(LineEndingsHelper.Normalize(
169257
@"Response Headers:
@@ -175,7 +263,7 @@ public async Task RedactHeaderValueWithPredicate_ValueIsRedactedBeforeLogging()
175263
{
176264
return
177265
m.EventId == EventIds.RequestPipelineResponseHeader &&
178-
m.LoggerName == "System.Net.Http.HttpClient.test.LogicalHandler";
266+
m.LoggerName == OuterLoggerName;
179267
}));
180268
Assert.StartsWith(LineEndingsHelper.Normalize(
181269
@"Response Headers:

0 commit comments

Comments
 (0)