Skip to content

[HttpClientFactory] Don't log header values by default #106271

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
merged 4 commits into from
Aug 13, 2024
Merged

[HttpClientFactory] Don't log header values by default #106271

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
58554ac
Don't log header values by default
CarnaViire Aug 12, 2024
76d2b10
Merge branch 'main' into hcf-header-values
CarnaViire Aug 12, 2024
0f90eca
Apply suggestions from code review
CarnaViire Aug 12, 2024
23a49cc
Update RedactedLogValueIntegrationTest.cs
CarnaViire Aug 12, 2024
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
3 changes: 2 additions & 1 deletion src/libraries/Microsoft.Extensions.Http/src/HttpClientFactoryOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Net.Http;
using System.Threading;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Http.Logging;

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

/// <summary>
/// <para>
Expand Down
2 changes: 2 additions & 0 deletions src/libraries/Microsoft.Extensions.Http/src/Logging/LogHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ private static class EventIds
public static readonly EventId RequestPipelineResponseHeader = new EventId(103, "RequestPipelineResponseHeader");
}

public static readonly Func<string, bool> ShouldRedactHeaderValue = (header) => true;

private static readonly Action<ILogger, HttpMethod, string?, Exception?> _requestStart = LoggerMessage.Define<HttpMethod, string?>(
LogLevel.Information,
EventIds.RequestStart,
Expand Down
4 changes: 1 addition & 3 deletions src/libraries/Microsoft.Extensions.Http/src/Logging/LoggingHttpMessageHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@ public class LoggingHttpMessageHandler : DelegatingHandler
private readonly ILogger _logger;
private readonly HttpClientFactoryOptions? _options;

private static readonly Func<string, bool> _shouldNotRedactHeaderValue = (header) => false;

/// <summary>
/// Initializes a new instance of the <see cref="LoggingHttpMessageHandler"/> class with a specified logger.
/// </summary>
Expand Down Expand Up @@ -55,7 +53,7 @@ private Task<HttpResponseMessage> SendCoreAsync(HttpRequestMessage request, bool

async Task<HttpResponseMessage> Core(HttpRequestMessage request, bool useAsync, CancellationToken cancellationToken)
{
Func<string, bool> shouldRedactHeaderValue = _options?.ShouldRedactHeaderValue ?? _shouldNotRedactHeaderValue;
Func<string, bool> shouldRedactHeaderValue = _options?.ShouldRedactHeaderValue ?? LogHelper.ShouldRedactHeaderValue;

// Not using a scope here because we always expect this to be at the end of the pipeline, thus there's
// not really anything to surround.
Expand Down
4 changes: 1 addition & 3 deletions src/libraries/Microsoft.Extensions.Http/src/Logging/LoggingScopeHttpMessageHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@ public class LoggingScopeHttpMessageHandler : DelegatingHandler
private readonly ILogger _logger;
private readonly HttpClientFactoryOptions? _options;

private static readonly Func<string, bool> _shouldNotRedactHeaderValue = (header) => false;

/// <summary>
/// Initializes a new instance of the <see cref="LoggingScopeHttpMessageHandler"/> class with a specified logger.
/// </summary>
Expand Down Expand Up @@ -56,7 +54,7 @@ async Task<HttpResponseMessage> Core(HttpRequestMessage request, bool useAsync,
{
var stopwatch = ValueStopwatch.StartNew();

Func<string, bool> shouldRedactHeaderValue = _options?.ShouldRedactHeaderValue ?? _shouldNotRedactHeaderValue;
Func<string, bool> shouldRedactHeaderValue = _options?.ShouldRedactHeaderValue ?? LogHelper.ShouldRedactHeaderValue;

using (_logger.BeginRequestPipelineScope(request, out string? formattedUri))
{
Expand Down
104 changes: 96 additions & 8 deletions ...ons.Http/tests/Microsoft.Extensions.Http.Tests/Logging/RedactedLogValueIntegrationTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ namespace Microsoft.Extensions.Http.Logging
{
public class RedactedLogValueIntegrationTest
{
private const string OuterLoggerName = "System.Net.Http.HttpClient.test.LogicalHandler";
private const string InnerLoggerName = "System.Net.Http.HttpClient.test.ClientHandler";

private static class EventIds
{
public static readonly EventId RequestHeader = new EventId(102, "RequestHeader");
Expand All @@ -25,6 +28,91 @@ private static class EventIds
public static readonly EventId RequestPipelineResponseHeader = new EventId(103, "RequestPipelineResponseHeader");
}

[Fact]
public async Task RedactLoggedHeadersNotCalled_AllValuesAreRedactedBeforeLogging()
{
// Arrange
var sink = new TestSink();

var serviceCollection = new ServiceCollection();
serviceCollection.AddLogging();
serviceCollection.AddSingleton<ILoggerFactory>(new TestLoggerFactory(sink, enabled: true));

// Act
serviceCollection
.AddHttpClient("test")
.ConfigurePrimaryHttpMessageHandler(() => new TestMessageHandler());

// Assert
var services = serviceCollection.BuildServiceProvider();

var client = services.GetRequiredService<IHttpClientFactory>().CreateClient("test");

var request = new HttpRequestMessage(HttpMethod.Get, "http://example.com");
request.Headers.Authorization = new AuthenticationHeaderValue("fake", "secret value");
request.Headers.CacheControl = new CacheControlHeaderValue() { NoCache = true, };

await client.SendAsync(request);

var messages = sink.Writes.ToArray();

var message = Assert.Single(messages.Where(m =>
{
return
m.EventId == EventIds.RequestPipelineRequestHeader &&
m.LoggerName == OuterLoggerName;
}));
Assert.StartsWith(LineEndingsHelper.Normalize(
"""
Request Headers:
Authorization: *
Cache-Control: *
"""),
message.Message);

message = Assert.Single(messages.Where(m =>
{
return
m.EventId == EventIds.RequestHeader &&
m.LoggerName == InnerLoggerName;
}));
Assert.StartsWith(LineEndingsHelper.Normalize(
"""
Request Headers:
Authorization: *
Cache-Control: *
"""),
message.Message);

message = Assert.Single(messages.Where(m =>
{
return
m.EventId == EventIds.ResponseHeader &&
m.LoggerName == InnerLoggerName;
}));
Assert.StartsWith(LineEndingsHelper.Normalize(
"""
Response Headers:
X-Sensitive: *
Y-Non-Sensitive: *
"""),
message.Message);

message = Assert.Single(messages.Where(m =>
{
return
m.EventId == EventIds.RequestPipelineResponseHeader &&
m.LoggerName == OuterLoggerName;
}));
Assert.StartsWith(LineEndingsHelper.Normalize(
"""
Response Headers:
X-Sensitive: *
Y-Non-Sensitive: *
"""),
message.Message);
}

[Fact]
public async Task RedactHeaderValueWithHeaderList_ValueIsRedactedBeforeLogging()
{
Expand Down Expand Up @@ -58,7 +146,7 @@ public async Task RedactHeaderValueWithHeaderList_ValueIsRedactedBeforeLogging()
{
return
m.EventId == EventIds.RequestPipelineRequestHeader &&
m.LoggerName == "System.Net.Http.HttpClient.test.LogicalHandler";
m.LoggerName == OuterLoggerName;
}));
Assert.StartsWith(LineEndingsHelper.Normalize(
@"Request Headers:
Expand All @@ -70,7 +158,7 @@ public async Task RedactHeaderValueWithHeaderList_ValueIsRedactedBeforeLogging()
{
return
m.EventId == EventIds.RequestHeader &&
m.LoggerName == "System.Net.Http.HttpClient.test.ClientHandler";
m.LoggerName == InnerLoggerName;
}));
Assert.StartsWith(LineEndingsHelper.Normalize(
@"Request Headers:
Expand All @@ -82,7 +170,7 @@ public async Task RedactHeaderValueWithHeaderList_ValueIsRedactedBeforeLogging()
{
return
m.EventId == EventIds.ResponseHeader &&
m.LoggerName == "System.Net.Http.HttpClient.test.ClientHandler";
m.LoggerName == InnerLoggerName;
}));
Assert.StartsWith(LineEndingsHelper.Normalize(
@"Response Headers:
Expand All @@ -94,7 +182,7 @@ public async Task RedactHeaderValueWithHeaderList_ValueIsRedactedBeforeLogging()
{
return
m.EventId == EventIds.RequestPipelineResponseHeader &&
m.LoggerName == "System.Net.Http.HttpClient.test.LogicalHandler";
m.LoggerName == OuterLoggerName;
}));
Assert.StartsWith(LineEndingsHelper.Normalize(
@"Response Headers:
Expand Down Expand Up @@ -139,7 +227,7 @@ public async Task RedactHeaderValueWithPredicate_ValueIsRedactedBeforeLogging()
{
return
m.EventId == EventIds.RequestPipelineRequestHeader &&
m.LoggerName == "System.Net.Http.HttpClient.test.LogicalHandler";
m.LoggerName == OuterLoggerName;
}));
Assert.StartsWith(LineEndingsHelper.Normalize(
@"Request Headers:
Expand All @@ -151,7 +239,7 @@ public async Task RedactHeaderValueWithPredicate_ValueIsRedactedBeforeLogging()
{
return
m.EventId == EventIds.RequestHeader &&
m.LoggerName == "System.Net.Http.HttpClient.test.ClientHandler";
m.LoggerName == InnerLoggerName;
}));
Assert.StartsWith(LineEndingsHelper.Normalize(
@"Request Headers:
Expand All @@ -163,7 +251,7 @@ public async Task RedactHeaderValueWithPredicate_ValueIsRedactedBeforeLogging()
{
return
m.EventId == EventIds.ResponseHeader &&
m.LoggerName == "System.Net.Http.HttpClient.test.ClientHandler";
m.LoggerName == InnerLoggerName;
}));
Assert.StartsWith(LineEndingsHelper.Normalize(
@"Response Headers:
Expand All @@ -175,7 +263,7 @@ public async Task RedactHeaderValueWithPredicate_ValueIsRedactedBeforeLogging()
{
return
m.EventId == EventIds.RequestPipelineResponseHeader &&
m.LoggerName == "System.Net.Http.HttpClient.test.LogicalHandler";
m.LoggerName == OuterLoggerName;
}));
Assert.StartsWith(LineEndingsHelper.Normalize(
@"Response Headers:
Expand Down
Loading