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

MIGRATED: Fix accidental double-escaping when Router.UseEncodedPath() is enabled #11

Conversation

tebruno99
Copy link

MIGRATED
Original PR: @nvx gorilla#641

Fixes gorilla#354
Fixes #10


Bug originally mentioned in gorilla#354 (comment)

Summary of Changes

When Router.UseEncodedPath() is enabled (and SkipClean is not enabled), a request to http://localhost/foo/../bar%25 causes a redirect to http://localhost/bar%2525 as there is a bug where this code path results in double-encoding (the % from the %25 is itself encoded as %25, resulting in %2525).

The expected output for this should have been a redirect to http://localhost/bar%25. A unit test for this behaviour has been added which originally failed with the following output:

Expected redirect location to http://localhost/bar%25, found http://localhost/bar%2525
The remaining changes allows the new unit test to pass and does not break any existing unit tests.

@tebruno99 tebruno99 merged commit 3a29f33 into ElasticPerch:bugfix/10-path-incorrectly-decoded-during-clean Dec 15, 2022
@nvx
Copy link

nvx commented Dec 21, 2022

I note this isn't merged into the main branch. Is there a plan for this (and presumably the other similar PRs)?

Are you guys looking at maintaining gorilla mux going forward?

@tebruno99
Copy link
Author

I note this isn't merged into the main branch. Is there a plan for this (and presumably the other similar PRs)?

Are you guys looking at maintaining gorilla mux going forward?

I pulled in all the original PRs into their own branches here so that I could test and familiarize myself with the code base and changes before merging the PRs to main. I was not previously a mux contributor so this will take a little time. I hope to either accept or reject the migrated PRs by mid Jan '23 after some time over the holidays reviewing.

The intent is to maintain mux & accept community contributions when they align well with general usage, etc. Eventually, if need or want to also start new feature development.

@brusMX
Copy link

brusMX commented Feb 1, 2024

@tebruno99 are there any updates to get this merged into main?

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.

3 participants