mux icon indicating copy to clipboard operation
mux copied to clipboard

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

Open pcabreus opened this issue 2 years ago • 3 comments

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).

pcabreus avatar Apr 21 '22 21:04 pcabreus

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

borisavz avatar Apr 30 '22 09:04 borisavz

@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

with-shrey avatar May 02 '22 20:05 with-shrey

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

amustaque97 avatar Jun 05 '22 19:06 amustaque97

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

coreydaley avatar Aug 17 '23 01:08 coreydaley