PathPattern encoding issue on .NET 6.0 RC2

[bogdandynamic]

Hello. I've encountered a problem after upgrading a aspnet core project from net5.0 (where all worked fine) to net6.0-rc2. All proxied paths have %2F instead of /.

Here is my appsettings.json ReverseProxy config:

  "ReverseProxy": {
    "Clusters": {
      "cluster1": {
        "Destinations": {
          "cluster1/rest": {
            "Address": "http://container1:49602/"
          }
        }
      },
      "cluster1Grpc": {
        "Destinations": {
          "cluster1/grpc": {
            "Address": "http://container1:49603/"
          }
        }
      },
      "cluster2": {
        "Destinations": {
          "cluster2/rest": {
            "Address": "http://container2:49604/"
          }
        }
      },
      "cluster3": {
        "Destinations": {
          "cluster3/rest": {
            "Address": "http://container3:49606/"
          }
        }
      },
      "cluster4": {
        "Destinations": {
          "cluster4/rest": {
            "Address": "http://container4:49608/"
          }
        }
      }
    }
    "Routes": {
      "CommonRestRoute": {
        "ClusterId": "cluster1",
        "CorsPolicy": "CorsPolicy",
        "Match": {
          "Path": "/service1/rest/{*remainder}"
        },
        "Transforms": [
          { "PathPattern": "/{remainder}" },
          { "RequestHeadersCopy": "true" }
        ]
      },
      "CommonSignalRRoute": {
        "ClusterId": "cluster1",
        "CorsPolicy": "CorsPolicy",
        "Match": {
          "Path": "/server-notifications/common/{*remainder}"
        },
        "Transforms": [
          { "PathPattern": "/server-notifications/{remainder}" },
          { "RequestHeadersCopy": "true" }
        ]
      },
      "ProjectsRestRoute": {
        "ClusterId": "cluster2",
        "Match": {
          "Path": "/service2/rest/{*remainder}"
        },
        "Transforms": [
          { "PathPattern": "/{remainder}" },
          { "RequestHeadersCopy": "true" }
        ]
      },
      "StorageRestRoute": {
        "ClusterId": "cluster3",
        "Match": {
          "Path": "/storage/rest/{*remainder}"
        },
        "Transforms": [
          { "PathPattern": "/{remainder}" },
          { "RequestHeadersCopy": "true" }
        ]
      },
      "StaticStorageRoute": {
        "ClusterId": "cluster3",
        "CorsPolicy": "CorsPolicy",
        "Match": {
          "Path": "/static-storage/{*remainder}"
        },
        "Transforms": [
          { "PathPattern": "/static-storage/{remainder}" },
          { "RequestHeadersCopy": "true" }
        ]
      },
      "ReceptionsRestRoute": {
        "ClusterId": "cluster4",
        "Match": {
          "Path": "/service4/rest/{*remainder}"
        },
        "Transforms": [
          { "PathPattern": "/{remainder}" },
          { "RequestHeadersCopy": "true" }
        ]
      }

In the container logs I receive the following output:

[09:51:41 INF] Proxying to http://container2:49604/v1.0%2Fprojects%2F?simpleDto=true
[09:51:41 INF] Proxying to http://container2:49604/v1.0%2Fprojects%2F?simpleDto=true
[09:51:41 INF] Proxying to http://container2:49604/v1.0%2Fdashboard%2Faq4Zv475

The whole path component, v1.0%2Fprojects%2F, is URL encoded on every transform path defined in appsettings.json.

I downloaded the source code of YARP and traced the problem to PathRouteValuesTransform.cs:

public override ValueTask ApplyAsync(RequestTransformContext context)
{
    if (context is null)
    {
        throw new ArgumentNullException(nameof(context));
    }

    // TemplateBinder.BindValues will modify the RouteValueDictionary
    // We make a copy so that the original request is not modified by the transform
    var routeValues = new RouteValueDictionary(context.HttpContext.Request.RouteValues);

    // Route values that are not considered defaults will be appended as query parameters. Make them all defaults.
    var binder = _binderFactory.Create(Template, defaults: routeValues);
    context.Path = binder.BindValues(acceptedValues: routeValues); // THIS IS WHERE context.Path gets values with %2F
    return default;
}

Development stack and packages:
All 4 services run under docker (container 1 through 4). Docker files use
FROM mcr.microsoft.com/dotnet/aspnet:6.0 AS base and FROM mcr.microsoft.com/dotnet/sdk:6.0 AS build.
Yarp.ReverseProxy 1.0.0-preview.12.21451.3
All other Microsoft nugets are upgraded to NET 6.0 RC2 or latest available stable version.

[Tratcher]

I think this was fixed by #1278, I can't repro the issue on main. Please try out our latest builds from https://github.com/microsoft/reverse-proxy/blob/main/docs/DailyBuilds.md.

I can reproduce the issue on preview12 with 6.0 rc2.

FYI { "RequestHeadersCopy": "true" } is the default so you don't need to set it.

[bogdandynamic]

Thanks @Tratcher for the info. I'll try the daily builds until another release comes out with the fix on the problem.

[Tratcher]

Nevermind, my repro was set up wrong. I can reproduce this with the latest main on 6.0-rc.2. Investigating...

[Tratcher]

RouteValues: service1/rest/abc/def
BindValues: /service1%2Frest%2Fabc%2Fdef

PathString.Value:
5.0: /service1/rest/abc/def
6.0: /service1%2Frest%2Fabc%2Fdef

Request:
5.0: https://example.com/service1/rest/abc/def
6.0: https://example.com/service1%252Frest%252Fabc%252Fdef

I think the relevant change was in 6.0-preview.7:
dotnet/aspnetcore@87b761d#diff-a757f64cc43bf55f008c0f848d205bb647e4c9444e13028891420e10e3b38c7e

BindValues shouldn't be encoding the slashes, but it didn't matter before because PathString.FromUriComponent would decode them. Fix one bug and find another...

Here's a different solution for your use case that's not affected by this:

      "route1": {
        "ClusterId": "cluster1",
        "Match": {
          "Path": "/abc/{*remainder}"
        },
        "Transforms": [
          /* { "PathPattern": "/{remainder}" } */
          { "PathRemovePrefix": "/abc" }
        ]
      }

[bogdandynamic]

It works ok with PathRemovePrefix set on every route in this scenario.
Glad you tracked the source of the bug.

[karelz]

@Tratcher do we want to fix something in YARP, or ASP.NET Core?

[Tratcher]

Both I suspect. I'll need to dig into how the model binder works, but a bug there is not going to be fixed for 6.0 and we'll have to mitigate it in YARP.