From e401f35b45e82f4c59a9b5d5e38138d1c62ef9fc Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Tue, 7 May 2019 11:07:42 -0700 Subject: [PATCH] Header propagation sample and UX improvements (#9793) Creating a sample for the header propagation package and some various misc UX improvements based on building the sample. A small list: - allow duplicate inbound names - de-dupe based on outbound names - add sugar for configuration - simplify pattern for transforming values - add error message for missing middleware Also a few small perf things. I started this out by wanting to remove the following from the configuration pattern: ```C# options.Headers.Add("X-TraceId", null); ``` This pattern with null is undiscoverable, but we didn't provide something simpler. The most common case was to add a custom collection type so we can define sugar methods. The next realization is that in practical case (dist tracing sample) you either way to *key* off of the same inbound header twice, or you don't have an inbound header at all, and you will synthesize the value every time. This means that the way we're treating inbound header names is a bit wrong. We don't want inbound header names to be unique, we want *outbound header names to be unique*. Next, I want to consolidate DefaultValue and ValueFactory. The problems I saw with this: - DefaultValue is a trap. It's rare to use a static value. - ValueFactory really wants the header name *and* value I think what's there now is much more terse to work with. --- ...NetCore.HeaderPropagation.netcoreapp3.0.cs | 29 +++- .../HeaderPropagationSample.csproj | 14 ++ .../HeaderPropagationSample/Program.cs | 21 +++ .../HeaderPropagationSample/Startup.cs | 128 ++++++++++++++++++ .../appsettings.Development.json | 9 ++ .../HeaderPropagationSample/appsettings.json | 10 ++ .../src/HeaderPropagationContext.cs | 54 ++++++++ .../src/HeaderPropagationEntry.cs | 82 ++++++----- .../src/HeaderPropagationEntryCollection.cs | 108 +++++++++++++++ .../src/HeaderPropagationMessageHandler.cs | 31 +++-- .../src/HeaderPropagationMiddleware.cs | 40 ++++-- .../src/HeaderPropagationOptions.cs | 11 +- .../src/HeaderPropagationValues.cs | 17 ++- .../test/HeaderPropagationIntegrationTest.cs | 55 +++++++- .../HeaderPropagationMessageHandlerTest.cs | 49 +++---- .../test/HeaderPropagationMiddlewareTest.cs | 84 ++++-------- src/Middleware/Middleware.sln | 18 +++ 17 files changed, 595 insertions(+), 165 deletions(-) create mode 100644 src/Middleware/HeaderPropagation/samples/HeaderPropagationSample/HeaderPropagationSample.csproj create mode 100644 src/Middleware/HeaderPropagation/samples/HeaderPropagationSample/Program.cs create mode 100644 src/Middleware/HeaderPropagation/samples/HeaderPropagationSample/Startup.cs create mode 100644 src/Middleware/HeaderPropagation/samples/HeaderPropagationSample/appsettings.Development.json create mode 100644 src/Middleware/HeaderPropagation/samples/HeaderPropagationSample/appsettings.json create mode 100644 src/Middleware/HeaderPropagation/src/HeaderPropagationContext.cs create mode 100644 src/Middleware/HeaderPropagation/src/HeaderPropagationEntryCollection.cs diff --git a/src/Middleware/HeaderPropagation/ref/Microsoft.AspNetCore.HeaderPropagation.netcoreapp3.0.cs b/src/Middleware/HeaderPropagation/ref/Microsoft.AspNetCore.HeaderPropagation.netcoreapp3.0.cs index c1c6efd0de..50c4584c37 100644 --- a/src/Middleware/HeaderPropagation/ref/Microsoft.AspNetCore.HeaderPropagation.netcoreapp3.0.cs +++ b/src/Middleware/HeaderPropagation/ref/Microsoft.AspNetCore.HeaderPropagation.netcoreapp3.0.cs @@ -10,12 +10,29 @@ namespace Microsoft.AspNetCore.Builder } namespace Microsoft.AspNetCore.HeaderPropagation { + [System.Runtime.InteropServices.StructLayoutAttribute(System.Runtime.InteropServices.LayoutKind.Sequential)] + public readonly partial struct HeaderPropagationContext + { + private readonly object _dummy; + public HeaderPropagationContext(Microsoft.AspNetCore.Http.HttpContext httpContext, string headerName, Microsoft.Extensions.Primitives.StringValues headerValue) { throw null; } + public string HeaderName { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } } + public Microsoft.Extensions.Primitives.StringValues HeaderValue { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } } + public Microsoft.AspNetCore.Http.HttpContext HttpContext { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } } + } public partial class HeaderPropagationEntry { - public HeaderPropagationEntry() { } - public Microsoft.Extensions.Primitives.StringValues DefaultValue { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } } - public string OutboundHeaderName { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } } - public System.Func ValueFactory { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } } + public HeaderPropagationEntry(string inboundHeaderName, string outboundHeaderName, System.Func valueFilter) { } + public string InboundHeaderName { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } } + public string OutboundHeaderName { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } } + public System.Func ValueFilter { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } } + } + public sealed partial class HeaderPropagationEntryCollection : System.Collections.ObjectModel.Collection + { + public HeaderPropagationEntryCollection() { } + public void Add(string headerName) { } + public void Add(string headerName, System.Func valueFilter) { } + public void Add(string inboundHeaderName, string outboundHeaderName) { } + public void Add(string inboundHeaderName, string outboundHeaderName, System.Func valueFilter) { } } public partial class HeaderPropagationMessageHandler : System.Net.Http.DelegatingHandler { @@ -30,12 +47,12 @@ namespace Microsoft.AspNetCore.HeaderPropagation public partial class HeaderPropagationOptions { public HeaderPropagationOptions() { } - public System.Collections.Generic.IDictionary Headers { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } } + public Microsoft.AspNetCore.HeaderPropagation.HeaderPropagationEntryCollection Headers { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } } } public partial class HeaderPropagationValues { public HeaderPropagationValues() { } - public System.Collections.Generic.IDictionary Headers { get { throw null; } } + public System.Collections.Generic.IDictionary Headers { get { throw null; } set { } } } } namespace Microsoft.Extensions.DependencyInjection diff --git a/src/Middleware/HeaderPropagation/samples/HeaderPropagationSample/HeaderPropagationSample.csproj b/src/Middleware/HeaderPropagation/samples/HeaderPropagationSample/HeaderPropagationSample.csproj new file mode 100644 index 0000000000..34c4dfc8b6 --- /dev/null +++ b/src/Middleware/HeaderPropagation/samples/HeaderPropagationSample/HeaderPropagationSample.csproj @@ -0,0 +1,14 @@ + + + + netcoreapp3.0 + + + + + + + + + + diff --git a/src/Middleware/HeaderPropagation/samples/HeaderPropagationSample/Program.cs b/src/Middleware/HeaderPropagation/samples/HeaderPropagationSample/Program.cs new file mode 100644 index 0000000000..bbfb777399 --- /dev/null +++ b/src/Middleware/HeaderPropagation/samples/HeaderPropagationSample/Program.cs @@ -0,0 +1,21 @@ +using System.IO; +using Microsoft.AspNetCore.Hosting; +using Microsoft.Extensions.Hosting; + +namespace HeaderPropagationSample +{ + public class Program + { + public static void Main(string[] args) + { + CreateHostBuilder(args).Build().Run(); + } + + public static IHostBuilder CreateHostBuilder(string[] args) => + Host.CreateDefaultBuilder(args) + .ConfigureWebHost(webBuilder => + { + webBuilder.UseStartup(); + }); + } +} diff --git a/src/Middleware/HeaderPropagation/samples/HeaderPropagationSample/Startup.cs b/src/Middleware/HeaderPropagation/samples/HeaderPropagationSample/Startup.cs new file mode 100644 index 0000000000..e404fc15e4 --- /dev/null +++ b/src/Middleware/HeaderPropagation/samples/HeaderPropagationSample/Startup.cs @@ -0,0 +1,128 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Net.Http; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.HeaderPropagation; +using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Http.Extensions; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Primitives; + +namespace HeaderPropagationSample +{ + public class Startup + { + public void ConfigureServices(IServiceCollection services) + { + services.AddRouting(); + + // A sample for configuration for propagating an A/B testing header. If we see + // the X-BetaFeatures header then we forward it to outgoing calls. If we don't + // see it, then we generate a new value randomly. + // + // To see this in action, send a request to / + // - If you do not specify X-BetaFeatures the server will generate a new value + // - If you specify X-BetaFeatures then you will see the value propagated + // + // This demonstrates two common uses of header propagation: + // 1. Forward a header as-is + // 2. Generate a new header value, or conditionally generate a header value + // + // It's also easy to forward a header with a different name, using Add(string, string) + services.AddHeaderPropagation(options => + { + // Propagate the X-BetaFeatures if present. + options.Headers.Add("X-BetaFeatures"); + + // Generate a new X-BetaFeatures if not present. + options.Headers.Add("X-BetaFeatures", context => + { + return GenerateBetaFeatureOptions(); + }); + }); + + services + .AddHttpClient("test") + .AddHeaderPropagation(); + + } + + public void Configure(IApplicationBuilder app, IWebHostEnvironment env, IHttpClientFactory clientFactory) + { + if (env.IsDevelopment()) + { + app.UseDeveloperExceptionPage(); + } + + app.UseHeaderPropagation(); + + app.UseRouting(); + + app.UseEndpoints(endpoints => + { + endpoints.MapGet("/", async context => + { + foreach (var header in context.Request.Headers) + { + await context.Response.WriteAsync($"'/' Got Header '{header.Key}': {string.Join(", ", header.Value)}\r\n"); + } + + await context.Response.WriteAsync("Sending request to /forwarded\r\n"); + + var uri = UriHelper.BuildAbsolute(context.Request.Scheme, context.Request.Host, context.Request.PathBase, "/forwarded"); + var client = clientFactory.CreateClient("test"); + var response = await client.GetAsync(uri); + + foreach (var header in response.RequestMessage.Headers) + { + await context.Response.WriteAsync($"Sent Header '{header.Key}': {string.Join(", ", header.Value)}\r\n"); + } + + await context.Response.WriteAsync("Got response\r\n"); + await context.Response.WriteAsync(await response.Content.ReadAsStringAsync()); + }); + + endpoints.MapGet("/forwarded", async context => + { + foreach (var header in context.Request.Headers) + { + await context.Response.WriteAsync($"'/forwarded' Got Header '{header.Key}': {string.Join(", ", header.Value)}\r\n"); + } + }); + }); + } + + private static StringValues GenerateBetaFeatureOptions() + { + var features = new string[] + { + "Widgets", + "Social", + "Speedy-Checkout", + }; + + var threshold = 0.80; // 20% chance for each feature in beta. + + var random = new Random(); + var values = new List(); + for (var i = 0; i < features.Length; i++) + { + if (random.NextDouble() > threshold) + { + values.Add(features[i]); + } + } + + if (values.Count == 0) + { + return new StringValues("none"); + } + + return new StringValues(values.ToArray()); + } + } +} diff --git a/src/Middleware/HeaderPropagation/samples/HeaderPropagationSample/appsettings.Development.json b/src/Middleware/HeaderPropagation/samples/HeaderPropagationSample/appsettings.Development.json new file mode 100644 index 0000000000..e203e9407e --- /dev/null +++ b/src/Middleware/HeaderPropagation/samples/HeaderPropagationSample/appsettings.Development.json @@ -0,0 +1,9 @@ +{ + "Logging": { + "LogLevel": { + "Default": "Debug", + "System": "Information", + "Microsoft": "Information" + } + } +} diff --git a/src/Middleware/HeaderPropagation/samples/HeaderPropagationSample/appsettings.json b/src/Middleware/HeaderPropagation/samples/HeaderPropagationSample/appsettings.json new file mode 100644 index 0000000000..d9d9a9bff6 --- /dev/null +++ b/src/Middleware/HeaderPropagation/samples/HeaderPropagationSample/appsettings.json @@ -0,0 +1,10 @@ +{ + "Logging": { + "LogLevel": { + "Default": "Information", + "Microsoft": "Warning", + "Microsoft.Hosting.Lifetime": "Information" + } + }, + "AllowedHosts": "*" +} diff --git a/src/Middleware/HeaderPropagation/src/HeaderPropagationContext.cs b/src/Middleware/HeaderPropagation/src/HeaderPropagationContext.cs new file mode 100644 index 0000000000..bacddeae00 --- /dev/null +++ b/src/Middleware/HeaderPropagation/src/HeaderPropagationContext.cs @@ -0,0 +1,54 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.Primitives; + +namespace Microsoft.AspNetCore.HeaderPropagation +{ + /// + /// A context object for delegates. + /// + public readonly struct HeaderPropagationContext + { + /// + /// Initializes a new instance of with the provided + /// , and . + /// + /// The associated with the current request. + /// The header name. + /// The header value present in the current request. + public HeaderPropagationContext(HttpContext httpContext, string headerName, StringValues headerValue) + { + if (httpContext == null) + { + throw new ArgumentNullException(nameof(httpContext)); + } + + if (headerName == null) + { + throw new ArgumentNullException(nameof(headerName)); + } + + HttpContext = httpContext; + HeaderName = headerName; + HeaderValue = headerValue; + } + + /// + /// Gets the associated with the current request. + /// + public HttpContext HttpContext { get; } + + /// + /// Gets the header name. + /// + public string HeaderName { get; } + + /// + /// Gets the header value from the current request. + /// + public StringValues HeaderValue { get; } + } +} diff --git a/src/Middleware/HeaderPropagation/src/HeaderPropagationEntry.cs b/src/Middleware/HeaderPropagation/src/HeaderPropagationEntry.cs index 5c9257400d..197c7d71ee 100644 --- a/src/Middleware/HeaderPropagation/src/HeaderPropagationEntry.cs +++ b/src/Middleware/HeaderPropagation/src/HeaderPropagationEntry.cs @@ -2,7 +2,6 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using Microsoft.AspNetCore.Http; using Microsoft.Extensions.Primitives; namespace Microsoft.AspNetCore.HeaderPropagation @@ -13,44 +12,61 @@ namespace Microsoft.AspNetCore.HeaderPropagation public class HeaderPropagationEntry { /// - /// Gets or sets the name of the header to be used by the for the + /// Creates a new with the provided , + /// , and + /// + /// + /// The name of the header to be captured by . + /// + /// + /// The name of the header to be added by . + /// + /// + /// A filter delegate that can be used to transform the header value. May be null. + /// + public HeaderPropagationEntry( + string inboundHeaderName, + string outboundHeaderName, + Func valueFilter) + { + if (inboundHeaderName == null) + { + throw new ArgumentNullException(nameof(inboundHeaderName)); + } + + if (outboundHeaderName == null) + { + throw new ArgumentNullException(nameof(outboundHeaderName)); + } + + InboundHeaderName = inboundHeaderName; + OutboundHeaderName = outboundHeaderName; + ValueFilter = valueFilter; // May be null + } + + /// + /// Gets the name of the header that will be captured by the . + /// + public string InboundHeaderName { get; } + + /// + /// Gets the name of the header to be used by the for the /// outbound http requests. /// - /// - /// If is present, the value of the header in the outbound calls will be the one - /// returned by the factory or, if the factory returns an empty value, the header will be omitted. - /// Otherwise, it will be the value of the header in the incoming request named as the key of this entry in - /// or, if missing or empty, the value specified in - /// or, if the is empty, it will not be - /// added to the outbound calls. - /// - public string OutboundHeaderName { get; set; } + public string OutboundHeaderName { get; } /// - /// Gets or sets the default value to be used when the header in the incoming request is missing or empty. + /// Gets or sets a filter delegate that can be used to transform the header value. /// /// - /// This value is ignored when is set. - /// When it is it has no effect and, if the header is missing or empty in the - /// incoming request, it will not be added to outbound calls. + /// + /// When present, the delegate will be evaluated once per request to provide the transformed + /// header value. The delegate will be called regardless of whether a header with the name + /// corresponding to is present in the request. If the result + /// of evaluating is null or empty, it will not be added to the propagated + /// values. + /// /// - public StringValues DefaultValue { get; set; } - - /// - /// Gets or sets the value factory to be used. - /// It gets as input the inbound header name for this entry as defined in - /// and the of the current request. - /// - /// - /// When present, the factory is the only method used to set the value. - /// The factory should return to not add the header. - /// When not present, the value will be taken from the header in the incoming request named as the key of this - /// entry in or, if missing or empty, it will be the values - /// specified in or, if the is empty, the header will not - /// be added to the outbound calls. - /// Please note the factory is called only once per incoming request and the same value will be used by all the - /// outbound calls. - /// - public Func ValueFactory { get; set; } + public Func ValueFilter { get; } } } diff --git a/src/Middleware/HeaderPropagation/src/HeaderPropagationEntryCollection.cs b/src/Middleware/HeaderPropagation/src/HeaderPropagationEntryCollection.cs new file mode 100644 index 0000000000..86eec5d9df --- /dev/null +++ b/src/Middleware/HeaderPropagation/src/HeaderPropagationEntryCollection.cs @@ -0,0 +1,108 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Collections.ObjectModel; +using Microsoft.Extensions.Primitives; + +namespace Microsoft.AspNetCore.HeaderPropagation +{ + /// + /// A collection of items. + /// + public sealed class HeaderPropagationEntryCollection : Collection + { + /// + /// Adds an that will use as + /// the value of and + /// . + /// + /// The header name to be propagated. + public void Add(string headerName) + { + if (headerName == null) + { + throw new ArgumentNullException(nameof(headerName)); + } + + Add(new HeaderPropagationEntry(headerName, headerName, valueFilter: null)); + } + + /// + /// Adds an that will use as + /// the value of and + /// . + /// + /// The header name to be propagated. + /// + /// A filter delegate that can be used to transform the header value. + /// . + /// + public void Add(string headerName, Func valueFilter) + { + if (headerName == null) + { + throw new ArgumentNullException(nameof(headerName)); + } + + Add(new HeaderPropagationEntry(headerName, headerName, valueFilter)); + } + + /// + /// Adds an that will use the provided + /// and . + /// + /// + /// The name of the header to be captured by . + /// + /// + /// The name of the header to be added by . + /// + public void Add(string inboundHeaderName, string outboundHeaderName) + { + if (inboundHeaderName == null) + { + throw new ArgumentNullException(nameof(inboundHeaderName)); + } + + if (outboundHeaderName == null) + { + throw new ArgumentNullException(nameof(outboundHeaderName)); + } + + Add(new HeaderPropagationEntry(inboundHeaderName, outboundHeaderName, valueFilter: null)); + } + + /// + /// Adds an that will use the provided , + /// , and . + /// + /// + /// The name of the header to be captured by . + /// + /// + /// The name of the header to be added by . + /// + /// + /// A filter delegate that can be used to transform the header value. + /// . + /// + public void Add( + string inboundHeaderName, + string outboundHeaderName, + Func valueFilter) + { + if (inboundHeaderName == null) + { + throw new ArgumentNullException(nameof(inboundHeaderName)); + } + + if (outboundHeaderName == null) + { + throw new ArgumentNullException(nameof(outboundHeaderName)); + } + + Add(new HeaderPropagationEntry(inboundHeaderName, outboundHeaderName, valueFilter)); + } + } +} diff --git a/src/Middleware/HeaderPropagation/src/HeaderPropagationMessageHandler.cs b/src/Middleware/HeaderPropagation/src/HeaderPropagationMessageHandler.cs index 524e29856f..c9d0df9d87 100644 --- a/src/Middleware/HeaderPropagation/src/HeaderPropagationMessageHandler.cs +++ b/src/Middleware/HeaderPropagation/src/HeaderPropagationMessageHandler.cs @@ -5,6 +5,7 @@ using System; using System.Net.Http; using System.Threading; using System.Threading.Tasks; +using Microsoft.AspNetCore.Builder; using Microsoft.Extensions.Options; using Microsoft.Extensions.Primitives; @@ -49,32 +50,44 @@ namespace Microsoft.AspNetCore.HeaderPropagation /// The task object representing the asynchronous operation. protected override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) { - foreach ((var headerName, var entry) in _options.Headers) + var captured = _values.Headers; + if (captured == null) { - var outputName = string.IsNullOrEmpty(entry?.OutboundHeaderName) ? headerName : entry.OutboundHeaderName; + var message = + $"The {nameof(HeaderPropagationValues)}.{nameof(HeaderPropagationValues.Headers)} property has not been " + + $"initialized. Register the header propagation middleware by adding 'app.{nameof(HeaderPropagationApplicationBuilderExtensions.UseHeaderPropagation)}() " + + $"in the 'Configure(...)' method."; + throw new InvalidOperationException(message); + } + // Perf: We iterate _options.Headers instead of iterating _values.Headers because iterating an IDictionary + // will allocate. Also avoiding foreach since we don't define a struct-enumerator. + var entries = _options.Headers; + for (var i = 0; i < entries.Count; i++) + { + var entry = entries[i]; var hasContent = request.Content != null; - if (!request.Headers.TryGetValues(outputName, out var _) && - !(hasContent && request.Content.Headers.TryGetValues(outputName, out var _))) + if (!request.Headers.TryGetValues(entry.OutboundHeaderName, out var _) && + !(hasContent && request.Content.Headers.TryGetValues(entry.OutboundHeaderName, out var _))) { - if (_values.Headers.TryGetValue(headerName, out var stringValues) && + if (captured.TryGetValue(entry.OutboundHeaderName, out var stringValues) && !StringValues.IsNullOrEmpty(stringValues)) { if (stringValues.Count == 1) { var value = (string)stringValues; - if (!request.Headers.TryAddWithoutValidation(outputName, value) && hasContent) + if (!request.Headers.TryAddWithoutValidation(entry.OutboundHeaderName, value) && hasContent) { - request.Content.Headers.TryAddWithoutValidation(outputName, value); + request.Content.Headers.TryAddWithoutValidation(entry.OutboundHeaderName, value); } } else { var values = (string[])stringValues; - if (!request.Headers.TryAddWithoutValidation(outputName, values) && hasContent) + if (!request.Headers.TryAddWithoutValidation(entry.OutboundHeaderName, values) && hasContent) { - request.Content.Headers.TryAddWithoutValidation(outputName, values); + request.Content.Headers.TryAddWithoutValidation(entry.OutboundHeaderName, values); } } } diff --git a/src/Middleware/HeaderPropagation/src/HeaderPropagationMiddleware.cs b/src/Middleware/HeaderPropagation/src/HeaderPropagationMiddleware.cs index 4183faa0fa..ed18acd3ac 100644 --- a/src/Middleware/HeaderPropagation/src/HeaderPropagationMiddleware.cs +++ b/src/Middleware/HeaderPropagation/src/HeaderPropagationMiddleware.cs @@ -35,33 +35,43 @@ namespace Microsoft.AspNetCore.HeaderPropagation public Task Invoke(HttpContext context) { - foreach ((var headerName, var entry) in _options.Headers) - { - var values = GetValues(headerName, entry, context); + // We need to intialize the headers because the message handler will use this to detect misconfiguration. + var headers = _values.Headers ??= new Dictionary(StringComparer.OrdinalIgnoreCase); - if (!StringValues.IsNullOrEmpty(values)) + // Perf: avoid foreach since we don't define a struct enumerator. + var entries = _options.Headers; + for (var i = 0; i < entries.Count; i++) + { + var entry = entries[i]; + + // We intentionally process entries in order, and allow earlier entries to + // take precedence over later entries when they have the same output name. + if (!headers.ContainsKey(entry.OutboundHeaderName)) { - _values.Headers.TryAdd(headerName, values); + var value = GetValue(context, entry); + if (!StringValues.IsNullOrEmpty(value)) + { + headers.Add(entry.OutboundHeaderName, value); + } } } return _next.Invoke(context); } - private static StringValues GetValues(string headerName, HeaderPropagationEntry entry, HttpContext context) + private static StringValues GetValue(HttpContext context, HeaderPropagationEntry entry) { - if (entry?.ValueFactory != null) + context.Request.Headers.TryGetValue(entry.InboundHeaderName, out var value); + if (entry.ValueFilter != null) { - return entry.ValueFactory(headerName, context); + var filtered = entry.ValueFilter(new HeaderPropagationContext(context, entry.InboundHeaderName, value)); + if (!StringValues.IsNullOrEmpty(filtered)) + { + value = filtered; + } } - if (context.Request.Headers.TryGetValue(headerName, out var values) - && !StringValues.IsNullOrEmpty(values)) - { - return values; - } - - return entry != null ? entry.DefaultValue : StringValues.Empty; + return value; } } } diff --git a/src/Middleware/HeaderPropagation/src/HeaderPropagationOptions.cs b/src/Middleware/HeaderPropagation/src/HeaderPropagationOptions.cs index ffd04583f0..e5f26e831b 100644 --- a/src/Middleware/HeaderPropagation/src/HeaderPropagationOptions.cs +++ b/src/Middleware/HeaderPropagation/src/HeaderPropagationOptions.cs @@ -1,8 +1,6 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. -using System.Collections.Generic; - namespace Microsoft.AspNetCore.HeaderPropagation { /// @@ -11,9 +9,14 @@ namespace Microsoft.AspNetCore.HeaderPropagation public class HeaderPropagationOptions { /// - /// Gets or sets the headers to be collected by the + /// Gets or sets the headers to be captured by the /// and to be propagated by the . /// - public IDictionary Headers { get; set; } = new Dictionary(); + /// + /// Entries in are processes in order while capturing headers inside + /// . This can cause an earlier entry to take precedence + /// over a later entry if they have the same . + /// + public HeaderPropagationEntryCollection Headers { get; set; } = new HeaderPropagationEntryCollection(); } } diff --git a/src/Middleware/HeaderPropagation/src/HeaderPropagationValues.cs b/src/Middleware/HeaderPropagation/src/HeaderPropagationValues.cs index 8c3d9ab26d..fdf0067a58 100644 --- a/src/Middleware/HeaderPropagation/src/HeaderPropagationValues.cs +++ b/src/Middleware/HeaderPropagation/src/HeaderPropagationValues.cs @@ -4,25 +4,34 @@ using System; using System.Collections.Generic; using System.Threading; +using Microsoft.AspNetCore.Builder; using Microsoft.Extensions.Primitives; namespace Microsoft.AspNetCore.HeaderPropagation { /// - /// Contains the headers values for the . + /// Contains the outbound header values for the . /// public class HeaderPropagationValues { - private readonly static AsyncLocal> _headers = new AsyncLocal>(); + private readonly static AsyncLocal> _headers = new AsyncLocal>(); /// - /// Gets the headers values collected by the from the current request that can be propagated. + /// Gets or sets the headers values collected by the from the current request + /// that can be propagated. /// + /// + /// The keys of correspond to . + /// public IDictionary Headers { get { - return _headers.Value ?? (_headers.Value = new Dictionary(StringComparer.OrdinalIgnoreCase)); + return _headers.Value; + } + set + { + _headers.Value = value; } } } diff --git a/src/Middleware/HeaderPropagation/test/HeaderPropagationIntegrationTest.cs b/src/Middleware/HeaderPropagation/test/HeaderPropagationIntegrationTest.cs index 207ececc63..60a417e883 100644 --- a/src/Middleware/HeaderPropagation/test/HeaderPropagationIntegrationTest.cs +++ b/src/Middleware/HeaderPropagation/test/HeaderPropagationIntegrationTest.cs @@ -18,16 +18,63 @@ namespace Microsoft.AspNetCore.HeaderPropagation.Tests { public class HeaderPropagationIntegrationTest { + [Fact] + public async Task HeaderPropagation_WithoutMiddleware_Throws() + { + // Arrange + Exception captured = null; + + var builder = new WebHostBuilder() + .ConfigureServices(services => + { + services.AddHttpClient("test").AddHeaderPropagation(); + services.AddHeaderPropagation(options => + { + options.Headers.Add("X-TraceId"); + }); + }) + .Configure(app => + { + // note: no header propagation middleware + + app.Run(async context => + { + try + { + var client = context.RequestServices.GetRequiredService().CreateClient("test"); + await client.GetAsync("http://localhost/"); // will throw + } + catch (Exception ex) + { + captured = ex; + } + }); + }); + + var server = new TestServer(builder); + var client = server.CreateClient(); + + var request = new HttpRequestMessage(); + + // Act + var response = await client.SendAsync(request); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.IsType(captured); + Assert.Equal( + "The HeaderPropagationValues.Headers property has not been initialized. Register the header propagation middleware " + + "by adding 'app.UseHeaderPropagation() in the 'Configure(...)' method.", + captured.Message); + } + [Fact] public async Task HeaderInRequest_AddCorrectValue() { // Arrange var handler = new SimpleHandler(); var builder = CreateBuilder(c => - c.Headers.Add("in", new HeaderPropagationEntry - { - OutboundHeaderName = "out", - }), + c.Headers.Add("in", "out"), handler); var server = new TestServer(builder); var client = server.CreateClient(); diff --git a/src/Middleware/HeaderPropagation/test/HeaderPropagationMessageHandlerTest.cs b/src/Middleware/HeaderPropagation/test/HeaderPropagationMessageHandlerTest.cs index a824fb13d1..d19348c3d0 100644 --- a/src/Middleware/HeaderPropagation/test/HeaderPropagationMessageHandlerTest.cs +++ b/src/Middleware/HeaderPropagation/test/HeaderPropagationMessageHandlerTest.cs @@ -2,11 +2,13 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Collections.Generic; using System.Net.Http; using System.Net.Http.Headers; using System.Threading; using System.Threading.Tasks; using Microsoft.Extensions.Options; +using Microsoft.Extensions.Primitives; using Xunit; namespace Microsoft.AspNetCore.HeaderPropagation.Tests @@ -18,6 +20,8 @@ namespace Microsoft.AspNetCore.HeaderPropagation.Tests Handler = new SimpleHandler(); State = new HeaderPropagationValues(); + State.Headers = new Dictionary(StringComparer.OrdinalIgnoreCase); + Configuration = new HeaderPropagationOptions(); var headerPropagationMessageHandler = @@ -41,8 +45,8 @@ namespace Microsoft.AspNetCore.HeaderPropagation.Tests public async Task HeaderInState_AddCorrectValue() { // Arrange - Configuration.Headers.Add("in", new HeaderPropagationEntry { OutboundHeaderName = "out" }); - State.Headers.Add("in", "test"); + Configuration.Headers.Add("in", "out"); + State.Headers.Add("out", "test"); // Act await Client.SendAsync(new HttpRequestMessage()); @@ -56,8 +60,8 @@ namespace Microsoft.AspNetCore.HeaderPropagation.Tests public async Task HeaderInState_WithMultipleValues_AddAllValues() { // Arrange - Configuration.Headers.Add("in", new HeaderPropagationEntry { OutboundHeaderName = "out" }); - State.Headers.Add("in", new[] { "one", "two" }); + Configuration.Headers.Add("in", "out"); + State.Headers.Add("out", new[] { "one", "two" }); // Act await Client.SendAsync(new HttpRequestMessage()); @@ -70,7 +74,7 @@ namespace Microsoft.AspNetCore.HeaderPropagation.Tests [Fact] public async Task HeaderInState_RequestWithContent_ContentHeaderPresent_DoesNotAddIt() { - Configuration.Headers.Add("in", new HeaderPropagationEntry() { OutboundHeaderName = "Content-Type" }); + Configuration.Headers.Add("in", "Content-Type"); State.Headers.Add("in", "test"); // Act @@ -84,8 +88,8 @@ namespace Microsoft.AspNetCore.HeaderPropagation.Tests [Fact] public async Task HeaderInState_RequestWithContent_ContentHeaderNotPresent_AddValue() { - Configuration.Headers.Add("in", new HeaderPropagationEntry() { OutboundHeaderName = "Content-Language" }); - State.Headers.Add("in", "test"); + Configuration.Headers.Add("in", "Content-Language"); + State.Headers.Add("Content-Language", "test"); // Act await Client.SendAsync(new HttpRequestMessage() { Content = new StringContent("test") }); @@ -98,8 +102,8 @@ namespace Microsoft.AspNetCore.HeaderPropagation.Tests [Fact] public async Task HeaderInState_WithMultipleValues_RequestWithContent_ContentHeaderNotPresent_AddAllValues() { - Configuration.Headers.Add("in", new HeaderPropagationEntry() { OutboundHeaderName = "Content-Language" }); - State.Headers.Add("in", new[] { "one", "two" }); + Configuration.Headers.Add("in", "Content-Language"); + State.Headers.Add("Content-Language", new[] { "one", "two" }); // Act await Client.SendAsync(new HttpRequestMessage() { Content = new StringContent("test") }); @@ -113,7 +117,7 @@ namespace Microsoft.AspNetCore.HeaderPropagation.Tests public async Task HeaderInState_NoOutputName_UseInputName() { // Arrange - Configuration.Headers.Add("in", new HeaderPropagationEntry()); + Configuration.Headers.Add("in"); State.Headers.Add("in", "test"); // Act @@ -128,7 +132,7 @@ namespace Microsoft.AspNetCore.HeaderPropagation.Tests public async Task NoHeaderInState_DoesNotAddIt() { // Arrange - Configuration.Headers.Add("inout", new HeaderPropagationEntry()); + Configuration.Headers.Add("inout"); // Act await Client.SendAsync(new HttpRequestMessage()); @@ -154,8 +158,8 @@ namespace Microsoft.AspNetCore.HeaderPropagation.Tests public async Task MultipleHeadersInState_AddsAll() { // Arrange - Configuration.Headers.Add("inout", new HeaderPropagationEntry()); - Configuration.Headers.Add("another", new HeaderPropagationEntry()); + Configuration.Headers.Add("inout"); + Configuration.Headers.Add("another"); State.Headers.Add("inout", "test"); State.Headers.Add("another", "test2"); @@ -175,7 +179,7 @@ namespace Microsoft.AspNetCore.HeaderPropagation.Tests public async Task HeaderEmptyInState_DoNotAddIt(string headerValue) { // Arrange - Configuration.Headers.Add("inout", new HeaderPropagationEntry()); + Configuration.Headers.Add("inout"); State.Headers.Add("inout", headerValue); // Act @@ -194,7 +198,7 @@ namespace Microsoft.AspNetCore.HeaderPropagation.Tests { // Arrange State.Headers.Add("inout", "test"); - Configuration.Headers.Add("inout", new HeaderPropagationEntry()); + Configuration.Headers.Add("inout"); var request = new HttpRequestMessage(); request.Headers.Add("inout", outgoingValue); @@ -207,21 +211,6 @@ namespace Microsoft.AspNetCore.HeaderPropagation.Tests Assert.Equal(expectedValues, Handler.Headers.GetValues("inout")); } - [Fact] - public async Task NullEntryInConfiguration_AddCorrectValue() - { - // Arrange - Configuration.Headers.Add("in", null); - State.Headers.Add("in", "test"); - - // Act - await Client.SendAsync(new HttpRequestMessage()); - - // Assert - Assert.True(Handler.Headers.Contains("in")); - Assert.Equal(new[] { "test" }, Handler.Headers.GetValues("in")); - } - private class SimpleHandler : DelegatingHandler { public HttpHeaders Headers { get; private set; } diff --git a/src/Middleware/HeaderPropagation/test/HeaderPropagationMiddlewareTest.cs b/src/Middleware/HeaderPropagation/test/HeaderPropagationMiddlewareTest.cs index ad9e8e10d2..f6576d2d68 100644 --- a/src/Middleware/HeaderPropagation/test/HeaderPropagationMiddlewareTest.cs +++ b/src/Middleware/HeaderPropagation/test/HeaderPropagationMiddlewareTest.cs @@ -32,7 +32,7 @@ namespace Microsoft.AspNetCore.HeaderPropagation.Tests public async Task HeaderInRequest_AddCorrectValue() { // Arrange - Configuration.Headers.Add("in", new HeaderPropagationEntry()); + Configuration.Headers.Add("in"); Context.Request.Headers.Add("in", "test"); // Act @@ -47,7 +47,7 @@ namespace Microsoft.AspNetCore.HeaderPropagation.Tests public async Task NoHeaderInRequest_DoesNotAddIt() { // Arrange - Configuration.Headers.Add("in", new HeaderPropagationEntry()); + Configuration.Headers.Add("in"); // Act await Middleware.Invoke(Context); @@ -73,8 +73,8 @@ namespace Microsoft.AspNetCore.HeaderPropagation.Tests public async Task MultipleHeadersInRequest_AddAllHeaders() { // Arrange - Configuration.Headers.Add("in", new HeaderPropagationEntry()); - Configuration.Headers.Add("another", new HeaderPropagationEntry()); + Configuration.Headers.Add("in"); + Configuration.Headers.Add("another"); Context.Request.Headers.Add("in", "test"); Context.Request.Headers.Add("another", "test2"); @@ -94,7 +94,7 @@ namespace Microsoft.AspNetCore.HeaderPropagation.Tests public async Task HeaderEmptyInRequest_DoesNotAddIt(string headerValue) { // Arrange - Configuration.Headers.Add("in", new HeaderPropagationEntry()); + Configuration.Headers.Add("in"); Context.Request.Headers.Add("in", headerValue); // Act @@ -107,40 +107,22 @@ namespace Microsoft.AspNetCore.HeaderPropagation.Tests [Theory] [InlineData(new[] { "default" }, new[] { "default" })] [InlineData(new[] { "default", "other" }, new[] { "default", "other" })] - public async Task NoHeaderInRequest_AddsDefaultValue(string[] defaultValues, - string[] expectedValues) - { - // Arrange - Configuration.Headers.Add("in", new HeaderPropagationEntry { DefaultValue = defaultValues }); - - // Act - await Middleware.Invoke(Context); - - // Assert - Assert.Contains("in", State.Headers.Keys); - Assert.Equal(expectedValues, State.Headers["in"]); - } - - [Theory] - [InlineData(new[] { "default" }, new[] { "default" })] - [InlineData(new[] { "default", "other" }, new[] { "default", "other" })] - public async Task UsesValueFactory(string[] factoryValues, - string[] expectedValues) + public async Task UsesValueFilter(string[] filterValues, string[] expectedValues) { // Arrange string receivedName = null; + StringValues receivedValue = default; HttpContext receivedContext = null; - Configuration.Headers.Add("in", new HeaderPropagationEntry + Configuration.Headers.Add("in", context => { - DefaultValue = "no", - ValueFactory = (name, ctx) => - { - receivedName = name; - receivedContext = ctx; - return factoryValues; - } + receivedValue = context.HeaderValue; + receivedName = context.HeaderName; + receivedContext = context.HttpContext; + return filterValues; }); + Context.Request.Headers.Add("in", "value"); + // Act await Middleware.Invoke(Context); @@ -148,18 +130,15 @@ namespace Microsoft.AspNetCore.HeaderPropagation.Tests Assert.Contains("in", State.Headers.Keys); Assert.Equal(expectedValues, State.Headers["in"]); Assert.Equal("in", receivedName); + Assert.Equal(new StringValues("value"), receivedValue); Assert.Same(Context, receivedContext); } [Fact] - public async Task PreferValueFactory_OverDefaultValuesAndRequestHeader() + public async Task PreferValueFilter_OverRequestHeader() { // Arrange - Configuration.Headers.Add("in", new HeaderPropagationEntry - { - DefaultValue = "no", - ValueFactory = (name, ctx) => "test" - }); + Configuration.Headers.Add("in", context => "test"); Context.Request.Headers.Add("in", "no"); // Act @@ -171,13 +150,10 @@ namespace Microsoft.AspNetCore.HeaderPropagation.Tests } [Fact] - public async Task EmptyValuesFromValueFactory_DoesNotAddIt() + public async Task EmptyValuesFromValueFilter_DoesNotAddIt() { // Arrange - Configuration.Headers.Add("in", new HeaderPropagationEntry - { - ValueFactory = (name, ctx) => StringValues.Empty - }); + Configuration.Headers.Add("in", (context) => StringValues.Empty); // Act await Middleware.Invoke(Context); @@ -187,31 +163,19 @@ namespace Microsoft.AspNetCore.HeaderPropagation.Tests } [Fact] - public async Task NullEntryInConfiguration_HeaderInRequest_AddsCorrectValue() + public async Task MultipleEntries_AddsFirstToProduceValue() { // Arrange - Configuration.Headers.Add("in", null); - Context.Request.Headers.Add("in", "test"); + Configuration.Headers.Add("in"); + Configuration.Headers.Add("in", (context) => StringValues.Empty); + Configuration.Headers.Add("in", (context) => "Test"); // Act await Middleware.Invoke(Context); // Assert Assert.Contains("in", State.Headers.Keys); - Assert.Equal(new[] { "test" }, State.Headers["in"]); - } - - [Fact] - public async Task NullEntryInConfiguration_NoHeaderInRequest_DoesNotAddHeader() - { - // Arrange - Configuration.Headers.Add("in", null); - - // Act - await Middleware.Invoke(Context); - - // Assert - Assert.DoesNotContain("in", State.Headers.Keys); + Assert.Equal("Test", State.Headers["in"]); } } } diff --git a/src/Middleware/Middleware.sln b/src/Middleware/Middleware.sln index 27500743f6..44be9e8d3b 100644 --- a/src/Middleware/Middleware.sln +++ b/src/Middleware/Middleware.sln @@ -277,6 +277,10 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.AspNetCore.Header EndProject Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "test", "test", "{8CDBD9C6-96D8-4987-AFCD-D248FBC7F02D}" EndProject +Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "samples", "samples", "{179A159B-87EA-4353-BE92-4FB6CC05BC7D}" +EndProject +Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "HeaderPropagationSample", "HeaderPropagation\samples\HeaderPropagationSample\HeaderPropagationSample.csproj", "{CDE2E736-A034-4748-98C4-0DEDAAC8063D}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -1511,6 +1515,18 @@ Global {7E18FA09-5E08-4E41-836F-25C94B60C608}.Release|x64.Build.0 = Release|Any CPU {7E18FA09-5E08-4E41-836F-25C94B60C608}.Release|x86.ActiveCfg = Release|Any CPU {7E18FA09-5E08-4E41-836F-25C94B60C608}.Release|x86.Build.0 = Release|Any CPU + {CDE2E736-A034-4748-98C4-0DEDAAC8063D}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {CDE2E736-A034-4748-98C4-0DEDAAC8063D}.Debug|Any CPU.Build.0 = Debug|Any CPU + {CDE2E736-A034-4748-98C4-0DEDAAC8063D}.Debug|x64.ActiveCfg = Debug|Any CPU + {CDE2E736-A034-4748-98C4-0DEDAAC8063D}.Debug|x64.Build.0 = Debug|Any CPU + {CDE2E736-A034-4748-98C4-0DEDAAC8063D}.Debug|x86.ActiveCfg = Debug|Any CPU + {CDE2E736-A034-4748-98C4-0DEDAAC8063D}.Debug|x86.Build.0 = Debug|Any CPU + {CDE2E736-A034-4748-98C4-0DEDAAC8063D}.Release|Any CPU.ActiveCfg = Release|Any CPU + {CDE2E736-A034-4748-98C4-0DEDAAC8063D}.Release|Any CPU.Build.0 = Release|Any CPU + {CDE2E736-A034-4748-98C4-0DEDAAC8063D}.Release|x64.ActiveCfg = Release|Any CPU + {CDE2E736-A034-4748-98C4-0DEDAAC8063D}.Release|x64.Build.0 = Release|Any CPU + {CDE2E736-A034-4748-98C4-0DEDAAC8063D}.Release|x86.ActiveCfg = Release|Any CPU + {CDE2E736-A034-4748-98C4-0DEDAAC8063D}.Release|x86.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE @@ -1630,6 +1646,8 @@ Global {D66BD4A3-DA19-413B-8FC5-4BCCFB03E084} = {0437D207-864E-429C-92B4-9D08D290188C} {7E18FA09-5E08-4E41-836F-25C94B60C608} = {8CDBD9C6-96D8-4987-AFCD-D248FBC7F02D} {8CDBD9C6-96D8-4987-AFCD-D248FBC7F02D} = {0437D207-864E-429C-92B4-9D08D290188C} + {179A159B-87EA-4353-BE92-4FB6CC05BC7D} = {0437D207-864E-429C-92B4-9D08D290188C} + {CDE2E736-A034-4748-98C4-0DEDAAC8063D} = {179A159B-87EA-4353-BE92-4FB6CC05BC7D} EndGlobalSection GlobalSection(ExtensibilityGlobals) = postSolution SolutionGuid = {83786312-A93B-4BB4-AB06-7C6913A59AFA}