[bug] Chaining route.Methods(...) more than once causes confusion

[tebruno99]

MIGRATED
From mux created by edgy-sphere: gorilla#694

Describe the bug
Using func (r *Route) Methods(methods ...string) more than once for a single route results in a response with status code 405 Method Not Allowed for this route.

Background
I tried to simplify the development of my web APIs, so I introduced some utility functions, with the relevant part essentially being:

func RegisterRoute(
    mux *mux.Router,
    path string,
    methods []string,
    handler func(http.ResponseWriter, *http.Request),
  ) {
    mux.HandleFunc(path, handler).Methods("OPTIONS").Methods(methods...)
  }

(OPTIONS is basically always required; methods as a slice because PUT, PATCH and even POST may point to the same handler)

Versions
go version go1.19 windows/amd64

package version: run git rev-parse HEAD inside the repo -- what repo? I used go to get [email protected]

Steps to Reproduce
(GitHub repo)

package main

import (
	"log"
	"net/http"

	"github.com/gorilla/mux"
)

func main() {
	mux := mux.NewRouter()
	mux.HandleFunc("/test", handler).Methods("PUT").Methods("PATCH")

	server := &http.Server{
		Addr: ":9710",
		Handler: mux,
	}

	err := server.ListenAndServe()
	if err != nil {
		log.Fatal(err)
	}
}

func handler(w http.ResponseWriter, r *http.Request) {
	w.WriteHeader(200)
}

Expected behavior

Response with status code 200 (for the latter example).

Solutions?
Either:

• Disallow using Methods(...) more than once, e.g. via changing field routeConf.matchers from type []matcher to type map[string]matcher, where key string (or any other practical type) may be "METHODS".
• Force method matchers to be unique and to be merged if it already exists, e.g. as per above.
• Change func (r *Route) Match(req *http.Request, match *RouteMatch) to not return ErrMethodMismatch if any method does not match, i.e. if there are multiple method matchers.

It has been not clear to me that using Methods(...) twice leads to this behaviour (hence this issue), so I would at least appreciate some kind of information at the function description in the source file -- although I do know now.
Also, I neither fully understand your intended design nor Git things like Pull Requests, so I am sorry if my provided solutions may very well be rather lacking.

[tebruno99]

MIGRATED
Original Comment: @amustaque97 gorilla#694 (comment)

Correct syntax to call Methods method is to write something like this:

mux.HandleFunc("/test", handler).Methods("PUT", "GET", "OPTIONS")

I believe there are ample examples mentioned in the README.md file. For instance under the matching routes section there is an example like

r.Methods("GET", "POST")

Regarding your background utility function, you can write something like this

func RegisterRoute(
    mux *mux.Router,
    path string,
    methods []string,
    handler func(http.ResponseWriter, *http.Request),
  ) {
    methods = append(methods, "OPTIONS")
    mux.HandleFunc(path, handler).Methods(methods...)
  }

Just for understanding, I have tried to write a basic generic minimal code to understand what happens when we call Methods multiple times.

// You can edit this code!
// Click here and start typing.
package main

import "fmt"

type matcher interface{}

func main() {
	str := []matcher{"GET"}
	str = append(str, matcher([]string{"PUT"}))

	fmt.Println(str)

}

Output of the above code is

[GET [PUT]]

Similarly in mux while matching the HTTP method [PUT] is not a valid method thus returns 405 not allowed.

I hope I answered your queries and doubts @edgy-sphere
Thank you!