Skip to content

Validate injected header values in DiagnosticsHandler #117884

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.

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 2 commits into from
Jul 22, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -356,12 +356,14 @@ private void InjectHeaders(Activity currentActivity, HttpRequestMessage request)
{
_propagator.Inject(currentActivity, request, static (carrier, key, value) =>
{
if (carrier is HttpRequestMessage request &&
key is not null &&
HeaderDescriptor.TryGet(key, out HeaderDescriptor descriptor) &&
!request.Headers.TryGetHeaderValue(descriptor, out _))
if (carrier is HttpRequestMessage request && key is not null)
{
request.Headers.TryAddWithoutValidation(descriptor, value);
HeaderDescriptor descriptor = request.Headers.GetHeaderDescriptor(key);

if (!request.Headers.Contains(descriptor))
{
request.Headers.Add(descriptor, value);
}
}
});
request.MarkPropagatorStateInjectedByDiagnosticsHandler();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1131,7 +1131,7 @@ private static void ParseAndAddValue(HeaderDescriptor descriptor, HeaderStoreIte
}
}

private HeaderDescriptor GetHeaderDescriptor(string name)
internal HeaderDescriptor GetHeaderDescriptor(string name)
{
ArgumentException.ThrowIfNullOrEmpty(name);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1406,6 +1406,27 @@ await GetFactoryForVersion(UseVersion).CreateClientAndServerAsync(
});
}

[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotBrowser))]
[InlineData("A B", "Foo", typeof(FormatException))] // Invalid header name
[InlineData("Content-Length", "42", typeof(InvalidOperationException))] // Invalid header name for the request headers collection
[InlineData("Foo", "Bar\nBaz", typeof(FormatException))] // Invalid header value
public async Task SendAsync_PropagatorInjectsInvalidHeaders_Throws(string headerName, string headerValue, Type exceptionType)
{
using Activity parent = new Activity("parent");
parent.SetIdFormat(ActivityIdFormat.W3C);
parent.Start();

using var handler = CreateSocketsHttpHandler(allowAllCertificates: true);
handler.ActivityHeadersPropagator = new DelegatingPropagator([headerName], (activity, carrier, setter) => setter(carrier, headerName, headerValue));

using var client = new HttpClient(handler);

// Url doesn't matter since the request should fail before hitting the network.
var request = CreateRequest(HttpMethod.Get, new Uri("https://microsoft.com"), UseVersion, exactVersion: true);

await Assert.ThrowsAsync(exceptionType, () => client.SendAsync(TestAsync, request));
}

public static IEnumerable<object[]> SocketsHttpHandler_ActivityCreation_MemberData()
{
foreach (var currentActivitySet in new bool[] {
Expand Down Expand Up @@ -1855,5 +1876,20 @@ private static void AssertHeadersAreInjected(HttpRequestData request, Activity p
var request = CreateRequest(HttpMethod.Get, uri, Version.Parse(useVersion), exactVersion: true);
return (request, await client.SendAsync(bool.Parse(testAsync), request, cancellationToken));
}

private sealed class DelegatingPropagator(string[] fields, Action<Activity?, object?, DistributedContextPropagator.PropagatorSetterCallback?> inject) : DistributedContextPropagator
{
public override IReadOnlyCollection<string> Fields => fields;

public override IEnumerable<KeyValuePair<string, string?>>? ExtractBaggage(object? carrier, PropagatorGetterCallback? getter) => [];

public override void ExtractTraceIdAndState(object? carrier, PropagatorGetterCallback? getter, out string? traceId, out string? traceState)
{
traceId = null;
traceState = null;
}

public override void Inject(Activity? activity, object? carrier, PropagatorSetterCallback? setter) => inject(activity, carrier, setter);
}
}
}
Loading