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

Don't strip trailing slash if it's escaped in StripSlashes middlware #790

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nuuls
Copy link

@nuuls nuuls commented Jan 30, 2023

This fixes an edge case where a path ending in a trailing encoded slash %2F is incorrectly stripped and rctx.RoutePath is set to the unescaped Path, resulting in routing errors.

The specific edge case in my project is:

  • rctx is nil
  • rctx.RoutePath is empty
  • r.URL.RawPath contains multiple escaped slashes in various places
  • rctx.RoutePath is set to the unescaped values contained in r.URL.Path
  • The Mux no longer matches the route /link_resolver/{url} because all %2F are now /

A similar issue also exists in RedirectSlashes, I can open a second PR for that as well if you'd like.

Copy link
Contributor

@VojtechVitek VojtechVitek left a comment

Choose a reason for hiding this comment

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

LGTM

Can you please force-push to the branch to kick off CI?

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.

2 participants