dotnet / dotnet/aspnetcore

Multiple X-Forwarded-Prefix header segments are not handled correctly in ForwarededHeadersMiddleware

Open
#60,587 2 comments 0 reactions 0 assignees View on GitHub
area-middleware Needs: Attention :wave:
Dominant language
C#
Stars
38.4k
Forks
10.9k
Avg merge
2d 5h
Merged PRs (30d)
276

Description

### Is there an existing issue for this?

- [x] I have searched the existing issues

### Describe the bug

When multiple values are sent for the X-Forwarded-Prefix header, e.g. `/firstsegment, /secondsegment`, contrary to the documentation [here](https://learn.microsoft.com/en-us/aspnet/core/host-and-deploy/proxy-load-balancer?view=aspnetcore-9.0#forwarded-headers) only the first value is consumed and the other values are not moved. The resulting PathBase is `/firstsegment` instead of `/firstsegment/secondsegment`. The documentation states that all values should be moved one after the other.

From a quick look into the ForwardedHeaderMiddleware it appears to me that in the loop in L220ff. `currentValues` is repeatedly replaced for all entries of `sets` instead of actually accumulating all the entries, despite the index being called `consumedValues`. `sets` contains two values in reverse order of the segments (first /secondsegment, then /firstsegment)

```C#
for (; entriesConsumed < sets.Length; entriesConsumed++)
{
var set = sets[entriesConsumed];
...
if (checkPrefix)
{
if (!string.IsNullOrEmpty(set.Prefix) && set.Prefix[0] == '/')
{
applyChanges = true;
currentValues.Prefix = set.Prefix;
```

Thus, in L383 the incomplete PathBase is set because `currentValues.Prefix` only contains `/firstsegment`.

```C#
request.PathBase = PathString.FromUriComponent(currentValues.Prefix);
```

The check in L371 is also false (both `forwarded.Prefix!.Length` and `currentValues` are 2) although the header is only partially handled. Therefore, the header is not correctly trucated either.

```C#
if (forwardedPrefix!.Length > entriesConsumed)
{
// Truncate the consumed header values
requestHeaders[_options.ForwardedPrefixHeaderName] =
TruncateConsumedHeaderValues(forwardedPrefix, entriesConsumed);
```

I did not check the logic for other headers in detail but since the handling is very similar, also similar issues might apply there.

I might be missing something in the intent of these headers, so please feel free to discuss if I misinterpret them. My use case is resolving headers behind multiple reverse proxies that are each responsible for one segment correctly.

### Expected Behavior

The path base contains all segments of the header, e.g. X-Forwarded-Prefix header `/firstsegment, /secondsegment` results in request.PathBase `/firstsegment/secondsegment`.

### Steps To Reproduce

```C#
using Microsoft.AspNetCore.HttpOverrides;

var builder = WebApplication.CreateBuilder(args);

var app = builder.Build();

app.UseForwardedHeaders(new ForwardedHeadersOptions
{
ForwardLimit = null,
ForwardedHeaders = ForwardedHeaders.All
});

app.Run(ctx =>
{
Console.WriteLine(ctx.Request.PathBase);
return Task.CompletedTask;
});

app.Run();
```

When I CURL the server on path `/` with X-Forwarded-Prefix `/firstsegment, /secondsegment` ctx.Request.PathBase prints `/firstsegment`

### Exceptions (if any)

_No response_

### .NET Version

9.0.100

### Anything else?

_No response_

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.