Double percent encoding of forward slashes (%2F) in paths

[DanielFischer79]

Describe the bug

we use yarp in front of an nexus server which hosting node packages.
Node packages urls can contains %2f instead of slash as part of the path.
yarp encode the percent again to %25 so the resulting path was wrongly modifed.

To Reproduce

GET https://nexus.domain.com/repository/npm-proxy/@types%2fcors

YARP convert the url to, but its wrong:
https://nexus.domain.com/repository/npm-proxy/@types%252fcors

YARP should not modify the path by default. If there is a special use case for duing that, it should be done by a feature flag

Further technical details

We tested it with new version 1.0.0 / .net 5.0

We analysed the issue in detail and found the reason for this issue: #
By fixing the issue #1219 a major change was done with path encoding.
The char '%' was not longer a valid path character for yarp. (Class RequestUtilities, Method: IsValidPathChar)

We temporary fix the issue by creating a custom request transformer and set the uri like RequestUtilities did it before.

[Tratcher]

There's a fundamental issue in aspnetcore with %2F and %252F that makes it impossible for YARP to correctly round trip both values (dotnet/aspnetcore#30655). #1219 defaulted to the safer of the two misunderstandings since double decoding %252F could be dangerous. I don't think we'll be able to sort this out properly until .NET 7.

[karelz]

Triage: We depend on ASP.NET issue dotnet/aspnetcore#30655 to be fixed first. Then we may have to react. Moving to Backlog for now.

[mcrio]

Not sure if I may be doing something differently than @DanielFischer79 but I am observing the following:

• Original path: /path/value%2F123
• Yarp transforms path to: /path/value/123

In my case Yarp seems to be url-decoding the path.

Yarp 1.0.0 on Kestrel

EDIT: actually it seems like Daniel said it does encode it to /path/value%252F123. Then I assume the destination application which is an ASP core (Kestrel) application decodes that internally to /path/value/123 :/

[mcrio]

Conclusion for my case. It is the combination of YARP > DAPR > ASP CORE that converts /path/value%2F123 to /path/value/123

Steps:

• Original path: /path/value%2F123
• Yarp requests path as DAPR method invoke (PathPrefix transform): /v1.0/invoke/myservice/method/path/value%252F123
• Dapr seems to be doing some path magic...
• ASP core incoming path: /path/value/123

Dapr seems to be doing some magic as well because when I make a request to ASP Core with path /path/value%252F123 it resolves it to /path/value%2F123... while when making the request through Dapr it resolves to /path/value/123

[karelz]

Triage: Depends solely on ASP.NET changes, no YARP work -- see #1419 (comment), will be (hopefully) part of 7.0.

[Schoch85]

It tried to use YARP as a ReverseProxy for GitLab, but it didn't worked because of the %252F problem.
I tried everything to circumvent the problem, but with no success.
In the end I modified the YARP function "IsValidPathChar" in the class "RequestUtilities" as described below.
I think this problem should be addressed as soon as possible, because it prevents the usage of YARP for different important applications.

[MethodImpl(MethodImplOptions.AggressiveInlining)]
    internal static bool IsValidPathChar(char c)
    {
        // Add this if condition to avoid encoding %
        if (c == 37)
            return true;

        // Use local array and uint .Length compare to elide the bounds check on array access
        var validChars = ValidPathChars;
        var i = (int)c;

        // Array is in chunks of 32 bits, so get offset by dividing by 32
        var offset = i >> 5; // i / 32;
        // Significant bit position is the remainder of the above calc; i % 32 => i & 31
        var significantBit = 1u << (i & 31);

        // Check offset in bounds and check if significant bit set
        return (uint)offset < (uint)validChars.Length &&
            ((validChars[offset] & significantBit) != 0);
    }

[giotab]

It tried to use YARP as a ReverseProxy for GitLab, but it didn't worked because of the %252F problem. I tried everything to circumvent the problem, but with no success. In the end I modified the YARP function "IsValidPathChar" in the class "RequestUtilities" as described below. I think this problem should be addressed as soon as possible, because it prevents the usage of YARP for different important applications.

[MethodImpl(MethodImplOptions.AggressiveInlining)]
    internal static bool IsValidPathChar(char c)
    {
        // Add this if condition to avoid encoding %
        if (c == 37)
            return true;

        // Use local array and uint .Length compare to elide the bounds check on array access
        var validChars = ValidPathChars;
        var i = (int)c;

        // Array is in chunks of 32 bits, so get offset by dividing by 32
        var offset = i >> 5; // i / 32;
        // Significant bit position is the remainder of the above calc; i % 32 => i & 31
        var significantBit = 1u << (i & 31);

        // Check offset in bounds and check if significant bit set
        return (uint)offset < (uint)validChars.Length &&
            ((validChars[offset] & significantBit) != 0);
    }

Can you elaborate where did you insert this code, and how did you use it?
Did you encounter any other issues in GitLab after introducing this "fix"?

[Schoch85]

I changed my approach to not affect other routes.

So what I did is I changed the method "MakeDestinationAddress" in the class "RequestUtilities" as follows:

public static Uri MakeDestinationAddress(string destinationPrefix, PathString path, QueryString query)
{
    ReadOnlySpan<char> prefixSpan = destinationPrefix;

    if (path.HasValue && destinationPrefix.EndsWith('/'))
    {
        // When PathString has a value it always starts with a '/'. Avoid double slashes when concatenating.
        prefixSpan = prefixSpan[0..^1];
    }

    // Do not encode Path's for GitLab
    string targetAddress;
    if (String.Compare(destinationPrefix, "https://gitlab.example.ch", StringComparison.OrdinalIgnoreCase) != 0)
    {
        targetAddress = string.Concat(prefixSpan, EncodePath(path), query.ToUriComponent());
    }
    else
    {
        targetAddress = string.Concat(prefixSpan, path.Value!, query.ToUriComponent());
    }

    return new Uri(targetAddress, UriKind.Absolute);
}

So the destinationPrefix is hardcoded. It is not a nice solution, but it works for GitLab, no problems anymore occurred.

But in my opinion it should be possible for every route to set a flag if we want to encode the path or not.

[Schoch85]

I forgot to mention, when I started to use the source code of YARP instead the Nuget package I got the following error message sometimes, independent from GitLab:

"The request cannot be forwarded, the response has already started"

So what I did it commented out the following lines in the "HttpForwarder" class:

    //if (RequestUtilities.IsResponseSet(context.Response))
    //{
    //    throw new InvalidOperationException("The request cannot be forwarded, the response has already started");
    //}

[jwfx]

I just ran into this issue which renders YARP useless for me.

Is there any workaround available that doesn't require to modify and maintain a custom version of YARP?

Also, it does not seem like there will be a fix on aspnet side any time soon (the linked issue is rotting away in the backlog), so is there anything that can be done on YARP side?

[DanielFischer79]

I just ran into this issue which renders YARP useless for me.

Is there any workaround available that doesn't require to modify and maintain a custom version of YARP?

Also, it does not seem like there will be a fix on aspnet side any time soon (the linked issue is rotting away in the backlog), so is there anything that can be done on YARP side?

Yes, there is a workaround. We use this code since few years:

// This transformer disables the default components for transforming the URL.
// Since version V1.0.0, RequestUtilities.MakeDestinationAddress encodes the path and no longer recognizes % as a valid path.
// For this reason, it gets encoded, which causes issues with NPM packages.
// Issue Sample: http://localhost:5000/repository/npm-proxy/@types%2fcors encodiert zu %252fcors
// Microsoft Ticket eröffnet: https://github.com/microsoft/reverse-proxy/issues/1419
public class SlashFixTransformer : RequestTransform
{
    public override ValueTask ApplyAsync(RequestTransformContext context)
    {
        context.ProxyRequest.RequestUri = MakeDestinationAddress(context.DestinationPrefix, context.Path, context.Query.QueryString);
        return ValueTask.CompletedTask;
    }
        
    public static Uri MakeDestinationAddress(string destinationPrefix, PathString path, QueryString query)
    {
        ReadOnlySpan<char> prefixSpan = destinationPrefix;

        if (path.HasValue && destinationPrefix.EndsWith('/'))
        {
            // When PathString has a value it always starts with a '/'. Avoid double slashes when concatenating.
            prefixSpan = prefixSpan[0..^1];
        }

        var targetAddress = string.Concat(prefixSpan, path.ToUriComponent(), query.ToUriComponent());

        return new Uri(targetAddress, UriKind.Absolute);
    }
}

public class PercentEncodingTransformProvider : ITransformProvider
{
    public void ValidateRoute(TransformRouteValidationContext context)
    {
    }

    public void ValidateCluster(TransformClusterValidationContext context)
    {
    }

    public void Apply(TransformBuilderContext context)
    {
        context.RequestTransforms.Add(new SlashFixTransformer());
    }
}

At least, add this line to program.cs:
builder.Services.AddReverseProxy().AddTransforms<PercentEncodingTransformProvider>();

[jwfx]

Thank you for that workaround, upon quick testing it looks like it does solve the issue for me. 👍

Right now I cannot pretend to fully understand what it does, there doesn't seem to be much going on there code-wise.
Are there any known side effects or is there any chance this transform could negatively affect other YARP functionality?

[mcrio]

As this problem hasn't been fixed for years, I believe it is something the MS team cannot really solve without introducing lot's of breaking changes. It is still a fundamental issue.

The other day I was deploying an app to Azure App Services, which seems to be using Yarp behind the scene as it automatically decodes %2f in the URL. In the end it was useless as I couldn't run the app as expected, and double encoding / is not a reliable solution. 😐

[DanielFischer79]

Thank you for that workaround, upon quick testing it looks like it does solve the issue for me. 👍

Right now I cannot pretend to fully understand what it does, there doesn't seem to be much going on there code-wise. Are there any known side effects or is there any chance this transform could negatively affect other YARP functionality?

We have the code since 3 years in a productive environment. Behind our YARP farm we have many different kinds of applications (common web apps, apis, grafana, loki, prometheus, gitlab, nexus with nuget, npm and docker repository). Everything is running smoothly.

[KrzysztofBranicki]

Similar issue here with proxying requests to HashiCorp Vault: hashicorp/vault-client-dotnet#204