gorilla / mux

Package gorilla/mux is a powerful HTTP router and URL matcher for building Go web servers with 🦍
https://gorilla.github.io
BSD 3-Clause "New" or "Revised" License
20.93k stars 1.85k forks source link

[bug] Wrong http method when clean path over an invalid path #673

Closed pcabreus closed 1 year ago

pcabreus commented 2 years ago

Describe the bug I was having an issue on my api calling a patch endpoint, that was never reach. Any patch call that I was making ended up on the get path of the same resource. By mistake I was making request to PATCH localhost//resources/item with two slashes. I think mux tried to clean the url by removing the double slashes but the original method was changed by a GET so I was finally receiving a GET localhost/resources/item.

Versions

Go version: 1.17 package version: run git rev-parse HEAD inside the repo

Steps to Reproduce

  1. Create a route PATCH localhost/resources/item
  2. Create a route GET localhost/resources/item
  3. Create a handler for each route (Be sure to know when any of this handler is called)
  4. Run the server and call the endpoint properly to check if working as expected
  5. Then call the patch endpoint with PATCH localhost//resources/item adding some extra slashes
  6. The Get endpoint is called

Expected behavior That is a not clean behavior. For my point of view could be any of:

  1. Not found because the path it not valid
  2. Preserve the method after the path cleaning.

Code Snippets

A minimum viable code snippet can be useful! (use backticks to format it).

borisavz commented 2 years ago

I came here to submit a bug report, but found a quite similar already submitted here.

router.StrictSlash(true) makes difference in case of missing trailing slash in path (ex. calling DELETE /configs/{uuid}), in that case the request gets redirected to GET handler. I've tried reordering HandleFuncs, but behavior remained the same.

Otherwise, calling DELETE /configs/{uuid}/// calls GET

Code:

router := mux.NewRouter()
router.StrictSlash(true)

...
//SLASH AT THE END!!!
router.HandleFunc("/configs/{uuid}/", server.getConfigHandler).Methods("GET")
router.HandleFunc("/configs/{uuid}/", server.delConfigHandler).Methods("DELETE")

Reproduction steps:

  1. Call DELETE /configs/{uuid} (without slash!!!), or DELETE /configs/{uuid}/// (invalid slashes)

Expected: Delete hadler is called, or some error

Actual: Get handler gets executed

with-shrey commented 2 years ago

@elithrar After a bit of triage, I found out that the redirection code here is the culprit.

Since the redirection can only to done to GET methods

The solution that I propose is to remove the redirection and use the cleaned path for finding a match. Let me know what are your thoughts / if you foresee any issues with this approach

Have raised a PR here

amustaque97 commented 2 years ago

Duplicate of https://github.com/gorilla/mux/issues/578

@elithrar we can close this issue and it's potential PR: https://github.com/gorilla/mux/pull/674

coreydaley commented 1 year ago

Duplicate of https://github.com/gorilla/mux/issues/578