Skip to content

Commit eef6936

Browse files
CopilotMihaZupan
andauthored
Don't validate/materialize HTTP headers when logging them (#132010)
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: MihaZupan <25307628+MihaZupan@users.noreply.github.com> Co-authored-by: Miha Zupan <mihazupan.zupan1@gmail.com>
1 parent 4b61662 commit eef6936

2 files changed

Lines changed: 62 additions & 32 deletions

File tree

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

Lines changed: 36 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ namespace Microsoft.Extensions.Http.Logging
1111
{
1212
internal sealed class HttpHeadersLogValue : IReadOnlyList<KeyValuePair<string, object>>
1313
{
14+
private const string RedactedValue = "*";
15+
1416
private readonly Kind _kind;
1517
private readonly Func<string, bool> _shouldRedactHeaderValue;
1618

@@ -36,19 +38,13 @@ private List<KeyValuePair<string, object>> Values
3638
{
3739
if (_values == null)
3840
{
39-
var values = new List<KeyValuePair<string, object>>();
41+
var values = new List<KeyValuePair<string, object>>(GetHeaderCount(Headers) + GetHeaderCount(ContentHeaders));
4042

41-
foreach (KeyValuePair<string, IEnumerable<string>> kvp in Headers)
42-
{
43-
values.Add(new KeyValuePair<string, object>(kvp.Key, kvp.Value));
44-
}
43+
AddHeaders(values, Headers);
4544

4645
if (ContentHeaders != null)
4746
{
48-
foreach (KeyValuePair<string, IEnumerable<string>> kvp in ContentHeaders)
49-
{
50-
values.Add(new KeyValuePair<string, object>(kvp.Key, kvp.Value));
51-
}
47+
AddHeaders(values, ContentHeaders);
5248
}
5349

5450
_values = values;
@@ -73,6 +69,36 @@ public KeyValuePair<string, object> this[int index]
7369

7470
public int Count => Values.Count;
7571

72+
// Enumerate the headers without triggering validation/parsing of the values, so that logging
73+
// doesn't alter how the headers are subsequently serialized on the wire.
74+
private void AddHeaders(List<KeyValuePair<string, object>> values, HttpHeaders headers)
75+
{
76+
#if NET
77+
foreach (KeyValuePair<string, HeaderStringValues> kvp in headers.NonValidated)
78+
#else
79+
foreach (KeyValuePair<string, IEnumerable<string>> kvp in headers)
80+
#endif
81+
{
82+
string value = _shouldRedactHeaderValue(kvp.Key)
83+
? RedactedValue
84+
#if NET
85+
: kvp.Value.ToString();
86+
#else
87+
: string.Join(", ", kvp.Value);
88+
#endif
89+
values.Add(new KeyValuePair<string, object>(kvp.Key, value));
90+
}
91+
}
92+
93+
private static int GetHeaderCount(HttpHeaders? headers)
94+
{
95+
#if NET
96+
return headers?.NonValidated.Count ?? 0;
97+
#else
98+
return 0;
99+
#endif
100+
}
101+
76102
public IEnumerator<KeyValuePair<string, object>> GetEnumerator()
77103
{
78104
return Values.GetEnumerator();
@@ -95,29 +121,7 @@ public override string ToString()
95121
KeyValuePair<string, object> kvp = Values[i];
96122
builder.Append(kvp.Key);
97123
builder.Append(": ");
98-
99-
if (_shouldRedactHeaderValue(kvp.Key))
100-
{
101-
builder.Append('*');
102-
builder.AppendLine();
103-
}
104-
else
105-
{
106-
#if NET
107-
builder.AppendJoin(", ", (IEnumerable<object>)kvp.Value);
108-
builder.AppendLine();
109-
#else
110-
foreach (object value in (IEnumerable<object>)kvp.Value)
111-
{
112-
builder.Append(value);
113-
builder.Append(", ");
114-
}
115-
116-
// Remove the extra ', '
117-
builder.Remove(builder.Length - 2, 2);
118-
builder.AppendLine();
119-
#endif
120-
}
124+
builder.AppendLine((string)kvp.Value);
121125
}
122126

123127
_formatted = builder.ToString();

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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
using System;
55
using System.Collections.Generic;
6+
using System.Net.Http;
67
using System.Net.Http.Headers;
78
using Xunit;
89

@@ -47,7 +48,32 @@ public void HttpHeadersLogValue_ToString_HidesOnlyLogSensitiveHeadersValue()
4748
"unsecureHeader2: value2" + Environment.NewLine +
4849
"secureHeader2: *" + Environment.NewLine,
4950
result);
51+
52+
// Redaction is applied to the structured values, not just to the formatted string.
53+
Assert.Equal("secureHeader1", httpHeadersLogValue[0].Key);
54+
Assert.Equal("*", httpHeadersLogValue[0].Value);
55+
Assert.Equal("secureHeader2", httpHeadersLogValue[3].Key);
56+
Assert.Equal("*", httpHeadersLogValue[3].Value);
57+
}
58+
59+
#if NET
60+
[Fact]
61+
public void HttpHeadersLogValue_DoesNotValidateHeaderValues()
62+
{
63+
var request = new HttpRequestMessage(HttpMethod.Get, "https://example.com");
64+
request.Headers.TryAddWithoutValidation("Accept", "application/vnd.example+json;version=1");
65+
66+
var httpHeadersLogValue = new HttpHeadersLogValue(HttpHeadersLogValue.Kind.Request, request.Headers, contentHeaders: null, _ => false);
67+
68+
Assert.Equal(
69+
"Request Headers:" + Environment.NewLine +
70+
"Accept: application/vnd.example+json;version=1" + Environment.NewLine,
71+
httpHeadersLogValue.ToString());
72+
73+
Assert.True(request.Headers.NonValidated.TryGetValues("Accept", out HeaderStringValues values));
74+
Assert.Equal("application/vnd.example+json;version=1", Assert.Single(values));
5075
}
76+
#endif
5177

5278
private class TestHttpHeaders : HttpHeaders { }
5379
}

0 commit comments

Comments
 (0)