Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

.NET 6 Yarp is directing to path with %2F rather than / #1777

Open
leonidzh opened this issue Jun 26, 2022 · 15 comments
Open

.NET 6 Yarp is directing to path with %2F rather than / #1777

leonidzh opened this issue Jun 26, 2022 · 15 comments
Labels
help wanted We will welcome a contribution Type: Documentation Improvements or additions to documentation
Milestone

Comments

@leonidzh
Copy link

leonidzh commented Jun 26, 2022

Describe the bug

Text below copied from bug #1617, because it is exactly the same issue but not solved as wrote in #1617.

Since upgrading to .NET 6, I get 404s for a proxied request. When debugging my reverse proxy api, I am seeing that the {*path} in my path is getting it's /'s converted to %2F. I didn't change Yarp versions, only updated my target framework and other MS packages to 6.0 versions.

To Reproduce

  1. Configure route as following:
"Routes": {
    "Test": {
        "Match": {
            "Path": "/api/gwtest/{*path}"
        },
        "Order": 200,
        "ClusterId": "clusterId",
        "AuthorizationPolicy": "ValidateToken",
        "Transforms": [
            {
                "PathPattern": "/api/{path}"
            }
        ]
    }
}
  1. Request a resource such as /api/gwtest/p1/p2

This works perfectly in .NET 5, but in .NET 6 path portion has the slash transformed, which ultimately gives a 404.

Further technical details

  • Yarp.ReverseProxy 1.1.0
  • The platform (Linux and Windows)
@leonidzh leonidzh added the Type: Bug Something isn't working label Jun 26, 2022
@Tratcher
Copy link
Member

That should be {**path} to match more than one segment.

@leonidzh
Copy link
Author

Thanks! I saw that in examples, but did not find in docs, so was not sure what is that about. Will try.

@karelz
Copy link
Member

karelz commented Jun 30, 2022

Triage: We should doc the difference between {*path} and {**path} as it is easy trap to fall into.

@karelz karelz added Type: Documentation Improvements or additions to documentation and removed Type: Bug Something isn't working labels Jun 30, 2022
@karelz karelz added this to the YARP 2.0.0 milestone Jun 30, 2022
@karelz karelz added the help wanted We will welcome a contribution label Jun 30, 2022
@leonidzh
Copy link
Author

{**path} did not help. I added the following configuration:

    "Test": {
        "Match": {
          "Path": "/api/gwtest/{**path}"
        },
        "Order": 200,
        "ClusterId": "k8sGeo",
        "AuthorizationPolicy": "ValidateToken",
        "Transforms": [
          {
            "PathPattern": "/api/{path}"
          }
        ]
      }

and got these logs:
Request starting HTTP/1.1 GET http://partnersgw.securitycenter.windows.com/api/gwtest/mtp/provision - -
Executing endpoint '"Test"'
TenantRegionRoutingMiddleware: Routing request to https://wdatpprd-can.securitycenter.windows.com using routeId Test
Proxying to "https://wdatpprd-can.securitycenter.windows.com/api/mtp%252Fprovision" "HTTP/1.1" "RequestVersionOrLower" "no-streaming"
Received HTTP/"1.1" response 404.
Executed endpoint '"Test"'
Request finished HTTP/1.1 GET http://partnersgw.securitycenter.windows.com/api/gwtest/mtp/provision - - - 404 0 - 365.4845ms

As you see %252F is still there instead of /

@kevbry
Copy link
Contributor

kevbry commented Jul 15, 2022

TLDR: change your output pattern to "/api/{**path}" and your issue will be resolved. Also see #1600 which is probably the same thing (and also happened when changing .net versions).

Long form:

The issue is in how the input path/route parameters are transformed into the output path/route from config by PathRouteValuesTransformer in Yarp.

Both your input and output patterns are parsed by RouteParameterParser::ParseRouteParameter
https://github.com/dotnet/aspnetcore/blob/c85baf8db0c72ae8e68643029d514b2e737c9fae/src/Http/Routing/src/Patterns/RouteParameterParser.cs#L12

The input route parameter is parsed by endpoint routing using the above method when Yarp sets up routes from config. The output route parameter is parsed by Yarp in the PathRouteValuesTransform::ApplyAsync method (https://github.com/microsoft/reverse-proxy/blob/main/src/ReverseProxy/Transforms/PathRouteValuesTransform.cs#L55) , whose job is to take the route values parsed from the incoming request and use a TemplateBinder from asp.net core to format them into the outgoing request Uri.

Your input path is "/api/gwtest/{**path}" will be parsed as three segments by asp.net core routing, one of which is a RoutePatternParameter. In this RoutePatternParameter, the name is "path", it's of type "CatchAll", and encodeSlashes is set to false since it's a catch-all parameter (starts with "**").

Your output path of "/api/{path}" will be parsed as two segments, one of which is a RoutePatternParameter. In this RoutePatternParameter, the name is also "path", but it's of type "Standard" and encodeSlashes is set to true, since it isn't a catch-all parameter.

Now that it has both the input and output patterns, Yarp uses a binder to try to format values out of the input pattern and route values into the output pattern. It finds a parameter in the output pattern named "path", and formats the value from the input route into it. Since the parameter wasn't a catch-all, it hits the code path in TemplateBinder (https://github.com/dotnet/aspnetcore/blob/c85baf8db0c72ae8e68643029d514b2e737c9fae/src/Http/Routing/src/Template/TemplateBinder.cs#L584) and passes "true" in for EncodeSlashes, resulting in the behaviour you see.

This doesn't explain the change in behaviour between 5 and 6, though.

@anton-roos
Copy link

Changing to {**path} does not work on .NET6. I am still getting 404s if I have a %2F in my URL.

@MihaZupan
Copy link
Member

@anton-roos Please share your full configuration

@leonidzh
Copy link
Author

It worked for me when I changed to {**path} in both places - Match and Transforms. Thanks! I think this issue can be closed.

@anton-roos
Copy link

It is still giving me issues. I will link my configuration soon.

@anton-roos
Copy link

anton-roos commented Oct 27, 2022

@MihaZupan Sorry for the delay but here is my config:

{
  "AllowedHosts": "*",
  "ReverseProxy": {
    "Routes": {
      "dev": {
        "ClusterId": "dev",
        "Match": {
          "Path": "dev/{**catch-all}"
        },
        "Transforms": [
          {
            "PathPattern": "{**catch-all}"
          }
        ]
      },
      "sit": {
        "ClusterId": "sit",
        "Match": {
          "Path": "sit/{**catch-all}"
        },
        "Transforms": [
          {
            "PathPattern": "{**catch-all}"
          }
        ]
      },
      "uat": {
        "ClusterId": "uat",
        "Match": {
          "Path": "uat/{**catch-all}"
        },
        "Transforms": [
          {
            "PathPattern": "{**catch-all}"
          }
        ]
      },
      "trn": {
        "ClusterId": "trn",
        "Match": {
          "Path": "trn/{**catch-all}"
        },
        "Transforms": [
          {
            "PathPattern": "{**catch-all}"
          }
        ]
      }
    },
    "Clusters": {
      "dev": {
        "Destinations": {
          "dev": {
            "Address": "https://api.dev.somedomain.com"
          }
        }
      },
      "sit": {
        "Destinations": {
          "sit": {
            "Address": "https://api.sit.somedomain.com"
          }
        }
      },
      "uat": {
        "Destinations": {
          "uat": {
            "Address": "https://api.uat.somedomain.com"
          }
        }
      },
      "trn": {
        "Destinations": {
          "trn": {
            "Address": "https://api.trn.somedomain.com"
          }
        }
      }
    }
  }
}

When I try a URL that has %2 it falls over:
dev/someapi/api/v3/Entity/ZZA%2FQB%2F000000351/reference/Default

It does not route it correctly to the backend URL.

@anton-roos
Copy link

For the example above we want the backend URL to be:
https://api.dev.somedomain.com/dev/someapi/api/v3/Entity/ZZA%2FQB%2F000000351/reference/Default

Is there a way we can keep the %2f intact when proxying it through to the destination?

@Tratcher
Copy link
Member

Tratcher commented Nov 1, 2022

@anton-roos are you on .NET 6?

@anton-roos
Copy link

@Tratcher I am on .NET 6 yes.

@Tratcher
Copy link
Member

Tratcher commented Nov 2, 2022

@anton-roos I can't repro this with YARP 1.1.1, the %2F's pass through fine with your given config.

Are you sure the issue isn't at your destination servers? Do you have a network trace between YARP and your destinations?

Proxy:
      Request starting HTTP/1.1 GET http://localhost:5002/dev/someapi/api/v3/Entity/ZZA%2FQB%2F000000351/reference/Default - -
      Proxying to http://localhost:5000/someapi/api/v3/Entity/ZZA%252FQB%252F000000351/reference/Default HTTP/2 RequestVersionOrLower no-streaming
Destination:
      Request starting HTTP/1.1 GET http://localhost:5000/someapi/api/v3/Entity/ZZA%2FQB%2F000000351/reference/Default - -

@Tratcher Tratcher self-assigned this Dec 6, 2022
@Tratcher Tratcher removed their assignment Jan 3, 2023
@karelz karelz modified the milestones: YARP 2.0.0, Backlog Jan 9, 2023
@murilocurti
Copy link

murilocurti commented Jan 5, 2024

Changing the output PathPattern to {**path} solved for me. The following route works:

"entities": {
  "ClusterId": "entities",
  "Match": {
    "Path": "/entities/{**path}"
  },
  "Transforms": [
    {
      "PathPattern": "/{**path}"
    }
  ]
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted We will welcome a contribution Type: Documentation Improvements or additions to documentation
Projects
Status: 📋 Backlog
Development

No branches or pull requests

7 participants