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

feature: percent-decode before routing #2729

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

mladedav
Copy link
Collaborator

@mladedav mladedav commented May 3, 2024

Note that this PR is built on top of #2645 so it also contains its changes. The only relevant change is the last commit.

Closes #2678

Motivation

Matching was happening before percent decoding. Therefore, /a and /%61 were two different paths although according to the spec they shouldn't be.

We were also unable to construct routes with special characters that would have to be percent encoded by clients such as ? or users would have to specify their paths already percent encoded when calling route.

Solution

The paths are percent decoded before passing them to matchit. One exception is the forward slash /, which must not be decoded as that could change the matching behavior. This is achieved by encoding the % in %2f which would otherwise be decoded to / and interpreted as a path separator by matchit::Router. This way, the matchit::Router still sees %2f which gets decoded later on when inserting matched captures to request extensions.

For example with a routes /api/{resource} and /api/car/{name}, making a call such as /api/car%2flada should match the first route, not the second one.

One drawback of this is that if a user makes a request such as /%252f_is_slash, which would be captured by route /{capture}, the value of the captured path should be %2f_is_slash but because slashes are decoded after everything else, this will actually produce /_is_slash. This should be very rare scenario though.

Other characters work as intended, e.g. request to /%2561_a would result in a capture of %61_a.

@2ndDerivative
Copy link

Is this blocked by anything that needs work still?

@jplatte
Copy link
Member

jplatte commented Nov 26, 2024

This is blocked on making a decision over in #2678.

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.

Routing percent-encoded paths
3 participants