Skip to content

Fix PathPattern encoding #1321

Merged
Tratcher merged 1 commit into
mainfrom
tratcher/pathpattern
Oct 27, 2021
Merged

Fix PathPattern encoding #1321
Tratcher merged 1 commit into
mainfrom
tratcher/pathpattern

Conversation

@Tratcher
Copy link
Copy Markdown
Member

@Tratcher Tratcher commented Oct 22, 2021

Fixes #1307

Sample route from the issue:

      "CommonRestRoute": {
        "ClusterId": "cluster1",
        "Match": {
          "Path": "/service1/rest/{**remainder}"
        },
        "Transforms": [
          { "PathPattern": "/{remainder}" },
        ]
      },

PathPattern is supposed to take route parameters matched from the path and inject them into the given pattern. ** in the path pattern indicates a catch-all that will capture the remainder of the path.

Problem: When the catch-all captured multiple path segments the resulting path would have the /'s escaped as %2F. You should have been able to specify the PathPattern as { "PathPattern": "/{**remainder}" }, to avoid this, but it doesn't work with the RouteTemplate type we're using.

Solution: Use RoutePattern instead of RouteTemplate. This comes with a few usage differences.

  • If you try to bind a value not present in the template it will be added as a query parameter. I had to filter these out.
  • We had been passing the RouteParameters in to the defaults parameter to avoid the prior issue, but with RoutePattern it would cause default segments to be dropped. We no longer pass in the defaults collection.

@Tratcher Tratcher added this to the YARP 1.0.0 milestone Oct 22, 2021
@Tratcher Tratcher requested a review from MihaZupan October 22, 2021 22:11
@Tratcher Tratcher self-assigned this Oct 22, 2021
Copy link
Copy Markdown
Member

@MihaZupan MihaZupan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So aside from the change in undesired escaping, I take it the observable behavior from the user's perspective is the same here?

@Tratcher
Copy link
Copy Markdown
Member Author

So aside from the change in undesired escaping, I take it the observable behavior from the user's perspective is the same here?

The biggest change is the clarification that the user needs to use the ** syntax in two places, in the route match and in the transform. Otherwise everything else should work as before.

@Tratcher
Copy link
Copy Markdown
Member Author

Waiting on a review from Javier.

@Tratcher Tratcher merged commit 3c059d5 into main Oct 27, 2021
@Tratcher Tratcher deleted the tratcher/pathpattern branch October 27, 2021 22:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PathPattern encoding issue on .NET 6.0 RC2

2 participants