From 28ba9d9929129c5179a1308030a4630aee40a4bd Mon Sep 17 00:00:00 2001 From: Ahmed ElSayed Date: Wed, 13 Nov 2019 13:35:20 -0800 Subject: [PATCH] Handle multivalue X-Forwarded-Proto header Closes #5198 --- .../AppServiceHeaderFixupMiddleware.cs | 14 +++++--- .../WebJobsApplicationBuilderExtension.cs | 2 +- .../TestFunctionHost.cs | 10 ++++-- .../WebHostEndToEnd/CSharpEndToEndTests.cs | 7 ++-- .../AppServiceHeaderFixupMiddlewareTests.cs | 34 +++++++++++++++++++ .../WebJobs.Script.Tests.csproj | 1 + 6 files changed, 57 insertions(+), 11 deletions(-) create mode 100644 test/WebJobs.Script.Tests/Middleware/AppServiceHeaderFixupMiddlewareTests.cs diff --git a/src/WebJobs.Script.WebHost/Middleware/AppServiceHeaderFixupMiddleware.cs b/src/WebJobs.Script.WebHost/Middleware/AppServiceHeaderFixupMiddleware.cs index 5b25196783..1a97f9aab9 100644 --- a/src/WebJobs.Script.WebHost/Middleware/AppServiceHeaderFixupMiddleware.cs +++ b/src/WebJobs.Script.WebHost/Middleware/AppServiceHeaderFixupMiddleware.cs @@ -1,18 +1,18 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the MIT License. See License.txt in the project root for license information. +using System.Linq; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; -using Microsoft.Azure.WebJobs.Script.Extensions; using Microsoft.Extensions.Primitives; namespace Microsoft.Azure.WebJobs.Script.WebHost.Middleware { public class AppServiceHeaderFixupMiddleware { - private const string DisguisedHostHeader = "DISGUISED-HOST"; - private const string HostHeader = "HOST"; - private const string ForwardedProtocolHeader = "X-Forwarded-Proto"; + internal const string DisguisedHostHeader = "DISGUISED-HOST"; + internal const string HostHeader = "HOST"; + internal const string ForwardedProtocolHeader = "X-Forwarded-Proto"; private readonly RequestDelegate _next; public AppServiceHeaderFixupMiddleware(RequestDelegate next) @@ -29,7 +29,11 @@ public async Task Invoke(HttpContext httpContext) if (httpContext.Request.Headers.TryGetValue(ForwardedProtocolHeader, out value)) { - httpContext.Request.Scheme = value; + string scheme = value.FirstOrDefault(); + if (!string.IsNullOrEmpty(scheme)) + { + httpContext.Request.Scheme = scheme; + } } await _next(httpContext); diff --git a/src/WebJobs.Script.WebHost/WebJobsApplicationBuilderExtension.cs b/src/WebJobs.Script.WebHost/WebJobsApplicationBuilderExtension.cs index e6471d1fa4..50c85d6218 100644 --- a/src/WebJobs.Script.WebHost/WebJobsApplicationBuilderExtension.cs +++ b/src/WebJobs.Script.WebHost/WebJobsApplicationBuilderExtension.cs @@ -46,7 +46,7 @@ public static IApplicationBuilder UseWebJobsScriptHost(this IApplicationBuilder // JobHost/ScriptHost scoped services. builder.UseMiddleware(); - if (!environment.IsAppService()) + if (environment.IsLinuxConsumption()) { builder.UseMiddleware(); } diff --git a/test/WebJobs.Script.Tests.Integration/TestFunctionHost.cs b/test/WebJobs.Script.Tests.Integration/TestFunctionHost.cs index d377a2af41..31eea02a92 100644 --- a/test/WebJobs.Script.Tests.Integration/TestFunctionHost.cs +++ b/test/WebJobs.Script.Tests.Integration/TestFunctionHost.cs @@ -28,6 +28,8 @@ using Microsoft.Extensions.Options; using Microsoft.WebJobs.Script.Tests; using Newtonsoft.Json.Linq; +using Microsoft.AspNetCore.Builder; +using Microsoft.Azure.WebJobs.Script.WebHost.Middleware; namespace Microsoft.Azure.WebJobs.Script.Tests { @@ -185,7 +187,7 @@ public static void WriteNugetPackageSources(string appRoot, params string[] sour } /// - /// The functions host has two logger providers -- one at the WebHost level and one at the ScriptHost level. + /// The functions host has two logger providers -- one at the WebHost level and one at the ScriptHost level. /// These providers use different LoggerProviders, so it's important to know which one is receiving the logs. /// /// The messages from the ScriptHost LoggerProvider @@ -193,7 +195,7 @@ public static void WriteNugetPackageSources(string appRoot, params string[] sour public IEnumerable GetScriptHostLogMessages(string category) => GetScriptHostLogMessages().Where(p => p.Category == category); /// - /// The functions host has two logger providers -- one at the WebHost level and one at the ScriptHost level. + /// The functions host has two logger providers -- one at the WebHost level and one at the ScriptHost level. /// These providers use different LoggerProviders, so it's important to know which one is receiving the logs. /// /// The messages from the WebHost LoggerProvider @@ -292,6 +294,10 @@ public void ConfigureServices(IServiceCollection services) public void Configure(AspNetCore.Builder.IApplicationBuilder app, AspNetCore.Hosting.IApplicationLifetime applicationLifetime, AspNetCore.Hosting.IHostingEnvironment env, Microsoft.Extensions.Logging.ILoggerFactory loggerFactory) { + // This middleware is only added when env.IsLinuxConsumption() + // It should be a no-op for most tests + app.UseMiddleware(); + _startup.Configure(app, applicationLifetime, env, loggerFactory); } } diff --git a/test/WebJobs.Script.Tests.Integration/WebHostEndToEnd/CSharpEndToEndTests.cs b/test/WebJobs.Script.Tests.Integration/WebHostEndToEnd/CSharpEndToEndTests.cs index 6aa0e132cc..f08a5fa5f5 100644 --- a/test/WebJobs.Script.Tests.Integration/WebHostEndToEnd/CSharpEndToEndTests.cs +++ b/test/WebJobs.Script.Tests.Integration/WebHostEndToEnd/CSharpEndToEndTests.cs @@ -110,9 +110,9 @@ await TestHelpers.Await(() => logs.Single(p => p.EndsWith($"From TraceWriter: {guid1}")); logs.Single(p => p.EndsWith($"From ILogger: {guid2}")); - + // Make sure we get a metric logged from both ILogger and TraceWriter - var key = MetricsEventManager.GetAggregateKey(MetricEventNames.FunctionUserLog, "Scenarios"); + var key = MetricsEventManager.GetAggregateKey(MetricEventNames.FunctionUserLog, "Scenarios"); Assert.Equal(2, Fixture.MetricsLogger.LoggedEvents.Where(p => p == key).Count()); // Make sure we've gotten a log from the aggregator @@ -137,6 +137,7 @@ public async Task VerifyHostHeader() const string actualHost = "actual-host"; const string actualProtocol = "https"; const string path = "api/httptrigger-scenarios"; + var protocolHeaders = new[] { "https", "http" }; var request = new HttpRequestMessage { RequestUri = new Uri(string.Format($"http://localhost/{path}")), @@ -144,7 +145,7 @@ public async Task VerifyHostHeader() }; request.Headers.TryAddWithoutValidation("DISGUISED-HOST", actualHost); - request.Headers.TryAddWithoutValidation("X-Forwarded-Proto", actualProtocol); + request.Headers.Add("X-Forwarded-Proto", protocolHeaders); var input = new JObject { diff --git a/test/WebJobs.Script.Tests/Middleware/AppServiceHeaderFixupMiddlewareTests.cs b/test/WebJobs.Script.Tests/Middleware/AppServiceHeaderFixupMiddlewareTests.cs new file mode 100644 index 0000000000..f8feff6581 --- /dev/null +++ b/test/WebJobs.Script.Tests/Middleware/AppServiceHeaderFixupMiddlewareTests.cs @@ -0,0 +1,34 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the MIT License. See License.txt in the project root for license information. + +using System.Threading.Tasks; +using FluentAssertions; +using Microsoft.AspNetCore.Http; +using Microsoft.Azure.WebJobs.Script.WebHost.Middleware; +using Microsoft.Extensions.Primitives; +using Xunit; + +namespace Microsoft.Azure.WebJobs.Script.Tests.Middleware +{ + public class AppServiceHeaderFixupMiddlewareTests + { + [Theory] + [InlineData(new[] { "http" }, "http")] + [InlineData(new[] { "https" }, "https")] + [InlineData(new[] { "https", "http" }, "https")] + [InlineData(new[] { "http", "https" }, "http")] + public async Task AppServiceFixupMiddleware_Handles_Multivalue_Header(string[] headerValues, string expectedScheme) + { + var ctx = new DefaultHttpContext(); + ctx.Request.Headers.Add(AppServiceHeaderFixupMiddleware.ForwardedProtocolHeader, new StringValues(headerValues)); + + var middleware = new AppServiceHeaderFixupMiddleware(nextCtx => + { + nextCtx.Request.Scheme.Should().Be(expectedScheme); + return Task.CompletedTask; + }); + + await middleware.Invoke(ctx); + } + } +} \ No newline at end of file diff --git a/test/WebJobs.Script.Tests/WebJobs.Script.Tests.csproj b/test/WebJobs.Script.Tests/WebJobs.Script.Tests.csproj index f7431bbeac..ae6baba6ec 100644 --- a/test/WebJobs.Script.Tests/WebJobs.Script.Tests.csproj +++ b/test/WebJobs.Script.Tests/WebJobs.Script.Tests.csproj @@ -49,6 +49,7 @@ +