-
-
Save davidfowl/c34633f1ddc519f030a1c0c5abe8e867 to your computer and use it in GitHub Desktop.
public void ConfigureServices(IServiceCollection services) | |
{ | |
services.AddHttpClient("myclient"); | |
// Global header propagation for any HttpClient that comes from HttpClientFactory | |
services.AddHeaderPropagation(options => | |
{ | |
options.HeaderNames.Add("Correlation-Id"); | |
}); | |
} |
using System; | |
using System.Collections.Generic; | |
using System.Net.Http; | |
using System.Threading.Tasks; | |
using Microsoft.AspNetCore.Http; | |
using Microsoft.Extensions.DependencyInjection; | |
using Microsoft.Extensions.DependencyInjection.Extensions; | |
using Microsoft.Extensions.Http; | |
using Microsoft.Extensions.Options; | |
using Microsoft.Extensions.Primitives; | |
namespace HeaderPropagation | |
{ | |
public static class HeaderPropagationExtensions | |
{ | |
public static IServiceCollection AddHeaderPropagation(this IServiceCollection services, Action<HeaderPropagationOptions> configure) | |
{ | |
services.AddHttpContextAccessor(); | |
services.ConfigureAll(configure); | |
services.TryAddEnumerable(ServiceDescriptor.Singleton<IHttpMessageHandlerBuilderFilter, HeaderPropagationMessageHandlerBuilderFilter>()); | |
return services; | |
} | |
public static IHttpClientBuilder AddHeaderPropagation(this IHttpClientBuilder builder, Action<HeaderPropagationOptions> configure) | |
{ | |
builder.Services.AddHttpContextAccessor(); | |
builder.Services.Configure(builder.Name, configure); | |
builder.AddHttpMessageHandler((sp) => | |
{ | |
var options = sp.GetRequiredService<IOptionsMonitor<HeaderPropagationOptions>>(); | |
var contextAccessor = sp.GetRequiredService<IHttpContextAccessor>(); | |
return new HeaderPropagationMessageHandler(options.Get(builder.Name), contextAccessor); | |
}); | |
return builder; | |
} | |
} | |
internal class HeaderPropagationMessageHandlerBuilderFilter : IHttpMessageHandlerBuilderFilter | |
{ | |
private readonly HeaderPropagationOptions _options; | |
private readonly IHttpContextAccessor _contextAccessor; | |
public HeaderPropagationMessageHandlerBuilderFilter(IOptions<HeaderPropagationOptions> options, IHttpContextAccessor contextAccessor) | |
{ | |
_options = options.Value; | |
_contextAccessor = contextAccessor; | |
} | |
public Action<HttpMessageHandlerBuilder> Configure(Action<HttpMessageHandlerBuilder> next) | |
{ | |
return builder => | |
{ | |
builder.AdditionalHandlers.Add(new HeaderPropagationMessageHandler(_options, _contextAccessor)); | |
next(builder); | |
}; | |
} | |
} | |
public class HeaderPropagationOptions | |
{ | |
public IList<string> HeaderNames { get; set; } = new List<string>(); | |
} | |
public class HeaderPropagationMessageHandler : DelegatingHandler | |
{ | |
private readonly HeaderPropagationOptions _options; | |
private readonly IHttpContextAccessor _contextAccessor; | |
public HeaderPropagationMessageHandler(HeaderPropagationOptions options, IHttpContextAccessor contextAccessor) | |
{ | |
_options = options; | |
_contextAccessor = contextAccessor; | |
} | |
protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, System.Threading.CancellationToken cancellationToken) | |
{ | |
if (_contextAccessor.HttpContext != null) | |
{ | |
// REVIEW: This logic likely gets more fancy and allows mapping headers in more complex ways | |
foreach (var headerName in _options.HeaderNames) | |
{ | |
// Get the incoming header value | |
var headerValue = _contextAccessor.HttpContext.Request.Headers[headerName]; | |
if (StringValues.IsNullOrEmpty(headerValue)) | |
{ | |
continue; | |
} | |
request.Headers.TryAddWithoutValidation(headerName, (string[])headerValue); | |
} | |
} | |
return base.SendAsync(request, cancellationToken); | |
} | |
} | |
} |
Good catch! The same options instance is being used. I've updated the code in this gist.
That said, I wouldn't use this code, it's not really safe to use the IHttpContextAccesor in code that might run in parallel.
Use this middleware instead https://craftbakery.dev/http-header-propagation-aspnet-core/
Out of curiosity, @davidfowl why do you say that? If I look at here it has the "same" AsyncLocal pattern used in the HeaderPropagationValues that is used in [HeaderPropagationMiddleware].(https://github.com/dotnet/aspnetcore/blob/63d2fb07e93007af1f016d447a904ce187256881/src/Middleware/HeaderPropagation/src/HeaderPropagationMiddleware.cs#L18)
Am I missing something? Some internal edge case that I'm not seeing? 🤔
Yes, this is pretty bad and breaks my rules for AsyncLocal<T>
Hey @davidfowl - we are using the HeaderPropagationMiddleware for header propagation but we now have a situation where we must get a Boolean value from some API (we'd do this in a Middleware) and pass that value as a header to the all HttpClients within that HttpRequest (via a DelegatingHandler).
One (potential naive) naive way of doing this is
- saving the
value
in a ScopedService. - resolving the ScopedService from IHttpContextAccessor in the DelegatingHandler, getting the
value
and add the custom header.
(I believe Items
property in HttpContext could be used for this but it would still require a IHttpContextAccessor)
This works but worried that the usage of IHttpContextAccessor may not be safe as you mention (and as seen in some other online sources). Would you have a recommendation for this use case? Thanks!
the safest thing would be having your own async local.
Hey.
I found an issue with
IHttpClientBuilder AddHeaderPropagation(this IHttpClientBuilder builder, Action<HeaderPropagationOptions> configure)
.If we configure two HttpClient instances to handle different sets of Http headers
and then we use them like this:
both
HttpClient
s propagate both my-correlation-id and my-correlation-id1.I suppose the culprit is
builder.Services.Configure(configure)
.