-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
allow router to use request.URL.RawPath #209
base: master
Are you sure you want to change the base?
Conversation
It seems that go 1.6 and 1.7 don't have url.PathUnescape. It would be nice to version httprouter so that updated go src functions can be used. This PR could be merged into the latest release of httprouter but not affect previous versions. |
Just noticed PR #55 |
The tests on this seem to be failing. I have been using a forked version of this library to do this for a while; any chance you could update to fix the tests so that I can cherry-pick this commit instead? |
@SamWhited I updated my fork to pull in changes since I made this PR but until Go 1.6 and 1.7 passing tests are no longer part the criteria, Travis will continue to fail. See my comment above. |
Made a minor change and updated tests to pass code coverage |
Ping. This is a very useful fix to a very long standing problem... Any way to get this moving along? |
@julienschmidt is there any reason why it's not merged yet? |
@julienschmidt I updated this PR if it's helpful :) |
@julienschmidt any update on this? I think this change could fix bugs in many other projects(example) which depend on |
Also here looking for this. Any timelines on when this might be added? Thanks. |
Hi, is there any update on this at all? (or issue #284) I really could use this being implemented. I could switch permanently to the gravitational fork, but that is 48 commits behind, and I could implement the capability locally I guess, but I'd rather stick with the original. I'm temporarily switching between the two to temporarily fix issues as they come up. Any feedback greatly appreciated. Appreciate time is a challenge. Many thanks for any assistance you can provide. |
Bumps [github.com/labstack/echo/v4](https://github.com/labstack/echo) from 4.3.0 to 4.4.0. - [Release notes](https://github.com/labstack/echo/releases) - [Changelog](https://github.com/labstack/echo/blob/master/CHANGELOG.md) - [Commits](labstack/echo@v4.3.0...v4.4.0) --- updated-dependencies: - dependency-name: github.com/labstack/echo/v4 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Aaron Schlesinger <[email protected]>
Our kafka entities (e.g. consumer group names) contain unsafe symbols, such as forward slash and colon. This change forces Burrow (httprouter) to use encoded paths in URLs (Burrow plugin for Telegraf has always used PathEscape from "net/url" to form Burrow URLs). With raw path support, we do not loose metrics for groups with unsafe symbols in name. Original pull request to support raw path in httprouter: julienschmidt/httprouter#209, waiting for merge since 2017. Looks like httprouter was abandoned around 2019. The author switched to their own fork: gravitational/teleport#12109. Let's do the same.
This addresses issue #208
When a path parameter contains url escaped characters (especially "/" as "%2F") it uses the raw path and doesn't automatically unescape the path.
This adds a field to the router struct called "UseRawPath" which allows for backward compatibility.