From 775143804aad7fecc9ecc775c3781d3980330d81 Mon Sep 17 00:00:00 2001
From: Masters <dustin.masters@olo.com>
Date: Tue, 28 Jun 2022 13:14:59 -0400
Subject: [PATCH] Fix logging conditions for additional request headers
(#42456)
---
.../HttpLogging/src/W3CLoggingMiddleware.cs | 34 ++++++++----------
.../test/W3CLoggingMiddlewareTests.cs | 36 +++++++++++++++++++
2 files changed, 51 insertions(+), 19 deletions(-)
diff --git a/src/Middleware/HttpLogging/src/W3CLoggingMiddleware.cs b/src/Middleware/HttpLogging/src/W3CLoggingMiddleware.cs
index de9f9334d2b..90fe835629a 100644
--- a/src/Middleware/HttpLogging/src/W3CLoggingMiddleware.cs
+++ b/src/Middleware/HttpLogging/src/W3CLoggingMiddleware.cs
@@ -82,10 +82,8 @@ internal sealed class W3CLoggingMiddleware
{
var options = _options.CurrentValue;
- var additionalHeadersLength = _additionalRequestHeaders.Count;
-
var elements = new string[_fieldsLength];
- var additionalHeaderElements = new string[additionalHeadersLength];
+ var additionalHeaderElements = new string[_additionalRequestHeaders.Count];
// Whether any of the requested fields actually had content
bool shouldLog = false;
@@ -154,11 +152,9 @@ internal sealed class W3CLoggingMiddleware
if ((W3CLoggingFields.RequestHeaders & options.LoggingFields) != W3CLoggingFields.None)
{
- var headers = request.Headers;
-
if (options.LoggingFields.HasFlag(W3CLoggingFields.Host))
{
- if (headers.TryGetValue(HeaderNames.Host, out var host))
+ if (request.Headers.TryGetValue(HeaderNames.Host, out var host))
{
shouldLog |= AddToList(elements, _hostIndex, host.ToString());
}
@@ -166,7 +162,7 @@ internal sealed class W3CLoggingMiddleware
if (options.LoggingFields.HasFlag(W3CLoggingFields.Referer))
{
- if (headers.TryGetValue(HeaderNames.Referer, out var referer))
+ if (request.Headers.TryGetValue(HeaderNames.Referer, out var referer))
{
shouldLog |= AddToList(elements, _refererIndex, referer.ToString());
}
@@ -174,7 +170,7 @@ internal sealed class W3CLoggingMiddleware
if (options.LoggingFields.HasFlag(W3CLoggingFields.UserAgent))
{
- if (headers.TryGetValue(HeaderNames.UserAgent, out var agent))
+ if (request.Headers.TryGetValue(HeaderNames.UserAgent, out var agent))
{
shouldLog |= AddToList(elements, _userAgentIndex, agent.ToString());
}
@@ -182,23 +178,23 @@ internal sealed class W3CLoggingMiddleware
if (options.LoggingFields.HasFlag(W3CLoggingFields.Cookie))
{
- if (headers.TryGetValue(HeaderNames.Cookie, out var cookie))
+ if (request.Headers.TryGetValue(HeaderNames.Cookie, out var cookie))
{
shouldLog |= AddToList(elements, _cookieIndex, cookie.ToString());
}
}
+ }
+ }
- if (_additionalRequestHeaders.Count != 0)
- {
- var additionalRequestHeaders = _additionalRequestHeaders.ToList();
+ if (_additionalRequestHeaders.Count != 0)
+ {
+ var additionalRequestHeaders = _additionalRequestHeaders.ToList();
- for (var i = 0; i < additionalHeadersLength; i++)
- {
- if (headers.TryGetValue(additionalRequestHeaders[i], out var headerValue))
- {
- shouldLog |= AddToList(additionalHeaderElements, i, headerValue.ToString());
- }
- }
+ for (var i = 0; i < additionalRequestHeaders.Count; i++)
+ {
+ if (context.Request.Headers.TryGetValue(additionalRequestHeaders[i], out var headerValue))
+ {
+ shouldLog |= AddToList(additionalHeaderElements, i, headerValue.ToString());
}
}
}
diff --git a/src/Middleware/HttpLogging/test/W3CLoggingMiddlewareTests.cs b/src/Middleware/HttpLogging/test/W3CLoggingMiddlewareTests.cs
index d9be128515e..988631cd96c 100644
--- a/src/Middleware/HttpLogging/test/W3CLoggingMiddlewareTests.cs
+++ b/src/Middleware/HttpLogging/test/W3CLoggingMiddlewareTests.cs
@@ -147,6 +147,42 @@ public class W3CLoggingMiddlewareTests
Assert.EndsWith("- - 1.3.3.7,+2001:db8:85a3:8d3:1319:8a2e:370:7348", lines[3]);
}
+ [Fact]
+ public async Task LogsAdditionalRequestHeaders_WithNoOtherOptions()
+ {
+ var options = CreateOptionsAccessor();
+ options.CurrentValue.AdditionalRequestHeaders.Add("x-forwarded-for");
+ options.CurrentValue.LoggingFields = W3CLoggingFields.None;
+
+ var logger = Helpers.CreateTestW3CLogger(options);
+
+ var middleware = new W3CLoggingMiddleware(
+ c =>
+ {
+ c.Response.StatusCode = 200;
+ return Task.CompletedTask;
+ },
+ options,
+ logger);
+
+ var httpContext = new DefaultHttpContext();
+ httpContext.Request.Protocol = "HTTP/1.0";
+ httpContext.Request.Headers["Cookie"] = "Snickerdoodle";
+ httpContext.Request.Headers["x-forwarded-for"] = "1.3.3.7, 2001:db8:85a3:8d3:1319:8a2e:370:7348";
+ httpContext.Response.StatusCode = 200;
+
+ var now = DateTime.UtcNow;
+ await middleware.Invoke(httpContext);
+ await logger.Processor.WaitForWrites(4).DefaultTimeout();
+
+ var lines = logger.Processor.Lines;
+ Assert.Equal("#Version: 1.0", lines[0]);
+
+ Assert.StartsWith("#Start-Date: ", lines[1]);
+ Assert.Equal("#Fields: cs(x-forwarded-for)", lines[2]);
+ Assert.Equal("1.3.3.7,+2001:db8:85a3:8d3:1319:8a2e:370:7348", lines[3]);
+ }
+
[Fact]
public async Task OmitsDuplicateAdditionalRequestHeaders()
{
--
GitLab