Skip to content

Commit

Permalink
Handle multivalue X-Forwarded-Proto header
Browse files Browse the repository at this point in the history
Closes #5198
  • Loading branch information
ahmelsayed committed Jan 7, 2020
1 parent 52a204e commit 28ba9d9
Show file tree
Hide file tree
Showing 6 changed files with 57 additions and 11 deletions.
Original file line number Diff line number Diff line change
@@ -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)
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public static IApplicationBuilder UseWebJobsScriptHost(this IApplicationBuilder
// JobHost/ScriptHost scoped services.
builder.UseMiddleware<ScriptHostRequestServiceProviderMiddleware>();

if (!environment.IsAppService())
if (environment.IsLinuxConsumption())
{
builder.UseMiddleware<AppServiceHeaderFixupMiddleware>();
}
Expand Down
10 changes: 8 additions & 2 deletions test/WebJobs.Script.Tests.Integration/TestFunctionHost.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -185,15 +187,15 @@ public static void WriteNugetPackageSources(string appRoot, params string[] sour
}

/// <summary>
/// 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.
/// </summary>
/// <returns>The messages from the ScriptHost LoggerProvider</returns>
public IList<LogMessage> GetScriptHostLogMessages() => _scriptHostLoggerProvider.GetAllLogMessages();
public IEnumerable<LogMessage> GetScriptHostLogMessages(string category) => GetScriptHostLogMessages().Where(p => p.Category == category);

/// <summary>
/// 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.
/// </summary>
/// <returns>The messages from the WebHost LoggerProvider</returns>
Expand Down Expand Up @@ -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<AppServiceHeaderFixupMiddleware>();

_startup.Configure(app, applicationLifetime, env, loggerFactory);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -137,14 +137,15 @@ 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}")),
Method = HttpMethod.Post
};

request.Headers.TryAddWithoutValidation("DISGUISED-HOST", actualHost);
request.Headers.TryAddWithoutValidation("X-Forwarded-Proto", actualProtocol);
request.Headers.Add("X-Forwarded-Proto", protocolHeaders);

var input = new JObject
{
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
}
}
}
1 change: 1 addition & 0 deletions test/WebJobs.Script.Tests/WebJobs.Script.Tests.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@

<ItemGroup>
<PackageReference Include="appinsights.testlogger" Version="1.0.0" />
<PackageReference Include="FluentAssertions" Version="5.9.0" />
<PackageReference Include="Microsoft.AspNetCore.TestHost" Version="2.2.0" />
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="15.8.0" />
<PackageReference Include="Moq" Version="4.9.0" />
Expand Down

0 comments on commit 28ba9d9

Please sign in to comment.