Skip to content

fix(middleware): keep RawPath in sync in trailing slash middlewares - #3134

Open
breken-ai wants to merge 1 commit into
labstack:masterfrom
breken-ai:fix/trailing-slash-escaped-path
Open

breken-ai wants to merge 1 commit into
labstack:masterfrom
breken-ai:fix/trailing-slash-escaped-path

Conversation

@breken-ai

Copy link
Copy Markdown

The router matches req.URL.RawPath when it is set, but AddTrailingSlash and RemoveTrailingSlash (in rewrite mode, without RedirectCode) only change req.URL.Path.

Go sets RawPath whenever the client's escaping differs from Go's default, which is common: encodeURIComponent("jane@example.com") gives jane%40example.com. For such requests the slash middlewares do nothing the router can see:

  • e.Pre(middleware.AddTrailingSlash()) with e.GET("/users/:email/", h): GET /users/jane%40example.com returns 404.
  • e.Pre(middleware.RemoveTrailingSlash()) with e.GET("/users/:email", h): GET /users/jane%40example.com/ reaches the handler with c.Param("email") == "jane%40example.com/". The slash that should have been removed ends up in the last path parameter.

Requests without escapes are unaffected, which is why this is easy to miss.

The fix updates RawPath next to Path in both middlewares when it is set (add or trim the trailing /). Redirect mode is unchanged.

Tests: TestAddTrailingSlash_escapedPathIsRouted and TestRemoveTrailingSlash_escapedPathIsRouted go through ServeHTTP. Without the fix the first gets 404 and the second gets jane%40example.com/. With the fix both pass, and so do go test -race ./middleware/ and go vet ./middleware/. middleware/slash.go on v4 has the same code, if you want a backport.

This change was found and written with AI assistance, and the failing and passing test runs above were run locally.

The router matches req.URL.RawPath when it is set, but AddTrailingSlash and
RemoveTrailingSlash only changed req.URL.Path. For a path with an escape
that differs from Go's default encoding (e.g. %40 from encodeURIComponent)
the added slash was ignored (404) and the removed slash stayed in the last
path parameter.
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.

1 participant