diff --git a/src/ReverseProxy/Service/Proxy/HttpProxy.cs b/src/ReverseProxy/Service/Proxy/HttpProxy.cs index e06bd3ab9..36071fce3 100644 --- a/src/ReverseProxy/Service/Proxy/HttpProxy.cs +++ b/src/ReverseProxy/Service/Proxy/HttpProxy.cs @@ -370,8 +370,8 @@ private static void CopyRequestHeaders(HttpContext context, HttpRequestMessage d foreach (var header in context.Request.Headers) { var headerName = header.Key; - var value = header.Value; - if (StringValues.IsNullOrEmpty(value)) + var headerValue = header.Value; + if (StringValues.IsNullOrEmpty(headerValue)) { continue; } @@ -385,14 +385,14 @@ private static void CopyRequestHeaders(HttpContext context, HttpRequestMessage d if (transforms.RequestHeaderTransforms.TryGetValue(headerName, out var transform)) { (transformsRun ??= new HashSet(StringComparer.OrdinalIgnoreCase)).Add(headerName); - value = transform.Apply(context, value); - if (StringValues.IsNullOrEmpty(value)) + headerValue = transform.Apply(context, headerValue); + if (StringValues.IsNullOrEmpty(headerValue)) { continue; } } - AddHeader(destination, headerName, value); + AddHeader(destination, headerName, headerValue); } } @@ -416,6 +416,15 @@ private static void CopyRequestHeaders(HttpContext context, HttpRequestMessage d // as long as they appear in one (and only one, otherwise they would be duplicated). static void AddHeader(HttpRequestMessage request, string headerName, StringValues value) { + // HttpClient wrongly uses comma (",") instead of semi-colon (";") as a separator for Cookie headers. + // To mitigate this, we concatenate them manually and put them back as a single header value. + // A multi-header cookie header is invalid, but we get one because of + // https://github.com/dotnet/aspnetcore/issues/26461 + if (string.Equals(headerName, HeaderNames.Cookie, StringComparison.OrdinalIgnoreCase) && value.Count > 1) + { + value = String.Join("; ", value); + } + if (value.Count == 1) { string headerValue = value; diff --git a/test/ReverseProxy.FunctionalTests/HttpProxyCookieTests.cs b/test/ReverseProxy.FunctionalTests/HttpProxyCookieTests.cs new file mode 100644 index 000000000..e33d67ac8 --- /dev/null +++ b/test/ReverseProxy.FunctionalTests/HttpProxyCookieTests.cs @@ -0,0 +1,213 @@ +using System; +using System.IO; +using System.Net; +using System.Net.Http; +using System.Net.Sockets; +using System.Text; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Server.Kestrel.Core; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Primitives; +using Microsoft.Net.Http.Headers; +using Microsoft.ReverseProxy.Abstractions; +using Xunit; + +namespace Microsoft.ReverseProxy +{ + public abstract class HttpProxyCookieTests + { + public const string CookieAKey = "testA"; + public const string CookieAValue = "A_Cookie"; + public const string CookieBKey = "testB"; + public const string CookieBValue = "B_Cookie"; + + public static readonly string CookieA = $"{CookieAKey}={CookieAValue}"; + public static readonly string CookieB = $"{CookieBKey}={CookieBValue}"; + public static readonly string Cookies = $"{CookieA}; {CookieB}"; + + public abstract HttpProtocols HttpProtocol { get; } + public abstract Task ProcessHttpRequest(Uri proxyHostUri); + + [Fact] + public async Task ProxyAsync_RequestWithCookieHeaders() + { + var tcs = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + + using var destinationHost = CreateDestinationHost( + context => + { + if (context.Request.Headers.TryGetValue(HeaderNames.Cookie, out var cookieHeaders)) + { + tcs.SetResult(cookieHeaders); + } + else + { + tcs.SetException(new Exception("Missing 'Cookie' header in request")); + } + return Task.CompletedTask; + }); + + await destinationHost.StartAsync(); + var destinationHostUrl = destinationHost.GetAddress(); + + using var proxyHost = CreateReverseProxyHost(HttpProtocol, destinationHostUrl); + await proxyHost.StartAsync(); + var proxyHostUri = new Uri(proxyHost.GetAddress()); + + await ProcessHttpRequest(proxyHostUri); + + Assert.True(tcs.Task.IsCompleted); + var cookieHeaders = await tcs.Task; + var cookies = Assert.Single(cookieHeaders); + Assert.Equal(Cookies, cookies); + + await destinationHost.StopAsync(); + await proxyHost.StopAsync(); + } + + private IHost CreateReverseProxyHost(HttpProtocols httpProtocol, string destinationHostUrl) + { + return CreateHost(httpProtocol, + services => + { + var proxyRoute = new ProxyRoute + { + RouteId = "route1", + ClusterId = "cluster1", + Match = { Path = "/{**catchall}" } + }; + + var cluster = new Cluster + { + Id = "cluster1", + Destinations = + { + { "cluster1", new Destination() { Address = destinationHostUrl } } + } + }; + + services.AddReverseProxy().LoadFromMemory(new[] { proxyRoute }, new[] { cluster }); + }, + app => + { + app.UseMiddleware(); + app.UseRouting(); + app.UseEndpoints(builder => + { + builder.MapReverseProxy(); + }); + }); + } + + private IHost CreateDestinationHost(RequestDelegate getDelegate) + { + return CreateHost(HttpProtocols.Http1AndHttp2, + services => + { + services.AddRouting(); + }, + app => + { + app.UseRouting(); + app.UseEndpoints(endpoints => endpoints.MapGet("/", getDelegate)); + }); + } + + private IHost CreateHost(HttpProtocols httpProtocols, Action configureServices, Action configureApp) + { + return new HostBuilder() + .ConfigureWebHost(webHostBuilder => + { + webHostBuilder + .ConfigureServices(configureServices) + .UseKestrel(kestrel => + { + kestrel.Listen(IPAddress.Loopback, 0, listenOptions => + { + listenOptions.Protocols = httpProtocols; + }); + }) + .Configure(configureApp); + }).Build(); + } + + private class CheckCookieHeaderMiddleware + { + private readonly RequestDelegate _next; + + public CheckCookieHeaderMiddleware(RequestDelegate next) + { + _next = next; + } + + public async Task Invoke(HttpContext context) + { + // Ensure that CookieA is the first and CookieB the last. + Assert.True(context.Request.Headers.TryGetValue(HeaderNames.Cookie, out var headerValues)); + + if (context.Request.Protocol == "HTTP/1.1") + { + Assert.Single(headerValues); + Assert.Equal(Cookies, headerValues); + } + else if (context.Request.Protocol == "HTTP/2") + { + Assert.Equal(2, headerValues.Count); + Assert.Equal(CookieA, headerValues[0]); + Assert.Equal(CookieB, headerValues[1]); + } + else + { + Assert.True(false, $"Unexpected HTTP protocol '{context.Request.Protocol}'"); + } + + await _next.Invoke(context); + } + } + } + + public class HttpProxyCookieTests_Http1 : HttpProxyCookieTests + { + public override HttpProtocols HttpProtocol => HttpProtocols.Http1; + + public override async Task ProcessHttpRequest(Uri proxyHostUri) + { + using var client = new HttpClient(); + using var message = new HttpRequestMessage(HttpMethod.Get, proxyHostUri); + message.Headers.Add(HeaderNames.Cookie, Cookies); + using var response = await client.SendAsync(message); + response.EnsureSuccessStatusCode(); + } + } + +#if NET5_0 + public class HttpProxyCookieTests_Http2 : HttpProxyCookieTests + { + public override HttpProtocols HttpProtocol => HttpProtocols.Http2; + + // HttpClient for H/2 will use different header frames for cookies from a container and message headers. + // It will first send message header cookie and than the container one and we expect them in the order of cookieA;cookieB. + public override async Task ProcessHttpRequest(Uri proxyHostUri) + { + using var handler = new HttpClientHandler(); + handler.CookieContainer.Add(new System.Net.Cookie(CookieBKey, CookieBValue, path: "/", domain: proxyHostUri.Host)); + using var client = new HttpClient(handler); + using var message = new HttpRequestMessage(HttpMethod.Get, proxyHostUri); + message.Version = HttpVersion.Version20; + message.VersionPolicy = HttpVersionPolicy.RequestVersionExact; + message.Headers.Add(HeaderNames.Cookie, CookieA); + using var response = await client.SendAsync(message); + response.EnsureSuccessStatusCode(); + } + } +#elif NETCOREAPP3_1 +// Do not test HTTP/2 on .NET Core 3.1 +#else +#error A target framework was added to the project and needs to be added to this condition. +#endif +} \ No newline at end of file diff --git a/test/ReverseProxy.Tests/Service/Proxy/HttpProxyTests.cs b/test/ReverseProxy.Tests/Service/Proxy/HttpProxyTests.cs index 9cf0b584b..01ff9303c 100644 --- a/test/ReverseProxy.Tests/Service/Proxy/HttpProxyTests.cs +++ b/test/ReverseProxy.Tests/Service/Proxy/HttpProxyTests.cs @@ -585,6 +585,41 @@ public async Task ProxyAsync_RequetsWithBodies_HasHttpContent(string method, str Assert.Equal(StatusCodes.Status200OK, httpContext.Response.StatusCode); } + [Fact] + public async Task ProxyAsync_RequestWithCookieHeaders() + { + // This is an invalid format per spec but may happen due to https://github.com/dotnet/aspnetcore/issues/26461 + var cookies = new [] { "testA=A_Cookie", "testB=B_Cookie", "testC=C_Cookie" }; + var httpContext = new DefaultHttpContext(); + httpContext.Request.Method = "GET"; + httpContext.Request.Headers.Add(HeaderNames.Cookie, cookies); + + var destinationPrefix = "https://localhost/"; + var sut = CreateProxy(); + var client = MockHttpHandler.CreateClient( + (HttpRequestMessage request, CancellationToken cancellationToken) => + { + // "testA=A_Cookie; testB=B_Cookie; testC=C_Cookie" + var expectedCookieString = String.Join("; ", cookies); + + Assert.Equal(new Version(2, 0), request.Version); + Assert.Equal("GET", request.Method.Method, StringComparer.OrdinalIgnoreCase); + Assert.Null(request.Content); + Assert.True(request.Headers.TryGetValues(HeaderNames.Cookie, out var cookieHeaders)); + Assert.NotNull(cookieHeaders); + var cookie = Assert.Single(cookieHeaders); + Assert.Equal(expectedCookieString, cookie); + + var response = new HttpResponseMessage(HttpStatusCode.OK) { Content = new ByteArrayContent(Array.Empty()) }; + return Task.FromResult(response); + }); + + await sut.ProxyAsync(httpContext, destinationPrefix, client, new RequestProxyOptions()); + + Assert.Null(httpContext.Features.Get()); + Assert.Equal(StatusCodes.Status200OK, httpContext.Response.StatusCode); + } + [Fact] public async Task ProxyAsync_UnableToConnect_Returns502() {